Skip to content

Fix RememberEntitiesStarterSpec constant-strategy throttle test - #8361

Merged
Aaronontheweb merged 1 commit into
akkadotnet:devfrom
Aaronontheweb:fix/remember-entities-starter-spec-constant-throttle
Jul 10, 2026
Merged

Aaronontheweb merged 1 commit into
akkadotnet:devfrom
Aaronontheweb:fix/remember-entities-starter-spec-constant-throttle

Conversation

@Aaronontheweb

Copy link
Copy Markdown
Member

Changes

Fixes the deterministic local failure of RememberEntitiesStarter_must_try_start_all_entities_in_a_throttled_way_with_entity_recovery_strategy_constant (skipped in CI via [LocalFact], but failed 100% of the time when run locally on Linux/net10.0).

Root cause (test bug — production throttle is correct)

The test configured retry-interval = 1s (the RememberEntityStarter no-ACK retry timer) alongside entity-recovery-constant-rate-strategy.frequency = 2s (the batch timer). At t≈2s both periodic timers fire in the same scheduler tick:

  1. The batch timer dispatches batch 2 and records those entities in _waitingForAck.
  2. The retry timer fires immediately afterwards — before the test's StartEntityAck replies can reach the actor's mailbox — and resends the just-dispatched batch (OnRetryUnacked resends everything unacked, with no grace period).

The duplicate StartEntity messages then trip the test's ExpectNoMsg(600ms) window (RememberEntitiesStarterSpec.cs:187):

Failed: Expected no messages during 00:00:00.6000000, instead we received <StartEntity(2)> ... after 00:00:00.0000032

The actor's debug log shows the collision directly — Starting batch of [2] and Found [2] remembered entities waiting for StartEntityAck, retrying at the same millisecond.

This is not a recent regression: the production files are unchanged (RememberEntityStarter last touched in #7401), and the test has been broken since it was added in #5857 / CI-disabled in #6012. The net6+ HashedWheelTimerScheduler processes same-tick timers in consistent insertion order, which converted the old flaky failure into a deterministic one. Duplicate StartEntity is idempotent at the region, so the underlying behavior is harmless in production (shipped defaults dispatch batches ~20× faster than the retry interval).

Fix

  • Raise the test's retry-interval to 30s so the no-ACK retry can never fire inside the ~4s recovery window, with a comment documenting the constraint. The test ACKs every entity immediately, so it never needs the retry.
  • Convert the test to async TestKit methods (ExpectMsgAsync / ExpectNoMsgAsync / ExpectTerminatedAsync) per repo guidelines.
  • Update the stale LocalFact skip reason ("suspected bad code underneath" → the code was never at fault; the test stays local-only because its real-time 600ms ExpectNoMsg windows remain CI-jitter-sensitive).

No production code changes; no BREAKING_CHANGES_V1.6.md entry needed.

Verification

  • Spec class: 3 consecutive runs, 4/4 passed each (previously 3/4, constant-strategy test failed every run).
  • Full Akka.Cluster.Sharding.Tests: 192/192 passed on net10.0/Release (previously 191/192).

The constant entity-recovery test set retry-interval = 1s alongside the
2s constant-rate batch frequency, so at t=2s the no-ack retry timer and
the batch timer fired in the same scheduler tick: the retry resent the
just-dispatched, not-yet-acked batch and the duplicate StartEntity
messages tripped the ExpectNoMsg(600ms) window. On the net6+ hashed
wheel timer the tick ordering is consistent, turning the old flakiness
into a deterministic local failure.

Raise retry-interval above the recovery window (30s) so the retry never
fires during the test, convert the test to async TestKit methods, and
update the stale LocalFact skip reason - the production throttle was
never at fault.
@Aaronontheweb
Aaronontheweb merged commit bb031d8 into akkadotnet:dev Jul 10, 2026
11 checks passed
@Aaronontheweb
Aaronontheweb deleted the fix/remember-entities-starter-spec-constant-throttle branch July 10, 2026 22:03
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