Repository navigation
[release/17.0] Allocate the next dashboard run ID when two dashboards start in the same millisecond - #22129
Closed
Sébastien Ros (sebastienros) wants to merge 1 commit into
Conversation
…ame millisecond (#21913) ## Description **What failed:** `DashboardUrlTests.DashboardRejectsWrongCrossApplicationAndBogusLoginTokens` failed 3 times on ubuntu in 7 days ([42064740887](https://github.com/microsoft/aspire/actions/runs/42064740887) on main, [41606741729](https://github.com/microsoft/aspire/actions/runs/41606741729), [42142277750](https://github.com/microsoft/aspire/actions/runs/42142277750)). `aspire-dashboard` failed to start with `InvalidOperationException: Dashboard run '<id>' is already in use by another dashboard process` (inner `IOException` on `runs/<id>.lock`). ## Failure stats (14d, attempt 1) `CI` workflow on microsoft/aspire, runs created 2026-10-25 06:28 → 2026-11-08 06:28 UTC, **attempt 1 only**. Denominator: attempt-1 jobs that ran (success or failure). Hits: the failed job's log matches the signature. Jobs that lost their runner, which have no log, are excluded. Main runs have no reruns, so their flake check is the next main run of the job. | | | |---|---| | Signature | `DashboardUrlTests.DashboardRejectsWrongCrossApplicationAndBogusLoginTokens` failed **and** the log has `Dashboard run '<id>' is already in use by another dashboard process` | | Job | Hosting.Testing (all OSes) | | main pushes: hits / runs | 1 / 316 (0.32%) | | main pushes, 30d: hits / runs | 1 / 546 (0.18%). The 16 days before the 14d window add 0 hits. | | PR runs: hits / runs | 2 / 1138 (0.18%); 0 PR-caused excluded | | Combined rate | 3 / 1454 = **0.21%** | | Flake check | main: next main run of the job passed 1/1; PR: 2/2 rerun hits passed on rerun (0 never rerun) | | First / last hit | 2026-11-03 / 2026-11-06 | | Still hitting main? | Last main hit: 2026-11-05. 46 main runs since the last hit (2026-11-06), with 0 hits | | Fix evidence | Local: same-millisecond collision unit tests fail before (same exception as CI) → 20/20 after. This PR's CI: Hosting.Testing (ubuntu + windows) passed 1/1 on the head commit (plus 2/2 attempts on an earlier commit). | Failing attempt-1 jobs, most recent first (3 of 3 log-verified hits): | Date (UTC) | Branch | Job | Signature excerpt | |---|---|---|---| | [2026-11-06 01:12](https://github.com/microsoft/aspire/actions/runs/42142277750/job/127646217683) | `sebros/resource-tag-position-storage-4a8108` | Hosting.Testing (ubuntu-latest) | `Dashboard run '20261106T012000537Z' is already in use by another dashboard process` | | [2026-11-05 16:28](https://github.com/microsoft/aspire/actions/runs/42064740887/job/127379268400) | **main** | Hosting.Testing (ubuntu-latest) | `Dashboard run '20261105T163937728Z' is already in use by another dashboard process` | | [2026-11-03 08:54](https://github.com/microsoft/aspire/actions/runs/41606741729/job/125819602511) | `tommasodotnet-linux-tray-support` | Hosting.Testing (ubuntu-latest) | `Dashboard run '20261103T090228658Z' is already in use by another dashboard process` | **Verdict: MODERATE.** Low frequency (3 hits, 1 on main, all ubuntu, all passed on rerun or on the next main run), but the exact CI exception reproduces deterministically in a unit test and the root cause is a real product race. Evidence that the end-to-end test stops failing rests on that unit repro; the PR's CI sample is too small to show it. <!-- /failure-stats --> **Why:** this is a product race, not a test bug. The test starts two AppHosts with `Task.WhenAll`. In `Run` persistence mode, `DashboardRunStore` uses the start time in milliseconds as the run ID and fails fast on a lock collision. When both dashboards start in the same millisecond, the second one crashes. Any two AppHosts started together can hit this. **Fix:** `DashboardRunStore` now claims the run lock first and requires the run directory to be new. If the lock is held or the directory already exists, it moves to the next millisecond and tries again (bounded by a named `MaxRunIdAttempts` constant). This keeps the run ID format and the chronological ordinal ordering used by pruning, and it never reuses an earlier run's directory. `docs/specs/dashboard-persistence.md` is updated. Resume mode still fails fast on a lock collision, as before. Refs #21600 ## Confidence - New regression tests in `DashboardDataSourceTests`: - `RunMode_ConcurrentTimestampCollision_AllocatesNextRunId` replaces `RunMode_RejectsConcurrentTimestampCollision`, which asserted the old crash. - `RunMode_ExistingRunDirectoryWithSameTimestamp_IsNotReused` - `RunMode_RunIdAllocationExhausted_PreservesLockAcquisitionError`: if every candidate fails, the error says allocation was exhausted and keeps the underlying I/O or permission exception. Added after review feedback. It fails before that change. - **Before the fix:** the two collision tests fail. The concurrent case throws the same exception seen in CI. - **After the fix:** the collision tests pass 20/20, and `DashboardDataSourceTests` passes 64/64. - **Ad-hoc stress test (not committed):** I created 16 stores in parallel at the same fixed timestamp, 50 times. Every run ID was unique. - **Full `Aspire.Dashboard.Tests`:** 2625/2626 pass. The one failure, `OtlpGrpcServiceTests.CallService_OtlpGrpcEndPoint_ExternalFile_FileChanged_UseConfiguredKey`, is a file-watch test that also fails on unmodified main in this environment. - **Not run locally:** the end-to-end `DashboardUrlTests` (it needs DCP) and Windows/macOS. CI covers those. **How we stop this recurring:** run ID allocation now handles collisions instead of assuming they are "very unlikely", and the unit tests reproduce the same-millisecond collision deterministically with a fixed `TimeProvider`. ## Checklist - Is this feature complete? - [x] Yes. Ready to ship. - Are you including unit tests for the changes and scenario tests if relevant? - [x] Yes - Did you add public API? - [x] No - Does the change make any security assumptions or guarantees? - [x] No **🤖 Posted by AspireAgent (AI Agent)** --------- Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> (cherry picked from commit fbad388)
Sébastien Ros (sebastienros)
requested review from
James Newton-King (JamesNK) and
Adam Ratzman (adamint)
as code owners
November 11, 2026 04:30
This was referenced Nov 11, 2026
James Newton-King (JamesNK)
approved these changes
Nov 11, 2026
David Fowler (davidfowl)
approved these changes
Nov 11, 2026
Ammann Labs (agammann)
left a comment
Ammann Labs (agammann)
left a comment
There was a problem hiding this comment.
CreateNewRunDirectory retries every UnauthorizedAccessException from OpenRunLock as if it were a timestamp collision. If the runs directory or lock path is unwritable, startup now makes 1,000 filesystem attempts before reporting the permission failure; the new exhaustion test exercises exactly that case. Could access-denied failures fail immediately, leaving retries for actual lock contention or pre-existing directories? That would keep a persistent configuration error quick and clear while preserving the collision fix.
Member
Jose Perez Rodriguez (joperezr)
added a commit
that referenced
this pull request
Nov 12, 2026
Combines the stacked dashboard backports into one PR for `release/17.0`. The commits are cherry-picked in stack order, one per source PR, onto the current `release/17.0` tip: 1. #22125: Store dashboard tags, resource order and metric pins in browser local storage (#21840) 2. #22126: Add dashboard resource-selector dropdown (#21861) 3. #22127: Rename the WithTags trait from Aspire_Dashboard_Tag to Aspire_Tag (#21881) 4. #22128: Restore single-line compact rows in the dashboard resource list (#21880) 5. #22129: Allocate the next dashboard run ID when two dashboards start in the same millisecond (#21913) 6. #22130: Simplify the dashboard around its two main jobs (#21943) 7. #22131: Return 400 for OTLP JSON requests with malformed ids (#21436) 8. #22132: Address deferred review feedback for the modern dashboard (#21766) ## Conflict resolution The stack was cut from an older `release/17.0`. Since then, #22123 (Harden Go SDK runtime) landed and regenerated `TwoPassScanningGeneratedAspire.verified.go`. That conflicted with #21766, which replaces `WithDashboardPosition` with `WithDisplayOrder`. The resolution drops `WithDashboardPosition` and regenerates the snapshot with the current Go generator, so `WithDisplayOrder` now uses the hardened `s.Err()` pattern. The other seven commits applied cleanly. ## Validation All `Aspire.Hosting.CodeGeneration.*.Tests` projects pass locally (Go, Java, Python, Rust, TypeScript). --------- Co-authored-by: Sébastien Ros <1165805+sebastienros@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: James Newton-King <james@newtonking.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: David Fowler <davidfowl@gmail.com> Co-authored-by: davidfowl <95136+davidfowl@users.noreply.github.com> Co-authored-by: Maddy Montaquila <maddyleger1@gmail.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Maddy Montaquila <maddy@MadBook-Pro-20.local> Co-authored-by: Taha Koçal <tahakocalgs@gmail.com> Co-authored-by: Jose Perez Rodriguez <joperezr@microsoft.com>
|
Follow-up to my earlier review: I rechecked the shared |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #21913 to release/17.0
/cc Sébastien Ros (@sebastienros)
Note
Part of a chain of dashboard backports. This PR targets the branch of #22128, so review only its own commit. Merge the chain in order; GitHub retargets each PR as its base merges.
Customer Impact
Testing
Risk
Regression?