Repository navigation
De-flake ShardingBufferAdapterSpec: re-send the cold messages with a fresh probe per attempt under a sharding-derived budget - #8573
Merged
Aaronontheweb merged 2 commits intoSep 10, 2026
Conversation
Aaronontheweb
added this pull request to stack #8567
September 10, 2026 12:57
Aaronontheweb
force-pushed
the
fix/sharding-buffer-adapter-spec-resend
branch
from
September 10, 2026 13:14
4a97ff8 to
f2a3ad8
Compare
…fresh probe per attempt under a sharding-derived budget; per-system settings The first message through a fresh region has to survive coordinator allocation and shard start, all ddata majority writes/reads that degrade to "all nodes" on this test's 2-node cluster. If the shard's remember-entities write stalls past its deadline, the Shard restarts and its buffered messages die with it - no dead letter, nothing left to re-deliver. Sharding is at-most-once, so a bare ExpectMsgAsync on the flat akka.test.single-expect-default can time out waiting for a message that no longer exists; waiting longer never helps. FirstMessageThrough re-sends each of the three cold sends through AwaitAssertAsync with a fresh TestProbe per attempt (so a late reply to an abandoned attempt can't satisfy a later one, or leak into the warm phase below), budgeted at ShardStartTimeout + UpdatingStateTimeout (the cold path plus one full Shard restart cycle) with each attempt capped at RetryInterval so the loop actually iterates. The counter snapshots are taken after the loops, so BeGreaterOrEqualTo still holds under retries. The warm-phase sends stay single sends (entities are live; a shard restart here would mean no remember-entities write ever happened, so retrying would hide a real bug) bounded by UpdatingStateTimeout instead of the flat probe default, plus a LastSender check against the original entity ref so a restart between phases can't hide behind the assertions. Also: StartShard was building both regions from Sys's settings instead of the settings of the system whose region is being started (harmless today since sysB is created from Sys.Settings.Config, but wrong); and AfterAll shut Sys down twice (once directly, once via TestKit.Dispose's finally) since _sysA is Sys - removed the redundant call and left a TODO(#8545) for migrating both shutdowns to the async TestKit dispose chain once that lands.
Aaronontheweb
force-pushed
the
fix/sharding-buffer-adapter-spec-resend
branch
from
September 10, 2026 14:54
c0b56ff to
6b44d1d
Compare
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
…fresh probe per attempt under a sharding-derived budget (#8573) * De-flake ShardingBufferAdapterSpec: re-send the cold messages with a fresh probe per attempt under a sharding-derived budget; per-system settings The first message through a fresh region has to survive coordinator allocation and shard start, all ddata majority writes/reads that degrade to "all nodes" on this test's 2-node cluster. If the shard's remember-entities write stalls past its deadline, the Shard restarts and its buffered messages die with it - no dead letter, nothing left to re-deliver. Sharding is at-most-once, so a bare ExpectMsgAsync on the flat akka.test.single-expect-default can time out waiting for a message that no longer exists; waiting longer never helps. FirstMessageThrough re-sends each of the three cold sends through AwaitAssertAsync with a fresh TestProbe per attempt (so a late reply to an abandoned attempt can't satisfy a later one, or leak into the warm phase below), budgeted at ShardStartTimeout + UpdatingStateTimeout (the cold path plus one full Shard restart cycle) with each attempt capped at RetryInterval so the loop actually iterates. The counter snapshots are taken after the loops, so BeGreaterOrEqualTo still holds under retries. The warm-phase sends stay single sends (entities are live; a shard restart here would mean no remember-entities write ever happened, so retrying would hide a real bug) bounded by UpdatingStateTimeout instead of the flat probe default, plus a LastSender check against the original entity ref so a restart between phases can't hide behind the assertions. Also: StartShard was building both regions from Sys's settings instead of the settings of the system whose region is being started (harmless today since sysB is created from Sys.Settings.Config, but wrong); and AfterAll shut Sys down twice (once directly, once via TestKit.Dispose's finally) since _sysA is Sys - removed the redundant call and left a TODO(#8545) for migrating both shutdowns to the async TestKit dispose chain once that lands. * ShardingBufferAdapterSpec: the identity check's comment names the right equality (cherry picked from commit 808de09)
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
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.
Stacked on #8569.
What changes
ShardingBufferAdapterSpecre-sends its first message through each region instead of waiting once on a probe's flat default.AwaitAssertAsyncwith a fresh probe per attempt. The budget is the shard start timeout plus the updating-state timeout from the sharding settings, about 15 s, which is the cold path plus one shard restart cycle; each attempt waits the retry interval, about 2 s, so the loop iterates instead of spending the budget in one wait. A fresh probe per attempt keeps a late reply from one attempt from satisfying the next.StartShardcreated both regions with the test system's settings; each region now uses its own system's.AfterAllno longer shuts the test system down twice, which also flips teardown to B then A, the order in which B's graceful leave can complete while A is still leader. The one blocking shutdown left inAfterAll, and the implicit one the TestKit's dispose runs after it, both wait on TestKit.Xunit: implement the async dispose chain so a derived DisposeAsync no longer leaks the ActorSystem; de-flake StreamRefsSpec #8545, which gives the TestKit an async dispose chain; aTODOmarks the spot.Why
Build 131348, Windows unit tests, on a PR that changed only the serialization generator: the first message through region A was not echoed within the probe's 5 s default. The message had been destroyed mid-wait. The healthy cold path on that same agent took 35 ms, then the whole test process stalled for about 5.8 s, both actor systems missing their heartbeat timers by identical amounts at identical instants. During the stall the shard's remember-entities write timed out, the region restarted the shard, and its buffered messages died with the instance with no dead letter. The write acknowledgement arrived 5 ms after the shard gave up. Waiting longer could not have helped; sharding is at-most-once, so the request has to be re-sent.
Two product findings came out of this. The shard arms its remember-entities write timeout with the read setting, 2 s instead of the documented 5 s, since #6479 in 2023, so the ddata store gets none of the retries it sizes against the 5 s value; that is fixed separately. And a shard restart drops its buffer silently; that is #8572. The stall itself is the thread-pool scheduler pattern on #8549, now with a fourth data point there.
The generator change on the triggering PR is not involved: the only generated types in Akka.Remote are Artery's, this spec runs the classic transport, and nothing in the default serialization bindings is generated.
How it was checked
dotnet build src/contrib/cluster/Akka.Cluster.Sharding.Tests -c Release -warnaserrorclean. The spec run five times: passed each time, about 4 s per run. The grep for synchronous TestKit calls, blocking waits, and sleeps on the file returns only the oneShutdown(_sysB)that #8545 blocks. Test-only change. No ledger entry.Second commit
From the adversarial review: the comment on the identity check said
ActorPath.Equalsincludes the uid. It does not;IActorRefequality compares the uid,ActorPathalone compares address and names. The assertion was right, the reason was wrong, and a reader who trusted it could have swapped in a path comparison and silently lost the check. Comment corrected, nothing else. A related TestKit finding from the same analysis,AwaitAssertreading the clock twice so a stall between the reads turns into a thrownArgumentOutOfRangeException, is recorded on #8565.