Repository navigation
Wave-3 de-flake: ReplicatorChaosSpec, ClusterShardingLeavingSpec pin, DistributedPubSubRestartSpec baseline - #8509
Merged
Conversation
The spec asserts that entities on surviving nodes keep the same actor incarnation across a graceful leave. Only the legacy threshold strategy has that property. The bounded default may move survivor shards during the leave window, which restarts their entities under new refs - that is the optimizer working as designed, not a regression, and the reference implementation behaves the same way. Two CI failures carried this exact signature: the DData variant in build 130784 and the Persistent variant in build 130956, both an entity ref identity mismatch followed by an after-4 barrier cascade. Pinning rebalance-absolute-limit = 0 selects the legacy strategy, so the identity assertion tests the algorithm that guarantees it. The spec becomes a regression test for that strategy's leave behavior instead of a coin flip against the bounded default. Closes akkadotnet#8481.
…rite burst
`ReplicatorChaosSpec` failed three times across two CI builds, on the Artery
Linux and Artery Windows lanes, with one fingerprint every time:
[Node1:first] Timeout (00:00:03) while expecting 15 messages. Only got 10
[Node2:second] Timeout 00:00:00.0000044 while waiting for a message of type
Akka.DistributedData.GetSuccess
[Node3..5] barrier failed:update-during-split-verified
Root cause. Nothing makes the five replicators ready at the same time. Each node
checks `ReplicaCount(5)` against its own replicator and then walks straight into
the update phase, so `first` can start writing while another replicator has not
yet processed `first`'s MemberUp. A replicator drops a Write whose sender it
does not know:
[.../user/replicator] Ignoring message [Write] from
[.../user/replicator/$a] unknown node [UniqueAddress: ...]
and sends no ack. `WriteAll` cannot recover from that. Under WriteAll every node
is primary, so `SendToSecondary` re-sends to nobody, and a GCounter update
leaves `WriteAggregator._delta` null, so the delta re-send does not run either.
One dropped Write costs the whole update. Build 130934 shows `first` issuing its
first Update at 40.136 and `fifth` receiving its Welcome at 40.977; `fifth`
ignored all five Writes 12ms after that, and all five KeyC updates timed out.
Every node now proves `ReplicaCount(5)` and meets at a new `replicas-ready`
barrier before anything is written.
Second defect, and the reason the failure read as "Only got 10" instead of
naming `UpdateTimeout`. `ReceiveN(15)` sat outside any `Within` and inherited
`akka.test.single-expect-default` - 3s, the same 3s as the `WriteAll` deadline
of the updates it collects. An aggregator answers no earlier than its own
deadline, counted from when the replicator picks the Update up, which is at or
after the `ReceiveN` call. The collect can therefore never see that answer; it
is a dead heat by construction. In build 130934 the aggregators replied at
43.226, roughly 70ms too late, and the ten replies that did arrive were exactly
the WriteLocal ones, which need no aggregator. Every expect that waits on a
`WriteTo`/`WriteAll` reply now gets `WriteReplyTimeout` - the aggregator
deadline plus one single-expect-default of slack, with the arithmetic stated in
a comment on the field.
Third defect. `AssertValue` ran `Within(10s)` around an `AwaitAssert` whose
inner `ExpectMsg<GetSuccess>()` carried no bound, so each attempt could consume
the whole remaining budget and the final attempt got almost none - the logged
`00:00:00.0000044`. That reports a value which never converged as a bogus
zero-budget timeout. `AssertDeleted` and the `ReplicaCount` loop had the same
shape. All three now use a fresh probe and a 1s bound per attempt, the treatment
`ClusterShardingSpec` got in akkadotnet#8500.
Also swept. The `TestConductor.Blackhole`, `PassThrough` and `Exit` calls used
`.Wait(timeout)` and discarded the returned bool, so a conductor round-trip
slower than the 1s cap let the test walk on as though the partition were already
installed. They are awaited now, bounded by the conductor's own 30s
query-timeout, and a failure raises instead of vanishing. The spec is converted
to the async TestKit API throughout, which removes the remaining `.Wait()`
calls.
Verified fail-first. A throwaway build that holds one replicator's membership
events back by 4s reproduces the CI fingerprint on all five nodes exactly, with
the same five ignored Writes; the same injection passes with these changes and
produces zero ignored Writes. Bounding `ReceiveN` on its own converts the
failure into the honest assertion `Expected 'UpdateSuccess' got
'UpdateSuccess','UpdateTimeout'`, and bounding the inner expect makes all five
nodes report `Assert.Equal() Failure: Values differ` where four of five
previously reported the zero-budget timeout.
Test-only change.
…fter gossip goes quiet Build 130927 (artery, Windows) failed on second with "Expected value to be 3L, but found 4L" - one extra Delta arrived between the baseline read and the post-restart read. This is a different signature from the one PR akkadotnet#8498 fixed on this spec, and it is not caused by third's restart at all. Mechanism, confirmed by instrumenting the mediator locally and reading the counter it actually exports: DeltaCount counts Delta MESSAGES received, not registry changes. The mediator's Delta handler increments _deltaCount and only then checks _nodes.Contains(bucket.Owner), so a Delta whose payload is thrown away is still counted. While second is still waiting for third's MemberUp, first pushes third's bucket on every 500ms gossip tick and second discards each push - all counted. Convergence therefore arrives as a burst, and redundant Deltas trail it, because a peer keeps re-sending until second's next outbound gossip tells it that second caught up. CountAsync unblocks on the first push second actually merges, so it returns while the burst is still draining. The old code read the baseline immediately after that. Over 20 local runs the last Delta landed just 104-1010ms before that read, against a 500ms gossip tick - one delayed tick puts a trailing Delta on the far side of the read and the invariance assertion fails through no fault of the restart. So the assertion was right and the baseline was wrong: "no new Deltas during third's restart" only holds if the baseline is taken once gossip has settled. ReadStableDeltaCountAsync now requires two samples 2s apart to agree before accepting a baseline. 2s is 4 ticks of this spec's 500ms gossip-interval - enough for our outbound tick plus the peer's reaction, with slack. Fresh probe per sample, matching CountAsync, so a late reply cannot be misread as the next sample. Verified by fault injection rather than by luck. Delivering exactly one extra Delta to the mediator 1s after the baseline query - precisely the event the CI log implies - reproduces the reported failure verbatim on the old code ("Expected value to be 3L, but found 4L" on second) and passes on the new code, where the gate re-samples and absorbs it. The post-fix margin between the last Delta and the baseline read is 3s instead of ~150ms. Soak: artery 8/8 under CPU load, classic 4/4. No timeout was raised, no sleep and no backoff added; the only change is when the baseline is sampled.
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
… DistributedPubSubRestartSpec baseline (#8509) * Pin the legacy allocation strategy in ClusterShardingLeavingSpec The spec asserts that entities on surviving nodes keep the same actor incarnation across a graceful leave. Only the legacy threshold strategy has that property. The bounded default may move survivor shards during the leave window, which restarts their entities under new refs - that is the optimizer working as designed, not a regression, and the reference implementation behaves the same way. Two CI failures carried this exact signature: the DData variant in build 130784 and the Persistent variant in build 130956, both an entity ref identity mismatch followed by an after-4 barrier cascade. Pinning rebalance-absolute-limit = 0 selects the legacy strategy, so the identity assertion tests the algorithm that guarantees it. The spec becomes a regression test for that strategy's leave behavior instead of a coin flip against the bounded default. Closes #8481. * De-flake ReplicatorChaosSpec: rendezvous the replicators before the write burst `ReplicatorChaosSpec` failed three times across two CI builds, on the Artery Linux and Artery Windows lanes, with one fingerprint every time: [Node1:first] Timeout (00:00:03) while expecting 15 messages. Only got 10 [Node2:second] Timeout 00:00:00.0000044 while waiting for a message of type Akka.DistributedData.GetSuccess [Node3..5] barrier failed:update-during-split-verified Root cause. Nothing makes the five replicators ready at the same time. Each node checks `ReplicaCount(5)` against its own replicator and then walks straight into the update phase, so `first` can start writing while another replicator has not yet processed `first`'s MemberUp. A replicator drops a Write whose sender it does not know: [.../user/replicator] Ignoring message [Write] from [.../user/replicator/$a] unknown node [UniqueAddress: ...] and sends no ack. `WriteAll` cannot recover from that. Under WriteAll every node is primary, so `SendToSecondary` re-sends to nobody, and a GCounter update leaves `WriteAggregator._delta` null, so the delta re-send does not run either. One dropped Write costs the whole update. Build 130934 shows `first` issuing its first Update at 40.136 and `fifth` receiving its Welcome at 40.977; `fifth` ignored all five Writes 12ms after that, and all five KeyC updates timed out. Every node now proves `ReplicaCount(5)` and meets at a new `replicas-ready` barrier before anything is written. Second defect, and the reason the failure read as "Only got 10" instead of naming `UpdateTimeout`. `ReceiveN(15)` sat outside any `Within` and inherited `akka.test.single-expect-default` - 3s, the same 3s as the `WriteAll` deadline of the updates it collects. An aggregator answers no earlier than its own deadline, counted from when the replicator picks the Update up, which is at or after the `ReceiveN` call. The collect can therefore never see that answer; it is a dead heat by construction. In build 130934 the aggregators replied at 43.226, roughly 70ms too late, and the ten replies that did arrive were exactly the WriteLocal ones, which need no aggregator. Every expect that waits on a `WriteTo`/`WriteAll` reply now gets `WriteReplyTimeout` - the aggregator deadline plus one single-expect-default of slack, with the arithmetic stated in a comment on the field. Third defect. `AssertValue` ran `Within(10s)` around an `AwaitAssert` whose inner `ExpectMsg<GetSuccess>()` carried no bound, so each attempt could consume the whole remaining budget and the final attempt got almost none - the logged `00:00:00.0000044`. That reports a value which never converged as a bogus zero-budget timeout. `AssertDeleted` and the `ReplicaCount` loop had the same shape. All three now use a fresh probe and a 1s bound per attempt, the treatment `ClusterShardingSpec` got in #8500. Also swept. The `TestConductor.Blackhole`, `PassThrough` and `Exit` calls used `.Wait(timeout)` and discarded the returned bool, so a conductor round-trip slower than the 1s cap let the test walk on as though the partition were already installed. They are awaited now, bounded by the conductor's own 30s query-timeout, and a failure raises instead of vanishing. The spec is converted to the async TestKit API throughout, which removes the remaining `.Wait()` calls. Verified fail-first. A throwaway build that holds one replicator's membership events back by 4s reproduces the CI fingerprint on all five nodes exactly, with the same five ignored Writes; the same injection passes with these changes and produces zero ignored Writes. Bounding `ReceiveN` on its own converts the failure into the honest assertion `Expected 'UpdateSuccess' got 'UpdateSuccess','UpdateTimeout'`, and bounding the inner expect makes all five nodes report `Assert.Equal() Failure: Values differ` where four of five previously reported the zero-budget timeout. Test-only change. * De-flake DistributedPubSubRestartSpec: read the DeltaCount baseline after gossip goes quiet Build 130927 (artery, Windows) failed on second with "Expected value to be 3L, but found 4L" - one extra Delta arrived between the baseline read and the post-restart read. This is a different signature from the one PR #8498 fixed on this spec, and it is not caused by third's restart at all. Mechanism, confirmed by instrumenting the mediator locally and reading the counter it actually exports: DeltaCount counts Delta MESSAGES received, not registry changes. The mediator's Delta handler increments _deltaCount and only then checks _nodes.Contains(bucket.Owner), so a Delta whose payload is thrown away is still counted. While second is still waiting for third's MemberUp, first pushes third's bucket on every 500ms gossip tick and second discards each push - all counted. Convergence therefore arrives as a burst, and redundant Deltas trail it, because a peer keeps re-sending until second's next outbound gossip tells it that second caught up. CountAsync unblocks on the first push second actually merges, so it returns while the burst is still draining. The old code read the baseline immediately after that. Over 20 local runs the last Delta landed just 104-1010ms before that read, against a 500ms gossip tick - one delayed tick puts a trailing Delta on the far side of the read and the invariance assertion fails through no fault of the restart. So the assertion was right and the baseline was wrong: "no new Deltas during third's restart" only holds if the baseline is taken once gossip has settled. ReadStableDeltaCountAsync now requires two samples 2s apart to agree before accepting a baseline. 2s is 4 ticks of this spec's 500ms gossip-interval - enough for our outbound tick plus the peer's reaction, with slack. Fresh probe per sample, matching CountAsync, so a late reply cannot be misread as the next sample. Verified by fault injection rather than by luck. Delivering exactly one extra Delta to the mediator 1s after the baseline query - precisely the event the CI log implies - reproduces the reported failure verbatim on the old code ("Expected value to be 3L, but found 4L" on second) and passes on the new code, where the gate re-samples and absorbs it. The post-fix margin between the last Delta and the baseline read is 3s instead of ~150ms. Soak: artery 8/8 under CPU load, classic 4/4. No timeout was raised, no sleep and no backoff added; the only change is when the baseline is sampled. (cherry picked from commit e340146)
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.
Wave-3 de-flake batch: three spec families, test-only, no product code touched. No timeout raised, no sleep, no retry backoff — every change removes a structural race or pins the algorithm an assertion was written against.
Closes #8481.
ReplicatorChaosSpec (3 artery-lane failures across builds 130927/130934, history to #3079)
Three defects, each fail-first-proven:
ReceiveN(15)could never observe a slowWriteAll: its 3s deadline starts at the call; eachWriteAll(3s)aggregator's deadline starts at or after it — the collector always loses the race by construction. Every write-reply expect now uses a bound derived asWriteTimeout + 3s(undilated, since TestKit dilates what it is handed).ReplicaCount(5), but nothing made nodes wait for each other, sofirst'sWriteAllcould reach a peer whose replicator had not yet processed membership — the peer logsIgnoring message [Write] from [unknown node], sends no ack, andWriteAllhas no re-send (with WriteAll every node is primary, soSendToSecondaryre-sends to nobody; GCounter's delta path nulls the aggregator's delta). One ignored Write = guaranteedUpdateTimeout. Areplicas-readybarrier now separates the phases. Paired-injection proof: a throwaway 4s membership hold reproduces the CI fingerprint on the original spec (5 ignored Writes) and passes on the fixed spec (0).Within → AwaitAssert → unbounded ExpectMsgshapes (AssertValue,AssertDeleted, theReplicaCountloop) inherited drained budgets — one CI run loggedTimeout 00:00:00.0000044. All three now use fresh probes with explicit 1s per-attempt bounds, per the De-flake ClusterShardingSpec family: re-send Gets that race shard hand-off #8500 pattern.Also found in the sweep:
TestConductor.Blackhole/PassThrough/Exitcalls used.Wait(1s)and discarded the bool — a conductor round-trip slower than 1s let the spec proceed as if the partition were installed. Now awaited and bounded by the conductor's own query timeout, so failures raise instead of being swallowed.ClusterShardingLeavingSpec family (2 failures: DData in 130784, Persistent in 130956)
The spec asserts entities on surviving nodes keep the same actor incarnation across a graceful leave. Only the legacy threshold strategy has that property; the bounded default (since #8445) may legitimately move survivor shards during the leave window — and the reference implementation's gate, ordering, and spec are all identical to ours, so deferring rebalance on Leaving would be an unjustified divergence (full parity analysis on #8481). The fix pins
rebalance-absolute-limit = 0in the family's base config: the identity assertion now tests the algorithm that guarantees it, as a real regression test for the legacy strategy's leave behavior.DistributedPubSubRestartSpec delta-count baseline (1 failure:
Expected 3L, found 4L, build 130927)Mechanism confirmed before changing anything:
DeltaCountcounts Delta messages received — including ones whose payload is discarded because the sender's bucket owner is not yet a known member. Whilesecondawaits the restarted node'sMemberUp, the peer pushes that node's bucket every 500ms tick and every discarded push still counts. The old baseline read landed mid-burst, so a trailing tick's Delta crossed the assertion window (locally measured margins of 104ms–1s against the 500ms tick). The baseline read now requires two samples 2s apart to agree (4 gossip ticks, derived in-comment) before the invariance assertion arms. Fail-first: injecting one extra Delta 1s after the old baseline read reproduces the verbatim CI signature on the original spec; the fixed spec absorbs it.Deliberately NOT fixed: ClusterClientHandoverSpec
Its two artery-lane failures are a true positive — a
ClusterClientproduct bug (aReconnectTimeouttimer armed per message duringEstablishingand only the last one cancelled; a stray kills the client on re-entry). No honest test-side fix exists; removing the spec'sreconnect-timeout = 3swould delete the only coverage of the buggy path. Filed with full evidence and a fix shape as #8508; the spec stays on the ledger as known-true-positive until that lands.Verification
Independent single-round replications of ReplicatorChaos (artery 5/5 nodes) and DistributedPubSubRestart (artery 3/3 nodes) on the pushed SHAs. All touched projects build at
-warnaserrorwith 0 warnings.