Repository navigation
fix(queue-storage): keep a late heartbeat from replacing parked progress - #727
Conversation
A heartbeat that read a progress snapshot before its attempt parked for a callback could write that snapshot after the park committed, replacing the final progress the park stored. Parking a receipt attempt now takes the attempt lock, which heartbeat writes take too, and the receipt progress upsert leaves out attempts that are waiting for a callback. The explicit flush test now waits until the flush is seen waiting on a lock before releasing it.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 3 minutes. View limit details
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. |
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: 95b0c99287
ℹ️ 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".
…alizers External completion, failure and retry deleted the lease row and then waited for the attempt lock, the reverse of the order every other receipt path and the callback park use, so a fast callback could deadlock with a parking worker. They now take the attempt locks for the callback's leases first.
psycopg 3.3.9 has no Python 3.15 binary wheel yet, so installing psycopg and psycopg-binary separately paired psycopg 3.3.9 with psycopg-binary 3.3.8, which fails to import. The extra pins matching versions.
Follow-up to #725, from a review finding there.
Problem
#725 made parking for a callback store the handler's final progress. A heartbeat that read an older progress snapshot before the park could still write it after the park committed:
mark_finalizingstops new snapshots but not one already being written, the park did not take the attempt lock, and the receipt progress upsert matched the parked attempt's still-open claim.Change
waiting_external, so a heartbeat that gets the lock after the park writes nothing.state = 'running'and lock the lease row, so they are unchanged.Tests
test_queue_storage_heartbeat_does_not_overwrite_parked_progressparks a receipt attempt with progress, sends a heartbeat with an older snapshot, and checks it writes nothing and the parked progress survives. It fails without the upsert change.test_queue_storage_flush_progress_waits_for_a_locked_receipt_attemptnow waits until the flush is seen waiting on a lock before releasing it, instead of sleeping 200 ms.0.6 is unaffected: there the park stores no progress at all.
Generated by Claude Code