Restart recovery runs while the receiver is already serving. It could find a just-written delivery pending, send it and release it before the receiver's Notify for that delivery ran. By then nothing owned the delivery, so Notify took it and a worker sent it again: processNewTask built the delivery from the task as pending and never read its row.
processNewTask now reads the delivery's status by primary key before sending, and skips the task unless the row still says pending, logging that it was already handled. processRetryTask already makes the same check for retrying. The check works whichever of recovery and Notify claims the delivery first, and it leaves startup order and the startup and shutdown budgets alone.
Two things the diff does not show:
The read cannot go stale before the send. Only the engine writes a delivery's status, and it only does so while it owns the delivery, which the worker does until the task returns.
A row left pending by a failed bookkeeping write still reads pending, so the sweep and restart recovery still send it again.
loadRetryDelivery is renamed loadDelivery, since both paths now use it.
Judgement call: a new task whose delivery row cannot be read is skipped, as a retry task already is. A row that is still pending is picked up by the pending sweep; a row that retention has already deleted is no longer sent.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/webhooker/issues/299.
Restart recovery runs while the receiver is already serving. It could find a just-written delivery `pending`, send it and release it before the receiver's `Notify` for that delivery ran. By then nothing owned the delivery, so `Notify` took it and a worker sent it again: `processNewTask` built the delivery from the task as `pending` and never read its row.
`processNewTask` now reads the delivery's status by primary key before sending, and skips the task unless the row still says `pending`, logging that it was already handled. `processRetryTask` already makes the same check for `retrying`. The check works whichever of recovery and `Notify` claims the delivery first, and it leaves startup order and the startup and shutdown budgets alone.
Two things the diff does not show:
- The read cannot go stale before the send. Only the engine writes a delivery's status, and it only does so while it owns the delivery, which the worker does until the task returns.
- A row left `pending` by a failed bookkeeping write still reads `pending`, so the sweep and restart recovery still send it again.
`loadRetryDelivery` is renamed `loadDelivery`, since both paths now use it.
Judgement call: a new task whose delivery row cannot be read is skipped, as a retry task already is. A row that is still `pending` is picked up by the pending sweep; a row that retention has already deleted is no longer sent.
Model: opus-5-5
Restart recovery could find a just-written delivery pending, send it
and release it before the receiver's Notify queued the same delivery.
Notify's claim then succeeded on the released id, and the worker sent
it again because the new-task path never read the delivery's row.
Before sending a new task the worker now reads the delivery's status
by primary key and skips the task unless the row still says pending,
as the retry path already does for retrying. Nothing else can change
the row while the worker owns the delivery. A row left pending by a
failed bookkeeping write is still sent again.
loadRetryDelivery is renamed loadDelivery now that both paths use it.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #299.
Restart recovery runs while the receiver is already serving. It could find a just-written delivery
pending, send it and release it before the receiver'sNotifyfor that delivery ran. By then nothing owned the delivery, soNotifytook it and a worker sent it again:processNewTaskbuilt the delivery from the task aspendingand never read its row.processNewTasknow reads the delivery's status by primary key before sending, and skips the task unless the row still sayspending, logging that it was already handled.processRetryTaskalready makes the same check forretrying. The check works whichever of recovery andNotifyclaims the delivery first, and it leaves startup order and the startup and shutdown budgets alone.Two things the diff does not show:
pendingby a failed bookkeeping write still readspending, so the sweep and restart recovery still send it again.loadRetryDeliveryis renamedloadDelivery, since both paths now use it.Judgement call: a new task whose delivery row cannot be read is skipped, as a retry task already is. A row that is still
pendingis picked up by the pending sweep; a row that retention has already deleted is no longer sent.Model: opus-5-5
Review passed.
Model: opus-5-5