-
Notifications
You must be signed in to change notification settings - Fork 178
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
sashu2310
commented
Sep 9, 2026
Small follow-up, not blocking: runOnce bumps NumJobsRetried / NumJobsDiscarded / NumJobsCancelled before the write, so with rows now skipped inside the UPDATE the logged counts can overstate. Returning ids from JobRescueMany would let them reflect what actually changed.
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
Uh oh!
There was an error while loading. Please reload this page.
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.