Add integration test for resetIndexer/replayFromLedger racing concurr… - #1411
Open
Fury03 wants to merge 3 commits into
Open
Add integration test for resetIndexer/replayFromLedger racing concurr…#1411Fury03 wants to merge 3 commits into
Fury03 wants to merge 3 commits into
Conversation
…ent poll Adds an integration test that exercises the race condition described in Functional Edge Case #19 (issue LabsCrypt#1293): resetIndexer and replayFromLedger bypass the SorobanEventWorker's batchMutex, so their DB cursor writes can be overwritten by a concurrent poll's stale upsert. The test deliberately fails against current code and will pass once the race is fixed (the poll must re-read the cursor before writing it back, or resetIndexer/replayFromLedger must go through the batchMutex). 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
CI was red on both backend jobs: the two tests asserted the pre-fix race and the logger mock stubbed `requestContext` as a plain vi.fn(), so replayFromLedger threw "requestContext.getStore is not a function" as an unhandled rejection. Since this branch was opened, LabsCrypt#1221 landed the production fix — resetIndexer now writes inside `sorobanEventWorker.runExclusive`, the same mutex that serialises poll/replay batches. Rework the tests to pin that behaviour down: - Drive the exported `sorobanEventWorker` singleton instead of a fresh `new SorobanEventWorker()`. That singleton is the instance resetIndexer and replayFromLedger lock against, so a separate instance shares no mutex with them and the ordering under test would not exist. - Assert the guarantee rather than the bug: the operator action is held until the in-flight batch releases the mutex, the poll's cursor write lands first, and the reset/replay cursor is the one that survives. - Give the logger mock a real AsyncLocalStorage for `requestContext`. Both tests still fail if resetIndexer's `runExclusive` wrapper is removed, so they guard the fix rather than merely passing alongside it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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 #1293
Problem Statement (The Bug)
The
resetIndexerandreplayFromLedgerfunctions are tested in isolation but never interleaved withSorobanEventWorker's own mutex-protected poll cycle. This means the race described in Functional Edge Case #19 is untested: when an operator triggers incident-recovery tooling while a poll is mid-flight, the poll's stale cursor can silently overwrite the reset/replay cursor, rendering the recovery action ineffective.This issue cannot be fixed with a local patch because it requires concurrent execution of two independently-tested code paths — something isolated unit tests structurally cannot cover.
Solution Comparison and Decision
getEventsmock to inject reset/replay mid-flight, assert reset cursor winsThe Change (Code modifications)
Added
backend/tests/integration/reset-replay-race.test.ts— a single new test file containing two integration tests.Core test mechanism:
Test 1 — resetIndexer race: Starts a poll, injects
resetIndexer(50)mid-flight via deferred promise, asserts reset cursor wins. Current code fails: poll overwrites 50 with stale 200.Test 2 — replayFromLedger race: Starts a second poll mid-flight, injects
replayFromLedger(100), asserts replay cursor wins. Current code fails: second poll overwrites 100 with stale 300.Compatibility Note
No
INTERFACE_VERSIONchange. This PR adds only a test file; no production code is modified.Testing
Pre-existing failures in indexer-state.test.ts, soroban-event-worker.test.ts, and eventRace.test.ts are unrelated.
Additional Notes
Scope: Only
backend/tests/integration/reset-replay-race.test.tsis added. No production code is modified.