Repository navigation
De-flake MNTR specs: stop inner waits from consuming their enclosing Within budget - #8429
Merged
Aaronontheweb merged 8 commits intoJul 29, 2026
Merged
Conversation
TestKit dilates any explicitly-passed duration: RemainingOrDilated(duration) returns Dilated(duration.Value) (TestKitBase.cs:516-521). Four call sites pre-dilated the value themselves, so it was dilated twice. StressSpec.cs:842 is the clearest case — 'ExpectMsgAsync<ActorIdentity>(Dilated(3s))' with akka.test.timefactor = 3 became a 27s wait (3 -> 9 -> 27), matching the 'Timeout 00:00:27 while waiting for a message of type ActorIdentity' failures seen in CI (Azure build 129447). That is self-defeating for a retry helper: ClusterResultAggregatorAsync retries inside a 90s budget, so 27s attempts allow ~3 tries where 9s attempts allow ~10. Pass the raw TimeSpan and let TestKit dilate once. No timeout is widened — the effective waits get shorter and the retry density under load goes up. Sites: StressSpec (ActorIdentity), ClusterShardingQueriesSpec (ClusterShardingStats, CurrentShardRegionState x2).
The config comment claimed Within(max) is not scaled by akka.test.timefactor, and said the raw Within bounds were therefore 'addressed per-site'. That is wrong: TestKitBase.WithinAsync applies Dilated(max) to the bound (TestKitBase_Within.cs:261) and every Within/WithinAsync overload routes through that core, as the XML docs on each overload state. Believing otherwise invites two mistakes this suite has already made: pre-multiplying a Within bound to 'compensate', and pre-Dilating a duration handed to a TestKit expect (which squares the factor - see the ActorIdentity fix in this branch). Replace the note with what the framework actually does.
Aaronontheweb
enabled auto-merge (squash)
July 29, 2026 11:50
… of one shared Within Both phases wrapped everything in a single WithinAsync(20s): 5 barriers, a deliberate 1s ExpectNoMsg, a journal blackhole/restore cycle, 8 ExpectMsg<Value> calls and 2 AwaitAssert loops across three roles. Every wait that omits an explicit duration resolves through RemainingOrDefault, which returns the enclosing Within's remainder clamped at zero - and EnterBarrierAsync does the same (MultiNodeSpec.cs:603, RemainingOr(barrier-timeout)), so barriers silently traded their 30s default for whatever scraps were left. Once the shared budget was consumed - by the deliberate wait, the blackhole round trips, and the coordinator/shard riding out coordinator-failure-backoff/shard-failure-backoff (3s) against the journal's 5s ask-timeout - every later expectation and barrier in the method inherited near-zero time. That is the CI signature exactly: 'Timeout 00:00:00' on second, a small insufficient remainder on first, and a barrier TimeoutException on controller (Azure build 129447). Shrinking the original Within to 4s reproduces that same three-node signature deterministically, confirming the mechanism. Remove both outer Within wrappers so barriers get their real timeout back, give each ordinary expectation its own explicit bound, and make the one recovery-critical check (the first Get after journal-ok) a retrying AwaitAssert bounded by the spec's own backoff constants rather than a single-shot expectation racing an ambient clock. Same fix shape as f13a8fa for this bug class. Verified: both variants pass, including under sustained CPU saturation.
… its retry loop RemoveOneAsync resolved the watchee inside AwaitAssertAsync but sent every Identify to the shared, long-lived IdentifyProbe. When an attempt's 1s expect timed out, that attempt's ActorIdentity stayed queued on the probe and the next attempt consumed it instead of its own reply. A reply resolved before the watchee existed carries a null Subject, so once the probe held one the retry loop could never recover: 'Expected object not to be <null>' at the guard (Azure build 129814, node-1). Use a fresh probe per attempt, matching ClusterResultAggregatorAsync, so a stale reply can never be read by a later attempt. Also give that AwaitAssertAsync an explicit duration. With none it resolved RemainingOrDefault - the enclosing phase's leftover budget - so a slow watch could consume the phase and starve the removal work that follows. Establishing a watch on a just-created actor is one round trip; 10s of retries is ample and leaves the phase intact.
Ten tests fired the call that is supposed to trip the breaker as detached work:
_ = breaker.Instance.WithCircuitBreaker(ct => Task.Run(ThrowException, ct));
Assert.True(CheckLatch(breaker.OpenLatch));
The breaker cannot record a failure - and therefore cannot open, half-open or close - until that call
actually runs, so every latch assertion after it was racing thread-pool scheduling rather than
testing breaker behaviour. Under pool starvation on a loaded agent the latch simply never trips
within its window. Observed in CI as 'An asynchronous circuit breaker that is half open must pass
through next call and close on success' failing with Assert.True() Failure.
Await the call instead, intercepting the TestException it is expected to throw. This is already the
idiom elsewhere in this file (the re-open tests use InterceptException the same way), and the file
also carries a WaitForTaskToBeScheduled helper showing the hazard was known and only partly
addressed. Awaiting is strictly stronger: it guarantees the failure is recorded before the assertion
rather than merely that the task started.
Methods that had no other await are converted to async Task.
No timeout was changed. Verified: full CircuitBreaker suite 29/29, four consecutive runs at 2x core
saturation.
Aaronontheweb
force-pushed
the
fix/ci-flake-batch-2
branch
from
July 29, 2026 18:36
911e3c8 to
ab407c3
Compare
CreateResultAggregatorAsync enters 'result-aggregator-created-<Step>' so the aggregator provably exists, then performs a REMOTE Identify inside RunOnAsync - and then returns. RunOnAsync provides no synchronization at all; it is just 'if (IsNode(nodes)) await thunkAsync()'. So 'the aggregator has been resolved' was only ever a per-node fact, never a cluster-wide one. PartitionSeveral is where that gap becomes a failure. The node that owns the aggregator returns from this method and immediately starts blackholing the very nodes that are still mid-Identify, in Direction.Both and never lifted. Their lookup can then never complete, so they burn the entire retry budget and fail with 'Timeout ... while waiting for a message of type Akka.Actor.ActorIdentity' - observed repeatedly on node-9 in CI (builds 129447, 129814, 129905), with the stack running through ClusterResultAggregatorAsync <- CreateResultAggregatorAsync <- PartitionSeveral. Add a trailing barrier so every node has finished resolving before the phase proceeds. This closes the hole for every phase, not only the partition one. Verified: StressSpec passes locally, 10/10 nodes, 2m46s, zero barrier failures.
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
…wn (port of #8543) Hand-port of dev's #8543 substance, not a cherry-pick - #8543 is the last commit of a five-deep stack (#8372, #8427, #8429, #8499) and its auto-merged parts would not compile as-is on v1.5 (BuildConfig's arithmetic and the ClusterResultAggregatorAsync call sites are written against dev's 10-node/1-per-phase config, which v1.5 does not have). What changed, and why each piece is still correct on v1.5: * acceptable-heartbeat-pause raised from 3s to 20s, with the arithmetic comment explaining the phi-accrual crossing-threshold math (1s + 20s + 3*0.1s = 21.3s of detection). v1.5's failure-detector defaults (heartbeat-interval=1s, min-std-deviation=100ms) and split-brain-resolver stable-after (10s) already match dev's, so the same 3s-was-too-tight problem applies here: a churn round abruptly tearing down an ActorSystem can starve the surviving node's own heartbeat sender for longer than a 3s pause tolerates. * akka.test.single-expect-default = 10s and akka.test.timefactor = 3, ported from #8372 (the first commit of the same dev stack), which the earlier port of this commit had missed. Every Within/WithinAsync bound in this file is dilated by timefactor - including RemoveOneAsync's removal budget (TimeSpan.FromSeconds(25) + ConvergenceWithin(3s, NbrUsedRoles - 1), which #8543 never widened) - so raising acceptable-heartbeat-pause to 20s without also porting timefactor left that budget structurally unable to cover the ~34.3s abrupt-removal path this spec's own ChurnMemberRemovalWithin() arithmetic predicts at the node counts the 7-node CI phase sequence reaches. Without timefactor, RemoveOneAsync's ceiling at NbrUsedRoles=3 is a flat 31s; with it, 93s. Confirmed against three consecutive 7-node runs: the abrupt-removal phase measured 32.9-33.4s in each, and the AwaitAssert measured a 30.98s give-up in each, both matching the undilated 31s ceiling to within noise. All three runs pass with timefactor ported. * ChurnMemberRemovalWithin(), ported byte-for-byte from dev (it only calls Cluster.Settings.FailureDetectorConfig/HeartbeatInterval/GossipInterval, all present on v1.5). Computes how long an abruptly-terminated churn member takes to actually leave the ring: detection + stable-after + a leader-gossip margin = 34.3s at this spec's config. * ExerciseJoinRemoveAsync's loopDuration now includes ChurnMemberRemovalWithin() so each round's Within budget covers the full removal path, not just the new join. The abrupt-shutdown behavior itself (ShutdownAsync, no cluster Leave) was already in place from #8499 - this keeps exactly what the phase proves (abrupt loss), it only fixes the budget around it. * Async TestKit migration of the call sites #8543 touches: the two RunOn sends inside the churn Loop become RunOnAsync, and ClusterResultAggregator (a one-shot, non-retried Identify/ExpectMsg lookup) gains a ClusterResultAggregatorAsync sibling (fresh-probe-per-attempt, retried over 30s) ported from dev, since a lone lost reply under this phase's deliberate churn must not be fatal. Repointed CreateResultAggregatorAsync, AwaitClusterResultAsync, and the async ReportResult<T> overload - the three call chains ExerciseJoinRemoveAsync depends on - at the new async lookup. Every remaining call site in the file passes an async lambda with an explicit return statement, so all of them already bound to the async ReportResult<T>(Func<Task<T>>) overload; the sync ClusterResultAggregator() and the sync ReportResult<T>(Func<T>) overload it served had no callers left after that repointing and are removed here. (Not ported: dev's extra "result-aggregator-identified" barrier in CreateResultAggregatorAsync, a related but separate hardening against a PartitionSeveral race - open item below.) * RemoveOneAsync's watchee lookup, hand-ported from dev (#8372/#8429): a fresh CreateTestProbe() per attempt instead of the shared IdentifyProbe (the shared probe kept a timed-out attempt's late ActorIdentity queued, so the next attempt consumed that stale reply, and a reply resolved before the watchee existed carries a null Subject that the retry could never recover from); an explicit identity.Subject.Should().NotBeNull() guard before WatchAsync; WatchAsync bounded with WaitAsync(Dilated(3s)) instead of inheriting the outer Within's RemainingOrDefault, since a hung/slow watch Ask would otherwise burn the whole retry budget in a single attempt; and an explicit AwaitAssertAsync(10s, 1.25s) bound instead of an unbounded retry loop. This is the exact method that was failing in the runs above, so it is ported alongside the budget fix rather than left as a separate follow-up. * StressSpecConfig node-count env override already existed on v1.5 (MNTR_STRESSSPEC_NODECOUNT, default 13). What it lacked was BuildConfig's shrink arithmetic: below the reference count, phase sizes must shrink or Settings' constructor throws. v1.5's reference config is heavier than dev's - every joining phase defaults to 2 nodes here, not 1 - so reaching the same practical floor of 7 requires shrinking both the joining side (halve every 2 back to 1) and the leaving/shutdown side (drop the "-large" one-by-one phases, halve the simultaneous counts), the same technique dev's config uses on one more group of phases. Derived and documented in StressSpecConfigSpec.cs (ported alongside, adapted from dev's 10-node version to v1.5's 13-node/2-per-phase defaults): 7 is confirmed the practical floor (6 throws because the joining phases alone need 7 regardless of how far leaving/shutdown shrinks). Also fixed while building this: MultiNodeTestRunner.cs's Process.Kill(bool) call from the earlier #8515 hand-merge doesn't compile against netstandard2.0 - see the preceding commit. Verified: dotnet build src/core/Akka.Cluster.Tests.MultiNode -warnaserror clean; StressSpecConfigSpec 9/9; StressSpec run three times at MNTR_STRESSSPEC_NODECOUNT=7, all three passed (see the PR body for full timings). Open items for the maintainer: - dev's extra CreateResultAggregatorAsync barrier (guards a PartitionSeveral aggregator- identification race) was not ported; the retrying ClusterResultAggregatorAsync lookup narrows that race but does not close it the way the extra barrier does.
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
…wn (port of #8543) Hand-port of dev's #8543 substance, not a cherry-pick - #8543 is the last commit of a five-deep stack (#8372, #8427, #8429, #8499) and its auto-merged parts would not compile as-is on v1.5 (BuildConfig's arithmetic and the ClusterResultAggregatorAsync call sites are written against dev's 10-node/1-per-phase config, which v1.5 does not have). What changed, and why each piece is still correct on v1.5: * acceptable-heartbeat-pause raised from 3s to 20s, with the arithmetic comment explaining the phi-accrual crossing-threshold math (1s + 20s + 3*0.1s = 21.3s of detection). v1.5's failure-detector defaults (heartbeat-interval=1s, min-std-deviation=100ms) and split-brain-resolver stable-after (10s) already match dev's, so the same 3s-was-too-tight problem applies here: a churn round abruptly tearing down an ActorSystem can starve the surviving node's own heartbeat sender for longer than a 3s pause tolerates. * akka.test.single-expect-default = 10s and akka.test.timefactor = 3, ported from #8372 (the first commit of the same dev stack), which the earlier port of this commit had missed. Every Within/WithinAsync bound in this file is dilated by timefactor - including RemoveOneAsync's removal budget (TimeSpan.FromSeconds(25) + ConvergenceWithin(3s, NbrUsedRoles - 1), which #8543 never widened) - so raising acceptable-heartbeat-pause to 20s without also porting timefactor left that budget structurally unable to cover the ~34.3s abrupt-removal path this spec's own ChurnMemberRemovalWithin() arithmetic predicts at the node counts the 7-node CI phase sequence reaches. Without timefactor, RemoveOneAsync's ceiling at NbrUsedRoles=3 is a flat 31s; with it, 93s. Confirmed against three consecutive 7-node runs: the abrupt-removal phase measured 32.9-33.4s in each, and the AwaitAssert measured a 30.98s give-up in each, both matching the undilated 31s ceiling to within noise. All three runs pass with timefactor ported. * ChurnMemberRemovalWithin(), ported byte-for-byte from dev (it only calls Cluster.Settings.FailureDetectorConfig/HeartbeatInterval/GossipInterval, all present on v1.5). Computes how long an abruptly-terminated churn member takes to actually leave the ring: detection + stable-after + a leader-gossip margin = 34.3s at this spec's config. * ExerciseJoinRemoveAsync's loopDuration now includes ChurnMemberRemovalWithin() so each round's Within budget covers the full removal path, not just the new join. The abrupt-shutdown behavior itself (ShutdownAsync, no cluster Leave) was already in place from #8499 - this keeps exactly what the phase proves (abrupt loss), it only fixes the budget around it. * Async TestKit migration of the call sites #8543 touches: the two RunOn sends inside the churn Loop become RunOnAsync, and ClusterResultAggregator (a one-shot, non-retried Identify/ExpectMsg lookup) gains a ClusterResultAggregatorAsync sibling (fresh-probe-per-attempt, retried over 30s) ported from dev, since a lone lost reply under this phase's deliberate churn must not be fatal. Repointed CreateResultAggregatorAsync, AwaitClusterResultAsync, and the async ReportResult<T> overload - the three call chains ExerciseJoinRemoveAsync depends on - at the new async lookup. Every remaining call site in the file passes an async lambda with an explicit return statement, so all of them already bound to the async ReportResult<T>(Func<Task<T>>) overload; the sync ClusterResultAggregator() and the sync ReportResult<T>(Func<T>) overload it served had no callers left after that repointing and are removed here. (Not ported: dev's extra "result-aggregator-identified" barrier in CreateResultAggregatorAsync, a related but separate hardening against a PartitionSeveral race - open item below.) * RemoveOneAsync's watchee lookup, hand-ported from dev (#8372/#8429): a fresh CreateTestProbe() per attempt instead of the shared IdentifyProbe (the shared probe kept a timed-out attempt's late ActorIdentity queued, so the next attempt consumed that stale reply, and a reply resolved before the watchee existed carries a null Subject that the retry could never recover from); an explicit identity.Subject.Should().NotBeNull() guard before WatchAsync; WatchAsync bounded with WaitAsync(Dilated(3s)) instead of inheriting the outer Within's RemainingOrDefault, since a hung/slow watch Ask would otherwise burn the whole retry budget in a single attempt; and an explicit AwaitAssertAsync(10s, 1.25s) bound instead of an unbounded retry loop. This is the exact method that was failing in the runs above, so it is ported alongside the budget fix rather than left as a separate follow-up. * StressSpecConfig node-count env override already existed on v1.5 (MNTR_STRESSSPEC_NODECOUNT, default 13). What it lacked was BuildConfig's shrink arithmetic: below the reference count, phase sizes must shrink or Settings' constructor throws. v1.5's reference config is heavier than dev's - every joining phase defaults to 2 nodes here, not 1 - so reaching the same practical floor of 7 requires shrinking both the joining side (halve every 2 back to 1) and the leaving/shutdown side (drop the "-large" one-by-one phases, halve the simultaneous counts), the same technique dev's config uses on one more group of phases. Derived and documented in StressSpecConfigSpec.cs (ported alongside, adapted from dev's 10-node version to v1.5's 13-node/2-per-phase defaults): 7 is confirmed the practical floor (6 throws because the joining phases alone need 7 regardless of how far leaving/shutdown shrinks). Also fixed while building this: MultiNodeTestRunner.cs's Process.Kill(bool) call from the earlier #8515 hand-merge doesn't compile against netstandard2.0 - see the preceding commit. Verified: dotnet build src/core/Akka.Cluster.Tests.MultiNode -warnaserror clean; StressSpecConfigSpec 9/9; StressSpec run three times at MNTR_STRESSSPEC_NODECOUNT=7, all three passed (see the PR body for full timings). Open items for the maintainer: - dev's extra CreateResultAggregatorAsync barrier (guards a PartitionSeveral aggregator- identification race) was not ported; the retrying ClusterResultAggregatorAsync lookup narrows that race but does not close it the way the extra barrier does.
Aaronontheweb
added a commit
that referenced
this pull request
Sep 12, 2026
…wn (port of #8543) Hand-port of dev's #8543 substance, not a cherry-pick - #8543 is the last commit of a five-deep stack (#8372, #8427, #8429, #8499) and its auto-merged parts would not compile as-is on v1.5 (BuildConfig's arithmetic and the ClusterResultAggregatorAsync call sites are written against dev's 10-node/1-per-phase config, which v1.5 does not have). What changed, and why each piece is still correct on v1.5: * acceptable-heartbeat-pause raised from 3s to 20s, with the arithmetic comment explaining the phi-accrual crossing-threshold math (1s + 20s + 3*0.1s = 21.3s of detection). v1.5's failure-detector defaults (heartbeat-interval=1s, min-std-deviation=100ms) and split-brain-resolver stable-after (10s) already match dev's, so the same 3s-was-too-tight problem applies here: a churn round abruptly tearing down an ActorSystem can starve the surviving node's own heartbeat sender for longer than a 3s pause tolerates. * akka.test.single-expect-default = 10s and akka.test.timefactor = 3, ported from #8372 (the first commit of the same dev stack), which the earlier port of this commit had missed. Every Within/WithinAsync bound in this file is dilated by timefactor - including RemoveOneAsync's removal budget (TimeSpan.FromSeconds(25) + ConvergenceWithin(3s, NbrUsedRoles - 1), which #8543 never widened) - so raising acceptable-heartbeat-pause to 20s without also porting timefactor left that budget structurally unable to cover the ~34.3s abrupt-removal path this spec's own ChurnMemberRemovalWithin() arithmetic predicts at the node counts the 7-node CI phase sequence reaches. Without timefactor, RemoveOneAsync's ceiling at NbrUsedRoles=3 is a flat 31s; with it, 93s. Confirmed against three consecutive 7-node runs: the abrupt-removal phase measured 32.9-33.4s in each, and the AwaitAssert measured a 30.98s give-up in each, both matching the undilated 31s ceiling to within noise. All three runs pass with timefactor ported. * ChurnMemberRemovalWithin(), ported byte-for-byte from dev (it only calls Cluster.Settings.FailureDetectorConfig/HeartbeatInterval/GossipInterval, all present on v1.5). Computes how long an abruptly-terminated churn member takes to actually leave the ring: detection + stable-after + a leader-gossip margin = 34.3s at this spec's config. * ExerciseJoinRemoveAsync's loopDuration now includes ChurnMemberRemovalWithin() so each round's Within budget covers the full removal path, not just the new join. The abrupt-shutdown behavior itself (ShutdownAsync, no cluster Leave) was already in place from #8499 - this keeps exactly what the phase proves (abrupt loss), it only fixes the budget around it. * Async TestKit migration of the call sites #8543 touches: the two RunOn sends inside the churn Loop become RunOnAsync, and ClusterResultAggregator (a one-shot, non-retried Identify/ExpectMsg lookup) gains a ClusterResultAggregatorAsync sibling (fresh-probe-per-attempt, retried over 30s) ported from dev, since a lone lost reply under this phase's deliberate churn must not be fatal. Repointed CreateResultAggregatorAsync, AwaitClusterResultAsync, and the async ReportResult<T> overload - the three call chains ExerciseJoinRemoveAsync depends on - at the new async lookup. Every remaining call site in the file passes an async lambda with an explicit return statement, so all of them already bound to the async ReportResult<T>(Func<Task<T>>) overload; the sync ClusterResultAggregator() and the sync ReportResult<T>(Func<T>) overload it served had no callers left after that repointing and are removed here. (Not ported: dev's extra "result-aggregator-identified" barrier in CreateResultAggregatorAsync, a related but separate hardening against a PartitionSeveral race - open item below.) * RemoveOneAsync's watchee lookup, hand-ported from dev (#8372/#8429): a fresh CreateTestProbe() per attempt instead of the shared IdentifyProbe (the shared probe kept a timed-out attempt's late ActorIdentity queued, so the next attempt consumed that stale reply, and a reply resolved before the watchee existed carries a null Subject that the retry could never recover from); an explicit identity.Subject.Should().NotBeNull() guard before WatchAsync; WatchAsync bounded with WaitAsync(Dilated(3s)) instead of inheriting the outer Within's RemainingOrDefault, since a hung/slow watch Ask would otherwise burn the whole retry budget in a single attempt; and an explicit AwaitAssertAsync(10s, 1.25s) bound instead of an unbounded retry loop. This is the exact method that was failing in the runs above, so it is ported alongside the budget fix rather than left as a separate follow-up. * StressSpecConfig node-count env override already existed on v1.5 (MNTR_STRESSSPEC_NODECOUNT, default 13). What it lacked was BuildConfig's shrink arithmetic: below the reference count, phase sizes must shrink or Settings' constructor throws. v1.5's reference config is heavier than dev's - every joining phase defaults to 2 nodes here, not 1 - so reaching the same practical floor of 7 requires shrinking both the joining side (halve every 2 back to 1) and the leaving/shutdown side (drop the "-large" one-by-one phases, halve the simultaneous counts), the same technique dev's config uses on one more group of phases. Derived and documented in StressSpecConfigSpec.cs (ported alongside, adapted from dev's 10-node version to v1.5's 13-node/2-per-phase defaults): 7 is confirmed the practical floor (6 throws because the joining phases alone need 7 regardless of how far leaving/shutdown shrinks). Also fixed while building this: MultiNodeTestRunner.cs's Process.Kill(bool) call from the earlier #8515 hand-merge doesn't compile against netstandard2.0 - see the preceding commit. Verified: dotnet build src/core/Akka.Cluster.Tests.MultiNode -warnaserror clean; StressSpecConfigSpec 9/9; StressSpec run three times at MNTR_STRESSSPEC_NODECOUNT=7, all three passed (see the PR body for full timings). Open items for the maintainer: - dev's extra CreateResultAggregatorAsync barrier (guards a PartitionSeveral aggregator- identification race) was not ported; the retrying ClusterResultAggregatorAsync lookup narrows that race but does not close it the way the extra barrier does.
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.
Second batch of deterministic MNTR de-flakes. All four commits share one root cause family: inner waits are allowed to consume the budget of the block that was supposed to bracket them, so the work that follows inherits little or no time and fails instantly. No timeout was widened to fix any of these.
The mechanism
AwaitAssertAsyncresolves its bound asvar max = RemainingOrDilated(duration)— it does not clamp to the enclosingWithin.ExpectMsgAsync/EnterBarrierAsyncwith no explicit duration resolve viaRemainingOrDefault, i.e. the enclosingWithin's remainder, clamped at zero. So a phase written asWithin(30s)(dilated to 90s attimefactor = 3) can have a single inner operation legally claim all 90s, after which every later expectation and barrier in that phase gets ≈0.That produces both CI signatures we chased:
Assert.Equal() Expected: 5, Actual: 4(a convergence wait given no time to converge in) andTimeout 00:00:00(an expectation given a zero budget).Commits
1. Fix double-dilated timeouts — four call sites passed
Dilated(x)into TestKit methods that dilate again (RemainingOrDilated→Dilated), squaring the factor.StressSpec.cs:842turned a 3s wait into 27s, matching theTimeout 00:00:27 while waiting for ActorIdentityfailures in Azure build 129447. Waits get shorter and retries ~3x denser. Sites: StressSpec, ClusterShardingQueriesSpec (x3).2. Correct the timefactor note in StressSpec — the config comment claimed
Within(max)is not scaled byakka.test.timefactorand that bounds were hand-sized to compensate. That is false:TestKitBase.WithinAsyncappliesDilated(max)and every overload routes through it. That false premise is what invited the pre-multiplied bounds and pre-dilated durations fixed in (1).3. StressSpec: resolve the result aggregator once per step — each phase looked the same aggregator up to three times (
CreateResultAggregatorAsync,ReportResult,AwaitClusterResultAsync), each a retrying remoteIdentifybounded atAwaitAssertAsync(..., 30s)→ 90s, i.e. the whole phase. Whichever retried consumed the phase and starved theAwaitMembersUpAsync(size, timeout: RemainingOrDefault)that followed. The aggregator is one actor perStep, created before theresult-aggregator-created-<Step>barrier, so it provably exists after that barrier — resolve once and reuse, and on the creating node keep the localActorOfreference so it never resolves its own actor remotely. One lookup per node per phase instead of three.4. De-flake ClusterShardingFailureSpec — both phases wrapped 5 barriers, a deliberate 1s
ExpectNoMsg, a journal blackhole/restore cycle, 8ExpectMsg<Value>calls and 2AwaitAssertloops in a singleWithinAsync(20s).EnterBarrierAsyncalso derives its timeout from the ambient remainder (MultiNodeSpec.cs:603), so barriers silently traded their 30s default for scraps. Remove the wrappers so barriers get their real bound back, give each ordinary expectation an explicit bound, and make the one recovery-critical check a retryingAwaitAssertbounded by the spec's owncoordinator-failure-backoff/shard-failure-backoff/journal-timeout constants. Same fix shape as f13a8fa for this bug class. Shrinking the originalWithinto 4s reproduces the exact CI three-node signature, confirming the mechanism rather than correlating with it.Verification
Test-only changes. A related framework bug found during this work (
TestKitBase_Expect.cs:207double-dilates) is filed separately since it is shipped TestKit code.