Repository navigation
De-flake ClusterShardingLeaseSpec: join the cluster in InitializeAsync instead of blocking a pool worker in the constructor - #8558
Merged
Conversation
Aaronontheweb
force-pushed
the
fix/sharding-lease-spec-async-join
branch
from
September 9, 2026 18:19
d632926 to
6c253de
Compare
Aaronontheweb
added this pull request to stack #8578
September 10, 2026 12:59
…c instead of blocking a pool worker in the constructor The constructor's blocking wait for Up against a flat 3s default missed by 462ms on CI, because the join's six dispatches queued behind the parked worker on a starved thread pool. Cluster formation now happens in IAsyncLifetime.InitializeAsync via JoinAsync/StartAsync, which waits on the real MemberUp signal without parking a thread. The file is also migrated to the async TestKit API per the repo's standing rule.
…ient-context attribute already pins the cell
Aaronontheweb
force-pushed
the
fix/sharding-lease-spec-async-join
branch
from
September 10, 2026 15:27
ea12ef2 to
0649029
Compare
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
…c instead of blocking a pool worker in the constructor (#8558) * De-flake ClusterShardingLeaseSpec: join the cluster in InitializeAsync instead of blocking a pool worker in the constructor The constructor's blocking wait for Up against a flat 3s default missed by 462ms on CI, because the join's six dispatches queued behind the parked worker on a starved thread pool. Cluster formation now happens in IAsyncLifetime.InitializeAsync via JoinAsync/StartAsync, which waits on the real MemberUp signal without parking a thread. The file is also migrated to the async TestKit API per the repo's standing rule. * ClusterShardingLeaseSpec: drop the redundant TestActor touch; the ambient-context attribute already pins the cell (cherry picked from commit 408e008)
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
ClusterShardingLeaseSpec, the base that all fifteen lease tests and variants run through, no longer joins the cluster from its constructor with a blocking wait. The join moves intoInitializeAsyncand awaitsCluster.JoinAsyncunder a dilated 30 s cancellation bound, which completes on the node's ownMemberUpand parks no thread. Sharding starts withStartAsync. ADisposeAsyncbridge callsDispose()so xUnit v3 still runs the TestKit teardown; once #8545 lands this becomes anoverridethat chains to the base, and a comment says so.The facts pass
TestActorexplicitly on every region tell, because the implicit sender reads a thread-static cell and a fact resumes on a different thread afterInitializeAsync. The nested expect inside the lease-lost recovery loop gets a 1 s inner budget so the outer 10 s loop retries about ten times instead of three. The whole file moves to the async TestKit API per the maintainer's rule: every fact isasync Task, and every wait is theAsyncform. No assertion changes.Why
Build 131196 (Windows unit tests) failed
PersistenceClusterShardingLeaseSpec.Cluster_sharding_with_lease_should_recover_if_lease_lost, but not in the lease logic. The constructor asserted that the node had reachedUpinside a blockingAwaitAssertwith a flat 3 s default, polling every 100 ms.MemberUparrived at 3.462 s, a 462 ms miss, after 3.17 s of log silence. The cluster daemon's startup needs about six dispatches on the default dispatcher, which is the thread pool, and the constructor's blocking wait held one of the two workers a 2-vCPU agent starts with. xUnit v3 never raises the pool floor for this assembly because it disables parallelization. The lease, shard-stop, and persistence paths have no race; the analysis ruled each out.A product item from the analysis is not in this PR:
Shard.ReleaseLeaseIfNeededblocks inPostStopwith a synchronous wait where Pekko fires and forgets.How it was checked
dotnet build src/contrib/cluster/Akka.Cluster.Sharding.Tests -c Release -warnaserrorclean. All fifteen tests and variants run three times: 15 passed each time. A grep for the synchronous TestKit forms over the file returns nothing.BREAKING_CHANGES_V1.6.mduntouched; this is test-only.Second commit
From the adversarial review: the
_ = TestActortouch inInitializeAsyncwas redundant, because the ambient-context attribute already pins the actor cell on the test thread before each fact, and the touch only left a live cell on a returning pool worker. Removed with its comment. The spec passes 15 of 15. #8545 is stacked on this PR and converts this spec's lifecycle methods to overrides once the TestKit gains the virtuals.