Repository navigation
test(rescue): serialise the bulk rescue tests and assert rescued claims were stale (0.6) - #706
Conversation
…heartbeat ages The 50,000-claim test requires that no claim of the live doomed runtime is rescued before it is killed. That holds only while the runtime heartbeats every claim within the staleness window, 18 s under CI scaling. Its heartbeat pass over 50,000 claims is sequential, and with the row-local backlog test loading the same database in parallel the pass took 17 to 19 s, so claims went stale and rescue took them. Instrumented runs showed every rescued claim had gone 18.4 s or more without a heartbeat, most of them never heartbeated since being claimed 21 to 25 s earlier; none was within the staleness window. The two backlog tests now share a lock so they never run together. A trigger records, as rescue closes each compact claim, how long it had gone without a heartbeat. The test first requires that no rescued claim was within the staleness window, and otherwise reports how stale the rescued claims were, so a future failure says whether rescue misjudged a claim or the live runtime's heartbeats fell behind.
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the
|
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f6dc843a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Summary
rescue_drains_an_abandoned_backlog_without_rescuing_live_claimsfailed twice on PG18 CI ("rescue took a claim while its runtime was alive and heartbeating", 500 and then 600). It is a test artifact, not a rescue bug: the runtime that the test keeps alive heartbeats ~50,000 in-flight compact claims 500 per transaction, and with the row-local backlog test (added in #647) running in parallel against the same database on a 4-vCPU runner, a full heartbeat pass took 17–20 s against CI's 18 s staleness window. Its claims really went stale, and rescue took them, one batch per pass, as designed.Reproduced on the merged release code (PG18 limited to 2 CPUs,
CI=true, the row-local test loading the same database) with a trigger on rescue closures recording each rescued claim's heartbeat age at the moment of closure, inside the closing statement after rescue holds the attempt's lock: none of the 2,154 rescued live claims had been heartbeated within the window — 1,654 never had been, the rest 18.40–18.52 s before. Without the extra load the passes took 7–9 s and the test passed. The rescue code paths a real bug would need (per-batch cutoff, fresh-snapshot re-check, the attempt lock against late heartbeats, compact heartbeats writing what the predicate reads, no acceleration without a runtime gone a full staleness window) were checked and hold.The change is test-only:
Testing
Clippy clean. The full
bulk_rescue_testbinary passed 13/13 twice underCI=trueagainst PG18 limited to 2 CPUs (about 7–8 minutes now the two tests run one after the other, inside the shard limit); the 50k test also passed once with external load.