Repository navigation
Give the maintenance chores a cadence, and a rescuer the queue cannot strand - #108
Merged
Merged
Conversation
The three chores were enqueued once, sixty seconds after boot, and nothing ever enqueued them again (#107). On a server that stays up the trash was swept exactly once, abandoned uploads were never pruned, and `reclaimStale` -- whose only caller was the prune chore -- never ran again, so a worker that died mid-job left its row `running` for good. A timer owns the cadence now, not the chores. Self-chaining was tried and rejected: a chain lives in the queue and so can be broken by the queue, and the prune chore is the one chain with no rescuer, because the thing that would rescue it is the thing it runs. `reclaimStale` moves out of the job and into the tick, where a dead worker cannot reach it. A tick reclaims, then asks for each chore with `enqueueUnique`, so a slow sweep or one backing off after a failure collects no copies. Nothing is lost when a tick declines: a tick is not a chain, so the next one asks again -- and a chore that has spent every attempt is no longer pending, so the next tick starts it over. A sweep that fails does not stop sweeping. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`stop` cancelled the next tick but said nothing about one already under way, so a SIGTERM landing inside a tick tore the pool down beneath its open transaction and logged a database error that means nothing. The test had the same hole from the other end: with a one-millisecond interval a tick almost always survived `stop` and committed its chores after the drain. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`reclaimStale` used to run about once per boot, so it never met a job that was legitimately still working. An hourly reclaim does, and `started_at` is stamped once at claim -- it dates the job, not its last sign of life. Any job slower than the fifteen-minute window would have been handed to a second worker while the first was still inside it: the 190 MB model download writes one `.partial` and would have had two writers, and a 4K transcode is the same shape. So a running job now touches `started_at` every minute, and the window means "no sign of life for fifteen minutes" rather than "started fifteen minutes ago" -- which is what `reclaimStale` already said it meant. The beat is guarded on `running`, so a row this worker no longer owns is not dragged back to life, and it stops with the job, so a dead worker is still reclaimed exactly as before. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three corners of the recurring reclaim, found in review. A heartbeat says the process is alive, not the job. A handler that hangs -- a fetch with no timeout, an ffmpeg that never exits -- would have defended its row for ever, and with it the name `enqueueUnique` keys on: #107 again, moved inside the heartbeat. A job is now believed for six hours and then let go of, which the window frees a quarter of an hour later. The beat matches the attempt this worker claimed, so a row that was reclaimed and taken by somebody else stops answering to it. And `reclaimStale` compared a cutoff from the app's clock against a `started_at` from Postgres's. That mattered little when it ran once at boot with nothing running; against live jobs a lagging database clock could reclaim a job in its first minute. The comparison is Postgres's now, which is the same reason `enqueue` already defers to it. The tick's reclaim also gets its own catch: a blip recovering stranded jobs must not cost the hour's chores as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two corners left by the last commit. The heartbeat wrote `started_at` from the app's clock while the cutoff it is measured against comes from Postgres's -- the very mismatch that commit removed, reintroduced facing the other way, where a lagging container reclaims a job that is beating perfectly well. It asks Postgres now, like `claim` does. And once the lifetime bound lets go of a hung job, the reclaim can hand the row to a second worker while the first is still inside the handler. The `done` and `fail` writes keyed on the id alone, so the original worker's eventual return erased the run that had replaced it. Both are scoped to the attempt they belong to, which is the guard `beat` already uses. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 16, 2026
Closed
ergofobe
added a commit
that referenced
this pull request
Sep 30, 2026
… row (#117) * Finish the heartbeat: recover the boot, and the worker as well as the row Two blockers the heartbeat left behind, and the two smaller things beside them. All of it is one change, because all of it is #108's own machinery. Boot recovery had quietly stopped recovering anything (#111). The beat rewrites `started_at` every minute, so a crash leaves rows whose last sign of life is under a minute old, and the fifteen-minute window matches none of them -- the comment above the call, which says a restart is the one moment we know nothing is running, had become false. `reclaimAllRunning` says what boot actually means: every `running` row, with no clock in the query at all. Crash recovery goes back to instant from about 61 minutes. The lifetime bound freed the row and left the worker (#112). Nothing abandoned the handler, so a wedged job kept its worker parked in `await` for ever and the released row let a second worker come and hang on the same job -- one slot per cycle until the pool was gone and an ordinary job never ran again, while the row sat `queued` beside four idle-looking workers. The lifetime now races the handler, and a beat that matches no row ends the wait too, since the job is somebody else's by then. Abandonment is the worker leaving and nothing more: a promise cannot be cancelled, so the handler goes on holding its memory, its file handles and any child it spawned until it returns or the process ends, and a retry runs beside it. What is recovered is the slot, which is the part that was never coming back on its own. Handler-side deadlines are filed separately. The attempt-scoped writes also have to agree that the row is still `running`. `reclaimStale` requeues without touching the attempt count, so scoping to the attempt alone did not tell "somebody else has it" from "nobody has it yet", and a late `done` marked a requeued job finished. And zero rows is no longer discarded: it is the one clean signal that a job was taken away mid-flight, which is the case the scoping exists for. Finally, the retry backoff asks Postgres for the time. It wrote `run_at` from the app's clock while `claim` compares it against the database's -- the skew #108 moved the reclaim cutoff to Postgres to remove, relocated rather than removed. Closes #111 Closes #112 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Fail a job that hung rather than sending a second copy after it Review of the commit before this one. Abandonment went through `fail`, which requeues while attempts remain -- and the run it would be retrying is still going, because nothing can stop it. Five attempts at a handler that hangs is five live copies of it, five children and five copies of its memory, one every six hours. Slot starvation traded for a resource leak. So the lifetime bound is the one place the queue gives up on a job it could retry: the row is failed, with the reason in `last_error` and the attempts it did not use still showing, and a person decides whether to start it again. The test moves off `maxAttempts: 1`, which was the one setting that could not have caught this. Abandonment carries whether the row is still this worker's, too. A job taken away mid-flight is somebody else's, so there is nothing to write -- where before, the write went out anyway and warned that it had matched nothing, on the one path where matching nothing is expected. And two notes that were claiming more than the code does: the boot reclaim assumes a single instance, which is the only thing in this file that does, and the attempt-scoped writes are evidence of ownership rather than proof of it while an administrator can rewind the counter (#115). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Say what the lifetime now costs, and stop beating before the last write Review notes on the commit before this one, all of them about a comment that had stopped matching the code. The constant a reader looks up still described being let go of and reclaimed a quarter of an hour later, which is no longer what happens to a job that runs past it. The vigil also stops before the write that fails the row rather than after it, so a beat cannot land in the middle of that round trip and report a row it has just been failed out of. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Keep a failed abandonment write from requeueing what it could not fail Third review round, and the one finding in it that matters: the write that fails a hung job sat inside the handler's own `try`, so a dropped connection under it fell through to the `catch`, which calls `fail`, which requeues while attempts remain. A database blip would have undone exactly the decision the branch above it exists to make, and sent a second copy after a handler that is still running. It has its own `catch` now: a row that cannot be marked failed is left `running` with its beat stopped, which is indistinguishable from a worker that died, and the hourly reclaim has known what to do with that since #107. The vigil now stops as soon as the race is decided rather than per branch, so no terminal write races a beat. A beat landing during the completion write found the row already `done` and warned that the job had been taken away -- on an ordinary success, which is the one place that warning must never appear if it is to mean anything. Two beats can be in flight at once when a round trip outlasts the interval, so the test no longer asks for exactly one of them. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * Keep beating when the abandonment write throws A thrown abandonment update left the row running with its heartbeat stopped, so reclaim would start a second worker beside the handler still in this process. * Record the row when a handler settles after a failed abandonment write --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: Jim Phillips <41898282+ergofobe@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #107.
The three chores were enqueued sixty seconds after boot and nothing enqueued them ever
again. On a server that stays up the trash was swept once, abandoned uploads were never
pruned, and
reclaimStale— whose only recurring caller was the prune chore — never ranagain, so a worker that died mid-job left its row
runningfor good.What drives the cadence
A timer, not the chores.
startMaintenanceschedules a tick, and each tickqueue.reclaimStale(), thenenqueueUnique.A self-chaining chore was tried in #92's PR and rejected, for a reason that still holds: a
chain lives in the queue and so can be broken by the queue, and the prune chore is the one
chain with no rescuer, because the thing that would rescue it is the thing it runs. A
death between its self-enqueue and its
donewrite forks it permanently.What rescues the rescuer: nothing has to.
reclaimStalemoves out ofPRUNE_JOBS_JOBand into the tick, so the recovery no longer depends on a job the queue has to keep alive.
A timer is outside the queue: no worker can strand it, and whatever kills the process that
owns it is also what starts the process that replaces it. The prune chore keeps only the
housekeeping —
pruneCompletedand expired sessions — where losing an hour costs nothing.When a chore throws
Deliberately: the successor is never the failing chore's responsibility, so a failure
cannot end the cadence. The queue's own backoff is the next run, and a tick declines to
add a second while a chore is queued or running. Once a chore has spent every attempt and
gone
failedit is no longer pending, so the next tick simply starts it over. A sweep thatfails does not stop sweeping — that being #107 in miniature.
enqueueUniqueis safe here in a way it was not for a chain, because a tick is not achain. Nothing is carried forward when a tick declines; the next one asks again.
What review turned up, and what it cost
Making
reclaimStalerecur means that for the first time it can meet a job that islegitimately still working.
started_atwas stamped once at claim, so it dated the jobrather than its last sign of life, and anything slower than the fifteen-minute window — the
190 MB model download, a 4K transcode — would have been handed to a second worker while the
first was still inside it. Three commits deal with the consequences:
started_atevery minute, so the window means silence, not age —which is what
reclaimStalealready said it meant. Written with Postgres's clock, andthe cutoff it is compared against moves to Postgres's clock too.
job; a
fetchwith no timeout would otherwise defend its row for ever and hold the nameenqueueUniquekeys on — The maintenance chores run once per boot and never again, so the trash is never swept #107 again, moved inside the heartbeat.beat,doneandfailare all scoped to the attempt they belong to. A worker returning late no longererases the run that replaced it.
stop()also waits for a tick already under way, so shutdown does not close the pool underan open transaction.
Tests
All six cadence tests were written first and watched fail against the old behaviour (five
failed; the sixth — that a tick queues all three — is the one thing the boot-time enqueue
did do). The queue tests were written the same way.
duplicate) and after it has exhausted every attempt (a fresh one)
runningis freed by a tick, with no help from the queueEverything asserts on what the queue holds, never on a timer firing.
Verify
bun install && bun run verify && bun run --filter '@imogen/web' build—963 pass, 0 fail,web bundle built. No
bun.lockdiff. SDK pin untouched (v0.5.1).Found, not fixed
sweepTrashandpruneUploadsare capped at 500 rows per run and neither asks foranother pass when it fills the cap. With a strictly hourly cadence the cap is now the
throughput ceiling: a ten-thousand-item trash backlog takes about twenty hours to
reclaim, and nothing says so. Pre-existing, but the timer is what makes it visible.
enqueueUniquecalls are sequential and unguarded, so a transientfailure on the first costs the hour's other two. The reclaim has its own catch; the
chores do not. Self-heals next hour.
maintenance.stop()waits on an in-flight tick with no timeout, so a SIGTERMarriving while a tick is blocked on a wedged pool waits out the orchestrator's grace
period instead of exiting.
FIRST_TICK_DELAY_MSnever ticks. Thechores used to be rows a dead process left behind for the next boot; now they are not.
The reclaim half is still covered by the boot-time
reclaimStaleinindex.ts.Review
code-reviewat medium, four rounds — one past the three-round cap, which I should flag.Rounds 1–3 were fixed as found. Round 4 raised two more, both one-line corrections to the
commit round 3 had just produced (the heartbeat's clock, and the terminal-write ownership
guard) rather than a new design, so they were taken and the loop stopped there. Nothing was
redesigned twice. The four items above are what is left, and they are listed rather than
fixed because none of them blocks this change.
🤖 Generated with Claude Code