Skip to content

De-flake RemoteDeliverySpec: give the final barrier a budget that covers the loop - #8542

Merged
Aaronontheweb merged 1 commit into
devfrom
fix/remote-delivery-spec-barrier-budget
Sep 11, 2026
Merged

Aaronontheweb merged 1 commit into
devfrom
fix/remote-delivery-spec-barrier-budget

Conversation

@Aaronontheweb

@Aaronontheweb Aaronontheweb commented Sep 9, 2026 •

Copy link
Copy Markdown
Member

What changes

RemoteDeliverySpec wraps its final after-1 barrier in a Within that gives the barrier a 300 s budget. The 500-letter loop, its relay route, its per-letter ExpectMsg bound and its assertions are unchanged, so the spec proves exactly what it proved before. The rest of the diff is the file's migration to the async TestKit API.

Why

second and third do no work of their own in this spec. They start a Postman, pass actors-started, and arrive at after-1 about a second into the run, while first is still sending its 500 letters. BarrierCoordinator arms a barrier's clock on the first arrival and only ever shortens it, so the 30 s default akka.testconductor.barrier-timeout, not the 5 s per-letter wait, is what bounds the whole loop.

On a saturated CI agent the loop needs more than 30 s. The barrier then fails, the relay nodes stop, and first fails its next per-letter wait because the route it was using has gone away. The reported failure is a 5 s delivery timeout, which reads like a dropped message but is not one.

EnterBarrierAsync asks the coordinator for RemainingOr(barrier-timeout), so a Within around the final barrier sets that budget directly.

The number. 300 s over 500 letters permits a 600 ms mean round trip. The slowest mean measured on a loaded two-core agent was about 190 ms, so this is roughly 3x that, and it stays far below the 5 s per-letter deadline, which remains the assertion that reports a real drop. A node that actually fails still aborts the barrier at once, because losing a client fails a barrier already in progress, so the longer budget does not slow down real failures.

Why not the config key. akka.testconductor.barrier-timeout also drives the teardown poll in MultiNodeSpecAfterAll, where AwaitCondition polls at max/10 and the first poll always misses. Raising the key adds barrier-timeout/10 of wall clock to every run. Measured at 30 s, 60 s, 100 s and 300 s, the spec took 5 s, 8 s, 12 s and 32 s. Scoping the budget to the barrier keeps the run at 5 s.

One comment was added to mark akka.remote.dot-netty.tcp.batching.enabled as classic-only; Artery has no batching switch and ignores it.

How it was checked

dotnet build src/core/Akka.Remote.Tests.MultiNode -c Release -warnaserror clean.

The mechanism was reproduced and fixed locally. Adding a 40 s delay to first's block makes the spec fail on dev's shape with barrier failed:after-1 on all three nodes, which is the CI signature. The same 40 s delay passes with the scoped budget in place.

The spec then ran through the multi-node adapter on the committed change: five runs on classic and three on Artery, all passed, 5 to 6 s each, the same as dev. The Artery runs were confirmed to load ArteryRemoting.

Measurements that did not justify a change: the two per-message classic log keys log-sent-messages and log-received-messages do emit 10 lines per letter, 5008 lines over a run, but turning them off, and turning off DebugConfig entirely, made no measurable difference to the loop (1.31 s, 1.38 s and 1.34 s mean over three runs each). They are left on, so a failing run keeps its diagnostics.

The grep for synchronous TestKit calls, blocking waits, and sleeps on the file returns nothing. Test-only change. No ledger entry.

@Aaronontheweb
Aaronontheweb force-pushed the fix/remote-delivery-spec-barrier-budget branch 2 times, most recently from 964770c to a627034 Compare September 10, 2026 19:50
…ers the loop

'second' and 'third' do no work of their own, so they reach the "after-1" barrier
about a second into the run while 'first' is still sending its 500 letters.
BarrierCoordinator arms a barrier's clock on the first arrival and only ever
shortens it, so the 30s default barrier-timeout - not the 5s per-letter wait - is
what bounds the whole loop. On a saturated CI agent the loop needs longer than
that, the barrier fails, the relay nodes stop, and 'first' then fails its next
per-letter wait because the route it was using has gone away.

EnterBarrierAsync asks the coordinator for RemainingOr(barrier-timeout), so a
Within around the final barrier sets that budget. 300s over 500 letters permits a
600ms mean round trip, about 3x the slowest mean measured on such an agent and far
below the 5s per-letter deadline, which remains the assertion that reports a real
drop.

Scoped to the barrier rather than set as akka.testconductor.barrier-timeout,
because that key also drives the teardown poll in MultiNodeSpecAfterAll, where
AwaitCondition polls at max/10 and the first poll always misses. Raising the key
to 300s adds 30s of wall clock to every run of this spec; measured here at 30s,
60s, 100s and 300s, the tax tracks barrier-timeout/10 exactly.

The loop, its route, its per-letter assertion and the letter count are unchanged,
so the spec proves exactly what it proved before. The rest of the diff is the
file's migration to the async TestKit API.
@Aaronontheweb
Aaronontheweb force-pushed the fix/remote-delivery-spec-barrier-budget branch from a627034 to 84d66c8 Compare September 11, 2026 14:57
@Aaronontheweb Aaronontheweb changed the title De-flake RemoteDeliverySpec: size the barrier budget above the spec's own deadlines and pipeline the letters De-flake RemoteDeliverySpec: give the final barrier a budget that covers the loop Sep 11, 2026

@Aaronontheweb Aaronontheweb left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

// under the 5s per-letter deadline, which stays the assertion that reports a real
// drop. A node that fails still aborts the barrier at once, because losing a client
// fails a barrier already in progress.
await WithinAsync(TimeSpan.FromSeconds(300), () => EnterBarrierAsync("after-1"));

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BIg time budget to allow the other remaining node to finish the loop. If this doesn't work, we'll remove in the future and re-assess how to address this problem.

@Aaronontheweb
Aaronontheweb merged commit e62aa19 into dev Sep 11, 2026
15 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/remote-delivery-spec-barrier-budget branch September 11, 2026 17:33
Aaronontheweb added a commit that referenced this pull request Sep 12, 2026
…ers the loop (#8542)

'second' and 'third' do no work of their own, so they reach the "after-1" barrier
about a second into the run while 'first' is still sending its 500 letters.
BarrierCoordinator arms a barrier's clock on the first arrival and only ever
shortens it, so the 30s default barrier-timeout - not the 5s per-letter wait - is
what bounds the whole loop. On a saturated CI agent the loop needs longer than
that, the barrier fails, the relay nodes stop, and 'first' then fails its next
per-letter wait because the route it was using has gone away.

EnterBarrierAsync asks the coordinator for RemainingOr(barrier-timeout), so a
Within around the final barrier sets that budget. 300s over 500 letters permits a
600ms mean round trip, about 3x the slowest mean measured on such an agent and far
below the 5s per-letter deadline, which remains the assertion that reports a real
drop.

Scoped to the barrier rather than set as akka.testconductor.barrier-timeout,
because that key also drives the teardown poll in MultiNodeSpecAfterAll, where
AwaitCondition polls at max/10 and the first poll always misses. Raising the key
to 300s adds 30s of wall clock to every run of this spec; measured here at 30s,
60s, 100s and 300s, the tax tracks barrier-timeout/10 exactly.

The loop, its route, its per-letter assertion and the letter count are unchanged,
so the spec proves exactly what it proved before. The rest of the diff is the
file's migration to the async TestKit API.

(cherry picked from commit e62aa19)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant