Repository navigation
fix: pre-warm thread pool in CircuitBreakerSpecBase to fix cold-start latch flakiness - #8287
Closed
Aaronontheweb wants to merge 1 commit into
Closed
Aaronontheweb wants to merge 1 commit into
Aaronontheweb wants to merge 1 commit into
Conversation
… latch flakiness The async CircuitBreakerSpec tests intermittently failed (e.g. Must_reopen_on_exception_in_call, Must_throw_exceptions_when_called_before_reset_timeout) with "Assert.True() Failure" when a CountdownEvent latch wait timed out. The breaker signals these latches from fire-and-forget callbacks dispatched via Task.Factory.StartNew, and the half-open transition also depends on the HashedWheelTimer reset firing — all thread-pool work. The .NET thread pool injects new workers above the minimum at only ~1 per 500ms, so on a cold/contended CI agent those work items can be starved past the 2s latch timeout. Surfaced most recently by the Microsoft.NET.Test.Sdk 18.x test-host change (akkadotnet#8282). Pre-warm the pool in the spec base (ThreadPool.SetMinThreads(ProcessorCount*2), guarded by Math.Max so an existing minimum is never lowered), matching the existing InMemoryPersistenceSpecConfig.EnsureThreadPoolWarmed() remedy for the identical root cause. Test-only; no public API or timeout changes. Verified: full Akka.Tests.Pattern suite green; the two previously-failing tests pass 25/25 in a tight loop.
Member
Author
|
Pretty gross code fix. |
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.
Problem
Several async
CircuitBreakerSpectests fail intermittently on CI with a bare:Most recently on the
Microsoft.NET.Test.Sdk17.9.0 → 18.7.0 bump (#8282) — e.g.Must_reopen_on_exception_in_call(CircuitBreakerSpec.cs:283, HalfOpen latch) andMust_throw_exceptions_when_called_before_reset_timeout(line 311, Open latch). TheAssert.True(false)is aCountdownEventlatch wait (latch.Wait(2s)) returningfalse.Root cause
CircuitBreakersignals the test latches from its transition listeners, which run fire-and-forget on the thread pool (AtomicState.Enter→NotifyTransitionListeners→Task.Factory.StartNew, deliberately un-awaited so listeners never block state transitions). The half-open transition additionally depends on theHashedWheelTimerreset firing — also pool-driven. Meanwhile the test thread blocks inCountdownEvent.Wait.The .NET thread pool injects new workers above the minimum at only ~1 per 500ms. On a cold/contended CI agent at process start, the fire-and-forget breaker call, the transition-listener signal, and the timer continuation can be starved behind that ramp and miss the fixed 2s latch budget. This is environmental timing, not a product bug — the SDK bump simply changed the test-host's startup footprint enough to expose a pre-existing latent race. (Confirmed:
akka.test.timefactoris1.0on CI, soDilated(...)is a no-op and the latch budget is a hard 2s.)Fix
Pre-warm the thread pool in
CircuitBreakerSpecBaseso those work items get a worker immediately instead of waiting on throttled injection:This is the same remedy already used in the repo for the identical cold-start root cause (
InMemoryPersistenceSpecConfig.EnsureThreadPoolWarmed()).Math.Maxguarantees a previously-raised minimum is never lowered.Test-only change — no public API touched (notably
TestBreakerand itsCountdownEventlatches are unchanged, per extend-only design) and no timeout values changed.Verification
Akka.Tests.Patternsuite green (Total: 71, Failed: 0).Local runs can't reproduce CI's cold-start starvation (the dev-box pool warms instantly), so local results confirm no regression; the fix is grounded in the root-cause analysis and the proven in-repo precedent.
Notes
Second of two flaky-test fixes split out from the #8282 investigation; the multi-node
NodeChurnSpecfix is #8286. The broader observation thatakka.test.timefactoris never raised on CI (making everyDilated(...)a no-op) is left as a separate, optional CI-config change.