Repository navigation
fix: migrate NodeChurnSpec to async TestKit APIs to fix shutdown-timeout flakiness - #8286
Merged
Aaronontheweb merged 1 commit intoJun 24, 2026
Conversation
…out flakiness NodeChurnSpec intermittently failed with "Failed to stop [NodeChurnSpec] within [00:00:05]" on slower CI agents. The synchronous Shutdown(node, verifySystemShutdown:true) helper blocks a thread-pool thread inside Task.Wait() for its 5s budget while the coordinated-shutdown pipeline itself needs the thread pool to make progress — a sync-over-async self-starvation. Migrate the spec to the async, task-returning TestKit APIs (WithinAsync, AwaitMembersUpAsync, EnterBarrierAsync, AwaitAssertAsync, ExpectNoMsgAsync) and replace the blocking per-system Shutdown loop with a concurrent `await Task.WhenAll(systems.Select(s => s.Terminate())).WaitAsync(30s)`, which frees the thread and preserves verify-shutdown semantics (throws TimeoutException if a system fails to stop). Same idiom as QuickRestartSpec / DistributedPubSubRestartSpec. Verified locally across 4 consecutive runs (3/3 node roles pass each, ~45s).
Member
Author
|
Basically this addresses a ton of legacy sync-over-async issues from the original TestKit / MNTR designs |
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.
Problem
NodeChurnSpecintermittently fails on CI with:(Most recently surfaced on the
Microsoft.NET.Test.Sdk17.9.0 → 18.7.0 bump in #8282, which is purely an environmental/test-runner timing change — not a product regression.)Root cause
The spec creates two transient
ActorSystems per round and tears them down with the synchronousShutdown(node, verifySystemShutdown:true)helper, which doessystem.Terminate().Wait(5s). That blocks a thread-pool thread for up to its 5s budget while the coordinated-shutdown pipeline (run-coordinated-shutdown-when-down = on→ cluster leave/down + remoting teardown + scheduler stop) needs that same thread pool to make progress. On slower/contended CI agents this sync-over-async self-starvation pushes a normally-fast shutdown past the hardcoded 5s default and the verify path throws.Fix
Migrate the spec to the async, task-returning TestKit APIs and stop blocking on shutdown:
[MultiNodeFact] void→async Task(already the dominant pattern; sibling specsLeaderElectionSpec/ClusterAccrualFailureDetectorSpecin this project are async)Within/AwaitMembersUp/EnterBarrier/AwaitAssert/ExpectNoMsg→WithinAsync/AwaitMembersUpAsync/EnterBarrierAsync/AwaitAssertAsync/ExpectNoMsgAsyncShutdown(...)loop with concurrent async termination:WaitAsyncthrowsTimeoutExceptionif a system fails to stop in time). Same idiom already used byQuickRestartSpecandDistributedPubSubRestartSpec.No production code is touched — this is a test-only change.
Verification
NodeChurnSpeclocally 4 consecutive times —Passed! Failed: 0, Passed: 3every run (~44–48s each).Notes
This is one of two flaky-test fixes split out from the #8282 investigation. The
CircuitBreakerSpectiming fragility (same family of cold-start/thread-pool root cause) will be addressed in a separate PR.