Guard job rescue against stale snapshots - #1373
Conversation
| @@ -503,7 +503,8 @@ SET | |||
| ) + 1 | |||
| ), | |||
| state = @state | |||
There was a problem hiding this comment.
This closes the completed-in-the-gap case but not the changed-hands case. Between JobGetStuck and this write a job can fail, go straight to available (JobSetStateErrorAvailable / JobSetStateInterrupted), and be claimed by another worker, so the row is running again with a fresh attempted_at and this predicate still matches. Re-checking the read predicate in full would cover both:
AND river_job.state = 'running'
AND river_job.attempted_at < @stuck_horizon::timestamptz;
StuckHorizon is already on JobRescueManyParams (the new test passes it too); it just isn't forwarded in riverpgxv5's JobRescueMany.
| state = @state | ||
| WHERE id = @id; | ||
| WHERE id = @id | ||
| AND state = 'running'; |
There was a problem hiding this comment.
Same re-claim gap as the Postgres query; AND attempted_at < cast(@stuck_horizon AS text) here, and forward params.StuckHorizon in the driver loop.
|
|
||
| ### Fixed | ||
|
|
||
| - Fixed `JobRescuer` overwriting jobs that complete or otherwise leave the running state after being fetched for rescue, preserving their state, errors, metadata, and timestamps across PostgreSQL and SQLite drivers. [Issue #1302](https://github.com/riverqueue/river/issues/1302). |
There was a problem hiding this comment.
If the horizon check goes in, worth a clause that a job re-claimed by another worker during the rescue window is also left alone. "Leave the running state" reads as if re-entering it is not covered.
|
Small follow-up, not blocking: |
A worker can complete or release a job after the rescuer fetches it but before the rescue update executes. The job can also be claimed again, leaving it running under a new worker when the stale rescue arrives. Require jobs to still be `running` with `attempted_at` before the original rescue horizon in both PostgreSQL and SQLite updates. Forward the horizon through every driver so stale rescues preserve completed jobs and fresh attempts, including their errors, metadata, and timestamps. Add shared driver coverage for completion, immediate retry, and worker interruption between fetch and rescue, plus strict horizon boundaries and mixed batches containing eligible jobs. Document the fix in the changelog. Fixes #1302.
0baef16 to
20b7323
Compare
bgentry
changed the title
2 participants
A worker can complete or release a job after
JobRescuerfetches it but before the rescue update executes. The job can also be claimed by another worker during that gap, returning it torunningwith a fresh attempt. A stale rescue must not overwrite either outcome.Require jobs to still be
runningwithattempted_atbefore the original rescue horizon in both PostgreSQL and SQLite updates. Forward the existing horizon parameter through all drivers so completed jobs and fresh attempts retain their state, errors, metadata, and timestamps, while eligible jobs in the same batch are rescued normally.Add shared driver regression coverage for completion, immediate retry, and worker interruption between fetch and rescue, along with strict horizon boundaries. Update the unreleased changelog entry to cover reclaimed jobs.
Fixes #1302.