Skip to content

Bound Orleans cluster teardown so it stops starving the running test - #1051

Merged
rbuergi merged 2 commits into
mainfrom
fix/orleans-shard0-cluster
Aug 10, 2026
Merged

rbuergi merged 2 commits into
mainfrom
fix/orleans-shard0-cluster

Conversation

@rbuergi

@rbuergi rbuergi commented Aug 9, 2026

Copy link
Copy Markdown
Contributor

The shard-0 Orleans flake is one cross-test interaction wearing six names. Across four runs it has failed as OrleansStaticRepoImportTest, OrleansMeshTests.HubWorksAfterDisposal, DelegationCompletionTest, OrleansMarkdownExportTest (twice, two different methods) and OrleansBrokenNodeTypeAccessTest — the last of those on #1045, whose entire diff adds Class="@Class" to four buttons in a .razor. None of them were anyone's diff.

Root cause

OrleansClusterDisposal hands each TestCluster's stop→dispose to IoPool.**Unbounded** and makes it hot immediately. Both of those are right on their own: teardown must not block the xUnit thread (awaiting it inline was the original deadlock this file was written to fix), and the next class should start at once.

With no bound, though, every cluster the assembly has built can be shutting down at once underneath the class currently running. Silo shutdown is not cheap. On a 4-vCPU runner those overlapping teardowns starve the live test's render, which then dies on its own 30s Timeout. The victim is whoever happens to be running when enough teardowns pile up — hence a different name every time, and hence it never correlated with any change.

Reproduced deterministically

Not a stress harness — just the runner's shape, via DOTNET_PROCESSOR_COUNT:

build processors result
unbounded 18 (native) 141/141 pass
unbounded 4 1 FAILED — 30s timeout
unbounded, victim class alone 4 5/5 pass in 5s
bounded(2) 4 141/141 pass
bounded(2) 2 141/141 pass

The third row is the one that settles it: the class that fails in the suite passes by itself under the same constraint, so it is the overlap and not the test.

The fix

IoPool(2) instead of IoPool.Unbounded for the disposal legs.

This is the framework's own prescription rather than a tuning knob — AGENTS.md: "concurrency bounding … channels through IIoPool" — and IoPool(int maxConcurrency) already exists. Teardown stays off the teardown thread, so the deadlock the background disposal exists to avoid stays avoided; it simply stops competing with the suite for the runner. Legs of different clusters never wait on one another, so the bound cannot deadlock the final drain.

No What's New entry: test infrastructure, no user-visible change.

🤖 Generated with Claude Code

The shard-0 Orleans flake was one cross-test interaction wearing six names.
OrleansClusterDisposal hands each TestCluster's stop→dispose to IoPool.Unbounded
and makes it hot immediately — correct, in that teardown must not block the
xUnit thread (that was the original deadlock) and the next class should start at
once. But with no bound, every cluster the assembly has built can be shutting
down AT ONCE underneath the class currently running. Silo shutdown is not cheap;
on a 4-vCPU runner those overlapping teardowns starve the live test's render,
which then dies on its own 30s Timeout. The victim is simply whoever is running
when enough teardowns pile up, which is why it was a different test every run
and why it never looked like anyone's diff.

Reproduced deterministically with DOTNET_PROCESSOR_COUNT=4 — the runner's shape,
not a stress harness:

  unbounded, 18 cores (native)   141/141 pass
  unbounded, 4 processors          1 FAILED (30s timeout)
  the victim class ALONE, 4        5/5 pass in 5s   → the overlap, not the test
  bounded(2), 4 processors       141/141 pass
  bounded(2), 2 processors       141/141 pass

Bounding is the framework's own prescription — "concurrency bounding channels
through IIoPool" — and IoPool(maxConcurrency) already exists. It keeps teardown
off the teardown thread, so the deadlock the background disposal was written to
avoid stays avoided, while stopping teardown from competing with the suite.
Legs of different clusters never wait on each other, so the bound cannot
deadlock the final drain.

No What's New entry: test infrastructure, no user-visible change.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings August 9, 2026 21:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR addresses an Orleans test-suite flake caused by cross-test resource starvation by bounding concurrent Orleans cluster teardown work, while still keeping teardown off the xUnit teardown thread (avoiding the original deadlock risk).

Changes:

  • Introduces a bounded IIoPool (new IoPool(2)) as a shared gate for Orleans TestCluster teardown.
  • Routes teardown legs through the bounded pool instead of IoPool.Unbounded to prevent concurrent shutdowns from starving currently-running tests.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 142 to 145
private static IObservable<Unit> RunVoid(Func<Task> leaf) =>
IoPool.Unbounded
DisposalPool
.Run(_ => leaf().ContinueWith(static _ => Unit.Default, TaskScheduler.Default))
.Catch(Observable.Return(Unit.Default));
@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 0)

690 tests   690 ✅  13m 46s ⏱️
 10 suites    0 💤
 10 files      0 ❌

Results for commit 062e31f.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 4)

1 284 tests   1 279 ✅  6m 50s ⏱️
   11 suites      5 💤
   11 files        0 ❌

Results for commit 062e31f.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 5)

1 725 tests   1 533 ✅  6m 17s ⏱️
   11 suites    192 💤
   11 files        0 ❌

Results for commit 062e31f.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 2)

1 947 tests   1 945 ✅  6m 0s ⏱️
   11 suites      2 💤
   11 files        0 ❌

Results for commit 062e31f.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 3)

   11 files     11 suites   4m 49s ⏱️
1 976 tests 1 868 ✅ 108 💤 0 ❌
2 121 runs  2 013 ✅ 108 💤 0 ❌

Results for commit 062e31f.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (shard 1)

1 202 tests   1 202 ✅  6m 10s ⏱️
   11 suites      0 💤
   11 files        0 ❌

Results for commit 062e31f.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Aug 9, 2026 •

Copy link
Copy Markdown
Contributor

Test Results

   65 files     65 suites   43m 54s ⏱️
8 824 tests 8 517 ✅ 307 💤 0 ❌
8 969 runs  8 662 ✅ 307 💤 0 ❌

Results for commit 062e31f.

♻️ This comment has been updated with latest results.

@rbuergi
rbuergi merged commit 75e8af8 into main Aug 10, 2026
17 checks passed
@rbuergi

rbuergi commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

Reporting a residual, from a branch that has your IoPool(2) bound merged in.

#1051 reduced this but did not eliminate it. On a re-synced branch (PR #1050), build green and shards 1-5 green, shard 0 red on OrleansMarkdownExportTest.SubHub_…ViaRegistryChain — another of the six aliases this issue itself enumerates, and the same shape every time: 141 tests, exactly 1 failure, TimeoutException, a different test name on each run.

That "different name each run, always exactly one" signature is the thing worth acting on — it says the failure is a property of the assembly's teardown/startup contention rather than of any test, so chasing whichever name surfaced is chasing an alias.

Attribution for that particular occurrence, so it is not mistaken for the PR under test: the failing test's own output shows Created Markdown node: TestUser/subhub-chain, i.e. the create was answered; the timeout is the later resolution at line 282. The PR's diff cannot reach it.

Not re-run to fish for green, per the standing rule.

Two other data points from tonight that may help size it:

Worth reopening or filing a follow-up for the residual — the bound helped, but the assembly still loses exactly one test per run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants