Skip to content

test(rescue): serialise the bulk rescue tests and assert rescued claims were stale (0.6) - #706

Merged
hardbyte merged 2 commits into
release/0.6.6from
brian/rescue-live-claim-0.6
Nov 9, 2026
Merged

hardbyte merged 2 commits into
release/0.6.6from
brian/rescue-live-claim-0.6

Conversation

@hardbyte

@hardbyte hardbyte commented Nov 9, 2026

Copy link
Copy Markdown
Owner

Summary

rescue_drains_an_abandoned_backlog_without_rescuing_live_claims failed 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:

  • The two backlog tests share a lock, so they no longer run concurrently.
  • A trigger records how long each rescued compact claim had gone without a heartbeat, and the test first asserts that no rescued claim was within the staleness window — a failure there would be a real rescue bug. The zero-rescue check stays, and its message now reports how stale the rescued claims were, so a future failure says which case it is.

Testing

Clippy clean. The full bulk_rescue_test binary passed 13/13 twice under CI=true against 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.

…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.
@hardbyte hardbyte added the full-ci Run the full CI matrix (Python build+test, E2E) on this PR label Nov 9, 2026
@coderabbitai

coderabbitai Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: b6f27b9c-17df-4688-a94c-4e7d166375b5

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-09T07:51:23.098669Z 2f6dc84 PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread awa/tests/bulk_rescue_test.rs Resolved
@hardbyte
hardbyte merged commit de8aa70 into release/0.6.6 Nov 9, 2026
66 of 67 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

full-ci Run the full CI matrix (Python build+test, E2E) on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant