Repository navigation
De-flake ClusterDeathWatchSpec: resolve first's end actor over the return lane before sending End - #8563
Merged
Aaronontheweb merged 2 commits intoSep 10, 2026
Conversation
Aaronontheweb
added this pull request to stack #8567
September 10, 2026 03:21
Aaronontheweb
force-pushed
the
fix/cluster-deathwatch-spec-resolve-end-actor
branch
from
September 10, 2026 13:14
7737f38 to
e5315a5
Compare
…turn lane before sending End; migrate the file to the async TestKit API On the Windows Artery lane, node fourth's fresh EndSystem sent End to first's /user/end and waited 15s for EndAck. The ack has to ride a brand-new outbound Artery lane from first to EndSystem, and first was observed terminating its ActorSystem ~340ms after replying, before that lane finished materializing, handshaking and connecting - so the ack lost the race and the wait timed out. Fix: before sending End, resolve first's /user/end from EndSystem with ActorSelection.ResolveOne. The ActorIdentity reply can only travel back over the same outbound lane the EndAck will use, so a successful resolve proves that lane is already up. This is safe because first is parked in ExpectMsgAsync<End>() and cannot start tearing down until the End we have not sent yet arrives. Bound the resolve at a dilated 8s and the subsequent EndAck wait at an explicit, undilated 5s - 8 + 5 = 13s, strictly narrower than the 15s single-expect-default the probe used to inherit unbounded. Also switch the EndSystem teardown to the async ShutdownAsync instead of the blocking Shutdown. While in the file, migrated every remaining synchronous TestKit call to its async equivalent: the RemoteWatcher property became an async GetRemoteWatcherAsync helper, TestLatch.Ready() became an AwaitConditionAsync poll on TestLatch.IsOpen with the same 5s budget, and the two remaining RunOn calls became RunOnAsync. No timeout was widened and no assertion was loosened.
…enclosing window; drop a dead null check
Aaronontheweb
force-pushed
the
fix/cluster-deathwatch-spec-resolve-end-actor
branch
from
September 10, 2026 14:54
e5315a5 to
fdf146a
Compare
Aaronontheweb
commented
Sep 10, 2026
| // up. It is safe to resolve before sending End because first is parked in its | ||
| // own ExpectMsgAsync<End>() and cannot start tearing down until the End we have | ||
| // not sent yet arrives. | ||
| await endSystem |
Aaronontheweb
deleted the
fix/cluster-deathwatch-spec-resolve-end-actor
branch
September 10, 2026 15:27
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
…turn lane before sending End (#8563) * De-flake ClusterDeathWatchSpec: resolve first's end actor over the return lane before sending End; migrate the file to the async TestKit API On the Windows Artery lane, node fourth's fresh EndSystem sent End to first's /user/end and waited 15s for EndAck. The ack has to ride a brand-new outbound Artery lane from first to EndSystem, and first was observed terminating its ActorSystem ~340ms after replying, before that lane finished materializing, handshaking and connecting - so the ack lost the race and the wait timed out. Fix: before sending End, resolve first's /user/end from EndSystem with ActorSelection.ResolveOne. The ActorIdentity reply can only travel back over the same outbound lane the EndAck will use, so a successful resolve proves that lane is already up. This is safe because first is parked in ExpectMsgAsync<End>() and cannot start tearing down until the End we have not sent yet arrives. Bound the resolve at a dilated 8s and the subsequent EndAck wait at an explicit, undilated 5s - 8 + 5 = 13s, strictly narrower than the 15s single-expect-default the probe used to inherit unbounded. Also switch the EndSystem teardown to the async ShutdownAsync instead of the blocking Shutdown. While in the file, migrated every remaining synchronous TestKit call to its async equivalent: the RemoteWatcher property became an async GetRemoteWatcherAsync helper, TestLatch.Ready() became an AwaitConditionAsync poll on TestLatch.IsOpen with the same 5s budget, and the two remaining RunOn calls became RunOnAsync. No timeout was widened and no assertion was loosened. * ClusterDeathWatchSpec: state the shutdown bound honestly against the enclosing window; drop a dead null check (cherry picked from commit a00df61)
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
ClusterDeathWatchSpecwaits on the event its last phase depends on instead of a clock.fourth, the freshEndSystemresolvesfirst's end actor over the network before it sendsEnd, with an 8 s dilated bound. The identity reply can only travel back over the outbound lane the acknowledgement will use, so a successful resolve proves that lane is up.firstcannot start tearing down during the resolve, because it is parked waiting for theEndthat has not been sent yet.finallykeeps its own 10 s bound; on the passing path the whole step takes a few seconds, and on the failure path one of these inner bounds reports first, so the enclosing 20 s window is a ceiling rather than the budget.EndSystemshuts down withShutdownAsync, and the file's remaining synchronous TestKit calls move to the async API: the remote-watcher lookup, the latch wait, and twoRunOnblocks. No assertion changes.EndActorandMultiNodeClusterSpecare untouched.Why
The spec failed twice on the Windows Artery lane with the same signature, builds 131108 and 131301: node four timed out after 15 s waiting for
EndAck; the other nodes passed.firstsends the acknowledgement over a brand-new outbound Artery association toEndSystem. Node four's log showsfirstaccepting the control connection at 37.692 and the ordinary one at 37.798, sofirstdid receiveEndand did reply. Then its own wait released, one conductor round trip cleared the last barrier, and the node runner terminated its ActorSystem. Both connections reset at 38.135, about 340 ms after the reply. The acknowledgement had to materialize a stream, connect, clear the handshake stage, and write inside that window.What made
Endarrive so late thatfirst's teardown was imminent: a 3.1 s thread-pool stall on node four between its main system finishing andEndSystemstarting. Five stream continuations whose tasks had already faulted at 34.425 did not run until 37.590. That is the blocking-wait cluster tracked on #8549. Creating the second ActorSystem itself took about 80 ms.The port matches the JVM spec line for line. This is the same change #8475 applied to
UnreachableNodeJoinsAgainSpec, the other user of the end handshake; this spec was outside that PR's scope.What this does and does not close
Build 131351 on the joins-again spec, which already carries this shape, showed the limit: the resolve succeeded over the exact lane the acknowledgement uses, the master received
End, replied, and terminated within about 100 ms, and the acknowledgement still died in the master's teardown because Artery's stream supervisor lives under the user guardian and is stopped before the shutdown flush runs. The acknowledgement is emitted after the message that releases the master to terminate, so no ordering on the sending side can guarantee it leaves. This PR removes the cold-association cost and the unbounded default, which is a real narrowing, and it migrates the file. It does not make the handshake deterministic on Artery. The change that does is #8554, whose second commit hosts the materializer under/systemso the shutdown flush carries the acknowledgement out; until it lands, this spec and the joins-again spec can still lose the acknowledgement on a fast teardown.Product findings from the analysis: the Artery shutdown flush was inert during a graceful terminate and a held handshake element could be lost silently, both fixed in #8554; the flush still logs "finished writing" after every stream faulted, filed as #8562. With #8554 applied, the case where
Endlands just afterfirststarts terminating fails deterministically rather than by timing, so this test fix matters more after it, not less.How it was checked
dotnet build src/core/Akka.Cluster.Tests.MultiNode -c Release -warnaserrorclean. The spec run locally through the multi-node adapter: classic transport three times, 5 of 5 nodes passed each time, 7 to 10 s; Artery three times, 5 of 5 passed, 10 to 11 s. The grep for synchronous TestKit calls on the file returns only a pre-existing commented-out block. Test-only change. No ledger entry.Second commit
From the adversarial review: the comment had claimed the 13 s kept the whole step inside the enclosing 20 s window, which ignored the 10 s shutdown bound in the
finally; it now states the arithmetic honestly. A dead null check afterResolveOne, which throws rather than returning null, is gone. Run once on each transport after the change: 5 of 5 nodes passed.