Repository navigation
De-flake ClusterSpec: stop passing an already-cancelled token to its WithinAsync block - #8540
Merged
Merged
Conversation
…WithinAsync block A_cluster_must_cancel_LeaveAsync_task_if_CancellationToken_fired_before_node_left cancels `cts` to prove LeaveAsync(cts.Token) is cancelled, then reused that same already-cancelled token as the `cancellationToken:` argument to the 10s WithinAsync block that drives Leaving -> Exiting -> Removed. WithinAsync races the block against a delay bound to that token; with the token born cancelled, any real suspension point inside the block before the race resolves throws OperationCanceledException out of WithinAsync (seen on CI as a 71ms failure). Fast/synchronous runs hid the bug; slower runs exposed it. Fix: drop the cancellationToken: argument so the block uses the default token. While here, switch the synchronous ExpectMsg<ClusterEvent.MemberRemoved>() call inside the block to the async ExpectMsgAsync, per the repo's async-only TestKit convention.
…sync API Migrates all 7 TestKit Shutdown(ActorSystem) calls in ClusterSpec.cs to ShutdownAsync, awaited in their enclosing async Task facts.
Aaronontheweb
force-pushed
the
fix/clusterspec-leaveasync-cancelled-token
branch
from
September 9, 2026 18:18
006f48e to
5ec4293
Compare
Aaronontheweb
deleted the
fix/clusterspec-leaveasync-cancelled-token
branch
September 10, 2026 12:57
This was referenced Sep 10, 2026
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
…WithinAsync block (#8540) * De-flake ClusterSpec: stop passing an already-cancelled token to the WithinAsync block A_cluster_must_cancel_LeaveAsync_task_if_CancellationToken_fired_before_node_left cancels `cts` to prove LeaveAsync(cts.Token) is cancelled, then reused that same already-cancelled token as the `cancellationToken:` argument to the 10s WithinAsync block that drives Leaving -> Exiting -> Removed. WithinAsync races the block against a delay bound to that token; with the token born cancelled, any real suspension point inside the block before the race resolves throws OperationCanceledException out of WithinAsync (seen on CI as a 71ms failure). Fast/synchronous runs hid the bug; slower runs exposed it. Fix: drop the cancellationToken: argument so the block uses the default token. While here, switch the synchronous ExpectMsg<ClusterEvent.MemberRemoved>() call inside the block to the async ExpectMsgAsync, per the repo's async-only TestKit convention. * ClusterSpec: migrate the remaining synchronous TestKit calls to the async API Migrates all 7 TestKit Shutdown(ActorSystem) calls in ClusterSpec.cs to ShutdownAsync, awaited in their enclosing async Task facts. (cherry picked from commit 9705d36)
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.
What changes
ClusterSpec.A_cluster_must_cancel_LeaveAsync_task_if_CancellationToken_fired_before_node_leftno longer passes an already-cancelled token to its own wait block. One synchronousExpectMsginside that block becomesawait ExpectMsgAsync, per the repo rule that tests use the async TestKit calls.Why
The test creates a token source, calls
LeaveAsyncwith its token, cancels it, confirms that task is cancelled, and then passed the same cancelled token as thecancellationToken:of the 10 secondWithinAsyncblock that drives the leader through Leaving, Exiting, and Removed.WithinAsyncraces the block against a delay bound to that token. With a cancelled token the delay is born cancelled, so whenever the block hits a real suspension point before the race is checked,WithinAsyncthrowsOperationCanceledException. A fast run finishes the block synchronously and hides it. A slow run, like CI build 131162 on a one-line generator PR, fails in 71 ms with a message that has nothing to do with the cancellation behavior under test.The bug has been in the test since the
cancellationToken:argument was added in 2022. No prior fix targeted it. The de-flake inventory for this week's PR builds records the history.How it was checked
dotnet build src/core/Akka.Cluster.Tests -c Release -warnaserrorclean. The single test run 25 times in a row: 25 passes. The wholeClusterSpecclass: 20 passed.Second commit: the remaining synchronous TestKit calls move to the async API
Per the maintainer's rule that a touched file migrates in the same PR: the seven
Shutdown(system)calls in the file'sfinallyblocks becomeawait ShutdownAsync(system). Three_cluster.Shutdown()calls stay; that is the cluster extension's own method, not a TestKit wait, and it has no async form. No assertion changes. The wholeClusterSpecclass run three times after the migration: 20 passed each time.