Backport MNTR conductor reliability to v1.5 (#8431 barrier honesty + #8485 conductor fixes) - #8488
Merged
Merged
Conversation
ClientFSM replied to a failed barrier with 'new Failure(...)'. Because ClientFSM derives from FSM<,>, that unqualified name binds to the inherited nested FSMBase.Failure - an FSM termination reason - not Akka.Actor.Status.Failure. FutureActorRef's ask-completion switch faults only on ISystemMessage, Status.Failure and Akka.Actor.Failure. FSMBase.Failure matches none of them, so it fell through to 'case T t: TrySetResult(t)' and the ask COMPLETED SUCCESSFULLY. EnterBarrierAsync discarded the result and logged 'passed barrier', so every non-success outcome - timeout, wrong barrier, client lost, duplicate node - was silently swallowed and the node walked on unsynchronized. The damage is misattribution rather than fabricated passes: when one node fails and never reaches a barrier, the remaining nodes should fail cleanly on a broken rendezvous. Instead they continued into the next phase out of sync and produced their own unrelated failures, turning one localized error into a multi-node mess. Use Status.Failure at all three sites in v1.5's Player.cs, matching the two places in the same file that already got it right. Also drop the unused ask result now that a failure propagates as an exception. Healthy barriers are unaffected - a successful BarrierResult still replies with the barrier name. Taken from the dev squash dcd9acb: - src/core/Akka.Remote.TestKit/Player.cs in full: all three `new Failure(...)` -> `new Status.Failure(...)` sites plus the discarded-ask cleanup in EnterAsync. Applied cleanly. - One line of MultiNodeTestRunner.cs: the failure path reported a failed node as "<spec> passed." Corrected to "<spec> failed." Same honesty theme, compiles as-is. Dropped from the dev squash: - The MultiNodeTestRunner hang backstop (DefaultNodeExitTimeout plus the Task.WhenAny/kill block). It applies textually but does not compile on v1.5: dev's Akka.MultiNode.TestAdapter targets net10.0 while v1.5's targets netstandard2.0, which has no Process.Kill(bool) overload. Falling back to the parameterless Kill() would leave the node's child processes orphaned on CI, which is worse than no backstop, so this is left for a separate change. - The ShardedDaemonProcessSpec redesign. Dev-specific test content, separately owned, no bearing on barrier honesty.
A node that calls StartNewSystem reconnects to the conductor under the same role name on a fresh channel. The ServerFSM for the connection it replaced then terminates and reports ClientDisconnected, and the controller removed the role by name - evicting the live registration that had just replaced it. From that point the barrier coordinator held no registration for the node, so it dropped every arrival from it and the next barrier stalled until it timed out. The evicting disconnect and the new registration arrive from independent sources, so which one lands first is pure timing; a slow agent loses the race. Four changes: - ServerFSM names itself as the sender of its ClientDisconnected, and the controller only lets a ServerFSM evict the registration it owns. - BarrierCoordinator answers an arrival from an unregistered client with a failed BarrierResult while waiting, instead of dropping it. Dropping it left that node blocked on an ask nothing would complete, so it reported a 60 second ask timeout long after the barrier had already failed on another node, which hid the real cause. Idle already answered this way. - StartNewSystemAsync pins Artery's canonical host and port as well as the classic ones. A node is restarted so that deployments and associations aimed at it still resolve, and writing only the classic key left Artery free to take a fresh ephemeral port whenever multinode.port is 0. - RemoteConnection.ReleaseAll detaches the event loop groups before shutting them down, so a later CreateConnection builds fresh ones instead of getting a dead pool. It also shuts the server worker pool down, which it leaked. Both new tests fail without the corresponding fix: the controller test finds the node gone from GetNodes, and the barrier test times out waiting for a reply that never comes. Backport of #8485. The cherry-pick applied clean on all six files - no conflicts, byte-identical to the source commit. The Artery bullet above describes work that lives outside this commit and has no v1.5 counterpart: v1.5 ships no Artery transport, so nothing there needed porting. (cherry picked from commit e9036d0)
This was referenced Aug 26, 2026
Aaronontheweb
added a commit
that referenced
this pull request
Aug 27, 2026
Backport of dev commit dcd9acb to v1.5, scoped to the ShardedDaemonProcessSpec rewrite. The Player.cs / MultiNodeTestRunner.cs portions of #8431 were already delivered to v1.5 in the MNTR conductor reliability backport (#8488), so only the spec change is taken here. Rewrite the ShardedDaemonProcess multinode spec to be async and resistant to slow-CI barrier timeouts: - task-returning TestKit methods throughout (no sync-over-async) - single cluster-wide collector probe on 'first' so shards reallocated during cluster settle are reported correctly - fish for a complete distinct-ID set instead of assuming 4 messages - raise testconductor barrier-timeout to 60s for slow agents - fix HOCON brace nesting that dropped keep-alive-interval Adapted to v1.5: AwaitClusterUpAsync(CancellationToken.None, ...) to match the v1.5 TestKit signature. (cherry picked from commit dcd9acb)
This was referenced Aug 27, 2026
Closed
This was referenced Aug 30, 2026
Closed
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.
Backports the multi-node conductor reliability line from dev, so v1.5's own MNTR gate (which every backport PR runs through) reports failures honestly and survives node restarts. Two commits.
Barrier honesty (#8431). Three sites in Player.cs constructed a bare
new Failure(...), which inside an FSM subclass binds to FSMBase.Failure rather than Status.Failure - so the barrier ask completed successfully and the failure was silently swallowed. The asking node walked on unsynchronized, and the eventual breakage surfaced on some other node, at some other barrier, as an unrelated-looking flake. All three now reply Status.Failure, plus the discarded-ask cleanup, plus a one-line runner fix where a failed node's output was labeled "passed." The binding bug was demonstrated end-to-end against v1.5's own binaries during validation: the unqualified reply is FSMBase.Failure, the ask completes successfully with it, and the same reply as Status.Failure faults the ask with the real barrier error.Deliberately not taken from the dev squash: the runner's node-hang kill backstop (dev's implementation needs Process.Kill(entireProcessTree:), which netstandard2.0 - v1.5's TestAdapter target - does not have; a parameterless Kill would orphan child node processes, which is worse than no backstop; needs a netstandard-safe follow-up), and dev-specific spec content.
Conductor fixes (#8485). When a node restarts its ActorSystem mid-spec, the old connection's ServerFSM death fired ClientDisconnected, which the controller matched by role name - evicting the freshly re-registered node - and the barrier coordinator then dropped the evicted node's arrivals with no reply at all, hanging it for its full ask timeout while the other nodes' barrier expired. Disconnects are now matched against the registered FSM identity, unregistered arrivals get an explicit BarrierResult(false), and ReleaseAll detaches the shared event-loop groups so a second conductor in one process gets a live pool. Cherry-picked clean, zero conflicts - v1.5's conductor had not drifted at any touched site.
Validation on v1.5: Akka.Remote.TestKit and the TestAdapter build at 0 warnings; Akka.Remote.TestKit.Tests all green including both ported tests; revert-proofs performed (reverting the identity check fails the re-registration test; reverting the coordinator reply hangs the arrival test on exactly the old symptom); ReDeployment MNTR specs 6/6, three consecutive runs.
Expectation for this branch's MNTR lane: with barrier failures now honest, latent spec failures the old code silently swallowed may start surfacing - attributed to the node that actually broke instead of the nodes downstream of it. That is the intended effect, and the same effect on dev is what made the rest of the de-flake campaign possible.