Repository navigation
Backport #8510 to v1.5: ClusterClient reconnect deadline via IWithTimers - #8511
Merged
Aaronontheweb merged 1 commit intoSep 5, 2026
Merged
Conversation
…lishing phase, not per message (akkadotnet#8510) ClusterClient.Establishing is used directly as the Receive delegate, so its prologue ran on every message rather than once on state entry. Each message armed another one-shot ReconnectTimeout timer and only the ActorIdentity-success branch cancelled - and it cancelled only the timer armed in that same invocation. Every other message handled while establishing (a Contacts reply, a buffered send, a refresh tick) left a live timer behind. Strays that fire while the client is Active are unhandled and merely noisy. A stray that fires after the client has gone back to establishing hits "case ReconnectTimeout: Context.Stop(Self)" and kills a healthy client mid-reconnect, long before its own deadline is due. Two CI runs of ClusterClientHandoverSpec show it: the client was stopped 622ms and 548ms into a 3s window, and in both cases (stop time - reconnect-timeout) lands on the previous phase's connect moment. Masked in production only because akka.cluster.client.reconnect-timeout defaults to off. Defect dates to the original 2015 port. The deadline measures time spent in the CURRENT establishing phase, so it is now armed once per entry into that phase - PreStart and Reestablish - and cancelled on the transition to Active. Uses IWithTimers rather than a field-held ICancelable, matching the sibling ClusterClientDiscovery actor. A fixed key means starting the timer replaces any live one, so deadlines cannot accumulate; timers are cancelled automatically on stop and restart, so there is no PostStop cleanup to forget; and the scheduler discards messages from a cancelled or replaced timer even when they are already in the mailbox. That last property is why a key beats a cancelable here - Cancel() cannot recall a message that has already been queued, so it leaves the very race this bug is made of. The heartbeat and refresh-contacts timers move to the same discipline so the actor has one timer model. reconnect-timeout = off remains a true no-op: no timer is armed at all. Tests, both failing first against the unfixed actor: - Should_Survive_Stale_Reconnect_Deadline_When_Reestablishing drives the kill deterministically. With a 10s deadline the client sits Active for 7s, then the receptionist is stopped and nothing replaces it. Unfixed, a leftover phase-1 deadline fires 2.997s into the new phase and the watch probe sees Terminated; fixed, the client is still establishing when the 5s window closes, 5s short of the deadline that actually applies. - Should_Not_Leak_ReconnectTimeout_Timers watches the event stream across a full establish -> active -> re-establish cycle. Unfixed, the first abandoned deadline is published at 10.011s and the log shows four unhandled ReconnectTimeouts; fixed, none. ClusterClientHandoverSpec is untouched and serves as the end-to-end regression test. It already passed locally before the fix - the race needs a slow agent - so its soak is no-regression evidence, not proof; the deterministic proof is the two tests above. What the soak does show directly is the leak itself disappearing: across 10 artery rounds under CPU load the client node logged 52 stray ReconnectTimeouts before the fix and 0 after, with the same 2 connects per run either way. Verification: new tests 2/2; Akka.Cluster.Tools.Tests 100/100; Akka.API.Tests 18/18; ClusterClientHandoverSpec 10/10 artery under load and 4/4 classic; -warnaserror clean. API approval files updated for a three-line delta on a sealed class: the IWithTimers interface and its Timers property are added, and the PostStop override is gone because timers now clean themselves up. Recorded in BREAKING_CHANGES_V1.6.md. Backport note (v1.5) Cherry-picked from dev. v1.5's ClusterClient.cs was byte-identical to dev's pre-fix file, so the actor change applied with no conflict; the bug is present on v1.5 exactly as reported. Three deviations from the dev commit, all forced by the branch: - BREAKING_CHANGES_V1.6.md does not exist on v1.5, so that hunk was dropped. The v1.5 PR description carries the notes instead. The sentence "Recorded in BREAKING_CHANGES_V1.6.md" above therefore applies to dev only. - Both ClusterTools approval files conflicted because v1.5 orders the approved members differently (Props before Receive). Resolved to v1.5's own ordering, applying only the semantic delta: -PostStop, +IWithTimers, +Timers, +PreStart. - v1.5 has no artery transport, so the ClusterClientHandoverSpec soak quoted above was re-run classic-only on this branch. (cherry picked from commit 855e8d8)
Aaronontheweb
added
akka-cluster-tools
api-change
akka.net v1.5
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.
Backport of #8510 to the v1.5 maintenance line — a straight
cherry-pick -xof dev's855e8d85f, not a re-implementation. The provenance trailer is in the commit, and the actor file on this branch hashes identical to dev's post-fixClusterClient.cs.Why v1.5 needs it
v1.5's
ClusterClient.cswas byte-identical to dev's pre-fix file: theReconnectTimeouttimer is armed on every message processed while establishing, only the success branch cancels (and only its own timer), and a stray landing after re-entry intoEstablishingstops a healthy client mid-reconnect (issue #8508 has the full mechanism and CI evidence). Masked by default becausereconnect-timeoutdefaults tooff— any v1.5 deployment enabling it is exposed.Fail-first, demonstrated on v1.5's own unfixed code
Both new tests were run against this branch's pre-pick
ClusterClient.cs:ReconnectTimeoutdead letters; measured acrossClusterClientHandoverSpecruns on this branch: 24 strays in 4 runs before, 0 in 8 runs after, with handover behavior unchanged (connects=2every run).Conflict resolution (the only non-picked content)
BREAKING_CHANGES_V1.6.mdhunk dropped — the file does not exist on v1.5.PropsbeforeReceive); resolved by keeping v1.5's ordering and applying only the semantic delta. Validated by the approval tool itself: 18/18 API tests, zero.received.txtfiles generated.API surface delta (identical in both approval variants)
Additive interface + property; the removed
PostStopoverride isprotectedon asealedclass — unreachable, uncallable, and un-overridable by consumers.Validation on v1.5
ClusterClientReconnectTimeoutSpecon unfixed codeAkka.Cluster.Tools.TestsAkka.API.Tests(net10.0)ClusterClientHandoverSpecclassic ×8 under CPU load-warnaserror(Tools + both test projects)Caveat: the net48
Akka.API.Testshost could not run locally (Linux box); the approval content for all target surfaces was validated on net10.0, and CI covers the net48 host.RELEASE_NOTES.mddeliberately untouched.