Repository navigation
fix(api): prove queue entry for both racers in the currency-lock FIFO test (#402) - #425
Merged
Merged
Conversation
… test (#402) WriterFirst_TheChangeWaitsOutTheWriterTransaction_NotJustItsRead flaked intermittently: WaitUntilDoneOrBlockedAsync's second call could return true on its very first poll purely because the writer was already parked, without ever proving the change's own FOR UPDATE reached Postgres's wait queue — confirmed by the failing CI run completing in 73ms, too fast for even one 50ms poll cycle. WaitUntilDoneOrBlockedAsync now takes a minBlockedCount (default 1, preserving every other caller's behavior) so a second competitor can prove it queued up too, not just that the first one is still there. Detecting this required a recursive walk rather than a plain pg_blocking_pids containment check: pg_blocking_pids reports only the nearest queue blocker, so the change (queued behind the writer, not directly behind the fence) reports the writer's pid, never the fence holder's. Verified against the evaporating-lock mutant the file's header comment describes (FOR SHARE autocommitted separately from the insert): the fix still catches the regression once the race window is realistic, confirming detection wasn't weakened.
90 tasks done
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.
Closes #402.
WriterFirst_TheChangeWaitsOutTheWriterTransaction_NotJustItsReadflaked intermittently — it guards the §4.6 currency-lock race (#162), and a false failure there is indistinguishable from a genuine security regression, which is exactly why it can't be left flaky.Root cause
WaitUntilDoneOrBlockedAsync's second call could returntrueon its very first poll purely because the writer was already parked, without ever proving the change's ownFOR UPDATEreached Postgres's wait queue. Confirmed by the failing CI run completing in 73ms — too fast for even one 50ms poll cycle to have observed the change actually registering.Fix
WaitUntilDoneOrBlockedAsyncnow takes aminBlockedCount(default 1, preserving every other caller's behavior). The test passesminBlockedCount: 2so a second competitor must prove it queued up too, not just that the first one is still there.Detecting this required a recursive walk rather than a plain
pg_blocking_pidscontainment check:pg_blocking_pidsreports only the nearest queue blocker, so the change (queued behind the writer, not directly behind the fence) reports the writer's pid, never the fence holder's.CountBlockedBehindAsyncwalks the full blocking chain to find every backend for whichholderPidappears anywhere in it (confirmed empirically against a live queue).Verification
Per the issue's acceptance criterion — a flake fixed by weakening the assertion is worse than the flake — this was checked against the evaporating-lock mutant the file's header comment describes (
FOR SHAREautocommitted separately from the insert): the fix still catches that regression once the race window is realistic, confirming detection wasn't weakened.