Repository navigation
MNTR: conductor keeps re-registered nodes; make RemoteReDeploymentSpec deterministic - #8485
Merged
Aaronontheweb merged 3 commits intoAug 26, 2026
Conversation
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.
The spec checked that `second` came back after its ActorSystem was replaced, but nothing checked that it came back on the same address, and the check that the restarted node could reach `first` again sat inside a nested `within` block. A `within` block whose body outruns the budget by more than 200ms returns without the failure the body was carrying, so a genuine reachability failure vanished and the spec walked into `ready-again` on a dead association. The run then failed as a barrier timeout on `first` and a 60 second barrier ask timeout on `second`, neither of which named the real problem. A CI run shows exactly that: `second` logged "AwaitAssert failed, timeout [00:00:14.9967247] is over after [5] attempts and [00:00:15.4089870] elapsed time" and entered the barrier anyway. - Both nodes now check the replacement system against the address it replaces: `first` against the address the conductor reports, `second` against its own provider address. - The reachability retry loop takes its bound directly, so a failure surfaces where it happens. 15 seconds nests inside the 30 second barrier budget `first` spends at `ready-again`. - `first` awaits a single address query instead of polling it. The conductor parks a query for an unregistered role and answers it the moment that role registers, so one await is the handshake that the node is back; polling only queued further queries, and the conductor client drops a query that arrives while another is outstanding. - Uses StartNewSystemAsync instead of blocking on the sync wrapper. No timeout was raised, and no sleep or retry was added to paper over timing. The delay that separates the Fast, Medium and Slow variants is untouched - that delay is the scenario.
Aaronontheweb
commented
Aug 26, 2026
Aaronontheweb
left a comment
Aaronontheweb
left a comment
Member
Author
There was a problem hiding this comment.
LGTM - probably also needs to be backported to v1.5
Aaronontheweb
enabled auto-merge (squash)
August 26, 2026 08:59
Aaronontheweb
added a commit
that referenced
this pull request
Aug 26, 2026
Barrier failures now actually fail the barrier: three Player.cs sites replied with the FSM-inherited Failure type, so the barrier ask completed successfully and the failure was silently swallowed - the asking node walked on unsynchronized and the breakage surfaced elsewhere as an unrelated-looking flake. All three now reply Status.Failure, plus the discarded-ask cleanup and the runner mislabeling fix. Conductor keeps re-registered nodes: a restarting node's stale ClientDisconnected was matched by role name and evicted the fresh registration, then the barrier coordinator dropped the evicted node's arrivals with no reply, hanging it for the full ask timeout. Disconnects are now matched against the registered FSM identity, unregistered arrivals get an explicit BarrierResult(false), and ReleaseAll detaches the shared event-loop groups. Validated locally with revert-proven tests and three consecutive green ReDeployment MNTR runs. With barrier failures now honest, latent v1.5 spec failures may surface attributed to the node that actually broke - the intended effect. Dev's node-hang kill backstop was deliberately not taken: it needs Process.Kill(entireProcessTree:), unavailable on netstandard2.0; needs a netstandard-safe follow-up.
This was referenced Aug 27, 2026
Closed
This was referenced Aug 27, 2026
This was referenced Aug 30, 2026
Closed
This was referenced Sep 6, 2026
This was referenced Sep 20, 2026
This was referenced Oct 4, 2026
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.
Two commits, found by evidence from build 130782, where RemoteReDeploymentSlowMultiNetSpec failed identically on two transports at once.
Conductor fix. When a node restarts its ActorSystem mid-spec, the old connection's ServerFSM eventually dies and fires ClientDisconnected - which the controller matched by role name, evicting the freshly re-registered node. The barrier coordinator then silently dropped the evicted node's barrier arrivals: no reply at all, so the node burned its full 60s ask (barrier-timeout + query-timeout) while the other side's barrier expired at 30s. Fixes: disconnects are matched against the registered FSM identity, not the role name; an arrival from an unregistered client in Waiting now gets an explicit BarrierResult(false) instead of silence; ReleaseAll detaches the shared event-loop groups so a second conductor in one process gets a live pool. New tests prove both by revert: without the identity check the re-registered node vanishes from GetNodes, and without the reply the arrival hangs forever.
This affects every spec that restarts a node's ActorSystem, on both transports - the ReDeployment family was just the loudest.
Spec fix. RemoteReDeploymentSpec now asserts what it used to assume: the restarted system's address is snapshotted and compared, the address query uses the conductor's own parked-reply handshake instead of a poll that blocked the test thread, and the re-association probe is bounded so it nests inside the barrier budget rather than relying on the WithinAsync timeout path - which silently swallows the block's failure (tracked separately in #8483; this spec no longer depends on it).
Verified: Akka.Remote.TestKit.Tests green; ReDeployment MNTR 6/6 on classic and artery; full Akka.Remote MNTR suite 58/58 on the pre-rebase branch.