Skip to content

Flaky: CurrencyLockRaceTests.WriterFirst_TheChangeWaitsOutTheWriterTransaction_NotJustItsRead fails intermittently in CI #402

Closed

Description

@mforce

What happened

Cluckwork.Api.IntegrationTests.CurrencyLockRaceTests.WriterFirst_TheChangeWaitsOutTheWriterTransaction_NotJustItsRead failed on PR #395 — a one-line, prose-only change to AGENTS.md (1 file changed, 1 insertion(+)).

Failed ... WriterFirst_TheChangeWaitsOutTheWriterTransaction_NotJustItsRead [73 ms]
  Error Message:
   the change must wait out the writer's WHOLE transaction and refuse

Run: https://github.com/mforce/cluckwork/actions/runs/30774249563

Why this is a flake, not a regression

  • The same branch passed a full CI run earlier (30768521988). The only commit between the two is 6db63be, which edits AGENTS.md and nothing else.
  • Every other CI run in the same window is green, including three PRs that change real code.
  • Its sibling CurrencyChange_SerializesBehindAnInFlightMoneyWrite_AndRefuses passed in the same run (76 ms).
  • No source file under tests/Cluckwork.Api.IntegrationTests/CurrencyLockRaceTests.cs has changed since feat(api): bounded retry-on-failure for transient DB connection loss (#269) #350.

A prose diff cannot alter Postgres lock-queue behaviour.

What the assertion means

CurrencyLockRaceTests.cs:186 — Assert.True(result.IsFailure, ...). The currency change succeeded when it should have been refused, which is precisely the symptom the file's own header comment describes for the evaporating-lock mutant. So the false failure is indistinguishable from a genuine security regression, which is the worst property for a flake to have: this test guards the §4.6 currency-lock race (#162), and the natural response to seeing it red is to re-run rather than investigate.

Suspected mechanism

The test fences both parties on an exclusive SELECT ... FOR UPDATE on the account row, waits for each to be blocked, then rolls the fence back and relies on Postgres granting the lock FIFO so the writer commits before the change reads. The header comment states the protocol assertion is deterministic under correct code.

Two things worth checking:

  1. WaitUntilDoneOrBlockedAsync returns true for done or blocked. Gating "the writer heads the queue" on that is an approximation of "the writer has actually entered the lock queue"; if the detection resolves before the lock request is registered, the change can take the queue head instead.
  2. EnableRetryOnFailure (Add transient-DB resilience (EnableRetryOnFailure) on the request path #269) is active for these handlers. A retry reconnects, which would abandon the queue position the test's protocol depends on. The failing run's log carries four RetryLimitExceededException occurrences. This is the same class of hazard Add transient-DB resilience (EnableRetryOnFailure) on the request path #269 documents elsewhere — an automatic replay sitting above stateful machinery that cannot tell a self-race from the real signal.

Suggested fix direction

Prove queue entry rather than infer it — e.g. assert the writer's backend appears in pg_locks as a waiter on the specific row lock before releasing the fence, instead of relying on a "done or blocked" poll. If retry is implicated, the handler region under test may need the same SingleAttemptExecution treatment the four documented regions already have.

Acceptance

  • The test either passes deterministically across repeated runs, or its guarantee is re-expressed in a way that does not depend on lock-queue ordering.
  • Whatever the fix, it must still fail under the evaporating-lock mutant the header describes — a flake fixed by weakening the assertion is worse than the flake.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingepic-1.1

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions