Repository navigation
De-flake RemoteNodeRestartDeathWatchSpec: probe with Identify before the destructive kill, and let the subject terminate on a sliding timer - #8557
Merged
Aaronontheweb merged 2 commits intoSep 10, 2026
Conversation
…the destructive kill, and let the subject terminate on a sliding timer
The old retry loop used the kill ("shutdown") as its own reachability probe, so
one lost shutdown-ack removed the target for every remaining attempt, and
first spent the rest of its window polling a dead address. The ack was lost
because it was parked in Artery's OutboundHandshakeStage: second's association
to first had only an inbound handshake, so the ack, as the first ordinary
outbound send, waited on a HandshakeReq/HandshakeRsp round trip that never got
to start before Context.System.Terminate() tore the system down. The
materializer placement that keeps Artery from flushing queued sends on
graceful shutdown is a separate product-side fix, not part of this change.
Split first's loop into a non-destructive Identify phase (30s bound, which
also warms second's ordinary lane so the ack never needs the handshake) and a
destructive shutdown phase (10s bound). Made Subject retry-safe: it now
replies and re-arms a 5s sliding terminate timer instead of terminating
inline, so a repeated "shutdown" never finds the system already gone.
Aaronontheweb
force-pushed
the
fix/remote-restart-deathwatch-spec-probe-then-kill
branch
from
September 9, 2026 18:19
f9c4ed6 to
83e6b76
Compare
Aaronontheweb
added a commit
that referenced
this pull request
Sep 10, 2026
…estarted system terminates Build 131332 (this PR's own CI run, Linux Artery) showed third's Shutdown actor replying "shutdown-ack" and calling Context.System.Terminate() inline in the same handler; Artery aborted third's outbound streams before the ack could flush, so it never reached first. First's closed-loop kill then retried "shutdown" every 500ms against an already-exited process and its 20s bound timed out. Local runs only passed because the ack happened to win that race on a fast machine. Slide the terminate behind a 2s timer instead of calling it inline - same shape as the Subject actor in RemoteNodeRestartDeathWatchSpec (PR #8557) - so a live system, and the warmed-up lane under it, always outlasts whichever "shutdown" attempt first's loop last sends. Cancel the timer in PostStop.
Aaronontheweb
added a commit
that referenced
this pull request
Sep 10, 2026
… contact, and the survivor waits on it (#8552) * De-flake DistributedPubSubRestartSpec: the restarted node makes first contact, and the survivor waits on it first's association to third's old incarnation carries no signal that a same-address restart even started; the survivor's association only learns the new incarnation exists from a frame the restarted node itself sends. The old test started its 45s clock at Shutdown()'s return, before any of third's restart cost had begun, and none of the ten prior fixes changed who makes first contact after the restart - they all kept first polling a stale association instead. Now third's Shutdown actor pings a forwarder on first the moment it exists (PreStart), so first waits on that ping (dilated 60s) before running a short 20s closed-loop kill over ActorSelection.Tell (only an ActorSelectionMessage pierces a quarantined association). The ping's own inbound handshake is what heals first's association as a side effect. barrier-timeout goes to 180s to fit the new worst case with headroom, and a bound-address assertion on third settles, on the next Linux failure, whether the fresh listener came up on the pinned port. * DistributedPubSubRestartSpec: repeat the ready ping until acknowledged, use the conductor's async shutdown, keep the port assertion inside the cleanup Addresses review findings on PR #8552: - Shutdown.PreStart no longer sends the ready ping once; it resends every 500ms on an undilated self-scheduled timer until first's ReadyPingForwarder acks it back to the sender, cancelling the timer on ack (and defensively in PostStop). first still waits once for the first ping it sees and ignores later resends. This removes the dependency on any Artery handshake-stage fix - a lost ping is simply retried instead of failing the 60s wait. - Replace TestConductor.Shutdown(...) with the ShutdownAsync(RoleName, ...) overload, keeping the same 30s bound. - Move the restarted-address port assertion inside the try/finally that owns newSystem, so a failed assertion still terminates it. - State the measured worst case behind the 60s ready-ping wait and re-derive newSystem.WhenTerminated's bound (120s -> 155s) against the current 110s worst-case pipeline, restoring its original 45s of slack. * DistributedPubSubRestartSpec: let the shutdown ack leave before the restarted system terminates Build 131332 (this PR's own CI run, Linux Artery) showed third's Shutdown actor replying "shutdown-ack" and calling Context.System.Terminate() inline in the same handler; Artery aborted third's outbound streams before the ack could flush, so it never reached first. First's closed-loop kill then retried "shutdown" every 500ms against an already-exited process and its 20s bound timed out. Local runs only passed because the ack happened to win that race on a fast machine. Slide the terminate behind a 2s timer instead of calling it inline - same shape as the Subject actor in RemoteNodeRestartDeathWatchSpec (PR #8557) - so a live system, and the warmed-up lane under it, always outlasts whichever "shutdown" attempt first's loop last sends. Cancel the timer in PostStop. * DistributedPubSubRestartSpec: terminate timer outlives the retry cycle; restore the 120 s termination wait; last sync call migrated
Aaronontheweb
commented
Sep 10, 2026
Aaronontheweb
left a comment
Aaronontheweb
left a comment
Member
Author
There was a problem hiding this comment.
LGTM - prevents a shutdown race on the node that is being terminated. We send the "ACK we're terminating" message back and schedule the ActorSystem termination to begin ~5 in the future.
Aaronontheweb
deleted the
fix/remote-restart-deathwatch-spec-probe-then-kill
branch
September 10, 2026 17:39
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
… contact, and the survivor waits on it (#8552) * De-flake DistributedPubSubRestartSpec: the restarted node makes first contact, and the survivor waits on it first's association to third's old incarnation carries no signal that a same-address restart even started; the survivor's association only learns the new incarnation exists from a frame the restarted node itself sends. The old test started its 45s clock at Shutdown()'s return, before any of third's restart cost had begun, and none of the ten prior fixes changed who makes first contact after the restart - they all kept first polling a stale association instead. Now third's Shutdown actor pings a forwarder on first the moment it exists (PreStart), so first waits on that ping (dilated 60s) before running a short 20s closed-loop kill over ActorSelection.Tell (only an ActorSelectionMessage pierces a quarantined association). The ping's own inbound handshake is what heals first's association as a side effect. barrier-timeout goes to 180s to fit the new worst case with headroom, and a bound-address assertion on third settles, on the next Linux failure, whether the fresh listener came up on the pinned port. * DistributedPubSubRestartSpec: repeat the ready ping until acknowledged, use the conductor's async shutdown, keep the port assertion inside the cleanup Addresses review findings on PR #8552: - Shutdown.PreStart no longer sends the ready ping once; it resends every 500ms on an undilated self-scheduled timer until first's ReadyPingForwarder acks it back to the sender, cancelling the timer on ack (and defensively in PostStop). first still waits once for the first ping it sees and ignores later resends. This removes the dependency on any Artery handshake-stage fix - a lost ping is simply retried instead of failing the 60s wait. - Replace TestConductor.Shutdown(...) with the ShutdownAsync(RoleName, ...) overload, keeping the same 30s bound. - Move the restarted-address port assertion inside the try/finally that owns newSystem, so a failed assertion still terminates it. - State the measured worst case behind the 60s ready-ping wait and re-derive newSystem.WhenTerminated's bound (120s -> 155s) against the current 110s worst-case pipeline, restoring its original 45s of slack. * DistributedPubSubRestartSpec: let the shutdown ack leave before the restarted system terminates Build 131332 (this PR's own CI run, Linux Artery) showed third's Shutdown actor replying "shutdown-ack" and calling Context.System.Terminate() inline in the same handler; Artery aborted third's outbound streams before the ack could flush, so it never reached first. First's closed-loop kill then retried "shutdown" every 500ms against an already-exited process and its 20s bound timed out. Local runs only passed because the ack happened to win that race on a fast machine. Slide the terminate behind a 2s timer instead of calling it inline - same shape as the Subject actor in RemoteNodeRestartDeathWatchSpec (PR #8557) - so a live system, and the warmed-up lane under it, always outlasts whichever "shutdown" attempt first's loop last sends. Cancel the timer in PostStop. * DistributedPubSubRestartSpec: terminate timer outlives the retry cycle; restore the 120 s termination wait; last sync call migrated (cherry picked from commit 2cc9d1e)
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
1 participant
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
RemoteNodeRestartDeathWatchSpecno longer uses its kill command as its own retry probe.firstnow probes the restarted node withIdentifyuntil it answers, inside a dilated 30 s bound, and only then sends the shutdown command and expects the acknowledgement inside a dilated 10 s bound. The probe phase also warms the restarted node's ordinary outbound lane, so the later acknowledgement does not wait on a handshake.Subjectreplies to a shutdown command and re-arms a 5 s sliding terminate timer instead of terminating inline, so a repeated command always finds the system alive and the acknowledgement always has time to leave.ExpectTerminatedAsyncthe spec is named for is unchanged.No thread pool or dispatcher setting changes.
Why
Build 131195 (Linux Artery). The
Terminatedarrived fine. Nodefirstfailed only the outer 30 s block: its first kill attempt reached the restartedSubject, which replied and terminated its own system in the same handler, so the reply never left and the remaining retries had no process to reach, 110 refused connects. The loss was effectively deterministic. The restarted node's association tofirsthad only an inbound handshake, so the outbound lane was still gating on a handshake round trip, and the acknowledgement was parked in the handshake stage's held element when the system went down. That stage discarded held elements on stop, which is fixed in #8554, and it is why the build carried no evidence.Underneath is a product fault fixed in a separate PR: Artery materializes its streams on a materializer hosted under the user guardian, which the system stops before the remoting terminator runs, so every Artery stream aborts before the shutdown flush and
flush-wait-on-shutdownhas been dead config. Both Akka JVM and Pekko disable this spec under Artery with a FIXME. This change lets it run.How it was checked
dotnet build src/core/Akka.Remote.Tests.MultiNode -c Release -warnaserrorclean. The spec run locally through the multi-node adapter: classic transport, 2 of 2 nodes passed in 12 s; Artery, the lane that failed on CI, 2 of 2 passed twice, 9 s and 8 s.Second commit
From the adversarial review:
Subjectnow cancels its sliding terminate timer inPostStop, so a stopped subject cannot terminate whatever system owns it later. Run once after the change: 2 of 2 nodes passed in 13 s.