Skip to content

feat(jobs): harden supersede lanes — TTL refresh + fenced releaseLane - #70

Merged
agreenspan merged 4 commits into
inixiative:mainfrom
stevenolay:backport/lane-ttl-refresh-and-release
Aug 24, 2026
Merged

feat(jobs): harden supersede lanes — TTL refresh + fenced releaseLane#70
agreenspan merged 4 commits into
inixiative:mainfrom
stevenolay:backport/lane-ttl-refresh-and-release

Conversation

@stevenolay

Copy link
Copy Markdown
Contributor

Backports two supersede-lane hardening improvements discovered while porting this primitive into Zealot (the same Zealot→template direction as the already-landed handler-scoped laneKey — that one is already present on main, so it's not re-touched here).

1. watchLane refreshes the TTL while it holds

Today a holder running past LANE_TTL_SEC (300s) lets its lane key expire. The key then reads back null, so a later usurp by a newer job is missed for stale long-runners. watchLane now pexpires the lane each poll while holder === jobId. An absent (expired) lane still counts as "held" — only a different holder is a usurp (unchanged).

2. Fenced releaseLane + enqueue rollback

enqueue.ts claims the lane before queue.add, but if add throws, the lane is left held by a job that never got created — a phantom holder that wrongly supersedes the real prior job. Added an atomic, fenced releaseLane (GET-and-DEL-if-mine in one Lua eval, so a concurrent newer claim isn't clobbered) and wired the rollback into enqueue.ts.

Validation

packages/db/src/lanes/lanes.test.ts (ioredis-mock): releaseLane is fenced (non-holder no-op; holder drops), and watchLane refreshes the TTL while holding → 6/6. makeSupersedingJob suite still green (usurp path unchanged) → 6/6.

Opened as a draft — initiated from a fork (no write access); flagging for review of the directionality.

🤖 Generated with Claude Code

stevenolay and others added 2 commits June 29, 2026 08:23
Backports two lane-baton improvements from the Zealot port (same Zealot→template
directionality already used for handler-scoped laneKey, which is already present here).

- watchLane now REFRESHES the lane TTL each poll while this job still holds it. Without
  it, a holder running past LANE_TTL_SEC (300s) lets its lane expire; the key reads back
  null and a later usurp by a newer job is missed for stale long-runners. An absent lane
  still counts as "held" (unchanged) — only a different holder is a usurp.
- Add fenced `releaseLane` (atomic GET-and-DEL-if-mine via one Lua eval) and use it in
  enqueue.ts: if `queue.add` throws after `claimLane`, roll the claim back so a job that
  never got created can't leave a phantom holder that supersedes the real prior job. The
  fence ensures a concurrent newer claim isn't clobbered by the rollback.

Tests (packages/db/src/lanes/lanes.test.ts, ioredis-mock): releaseLane is fenced
(non-holder no-op, holder drops), and watchLane refreshes the TTL while holding.
makeSupersedingJob suite still green (usurp path unchanged).

🤖 Generated with [Claude Code](https://claude.com/claude-code)
…vacant-lane reclaim

Backports the rest of the Zealot lane hardening: claimLane atomically
tombstones the displaced holder (superseded:<jobId> → usurper, 7d TTL) and
stretches the claim TTL by the job's delay; makeSupersedingJob checks the
tombstone at start and re-asserts a lapsed baton iff the lane is vacant (NX);
releaseLane also rolls back the tombstone it created (fenced) and logs instead
of swallowing failures; enqueue claims at spill time with fenced rollback, and
the drain pass rolls back its claim when the re-add fails.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@agreenspan

Copy link
Copy Markdown
Contributor

Pushed a second commit (e493789) backporting the rest of the Zealot lane hardening, so this PR now carries the full primitive:

3. Per-job superseded tombstone (durable edge)

claimLane is now an atomic Lua eval: it records the new holder AND writes a superseded:<displacedJobId> → usurperJobId tombstone (7-day TTL) when it evicts a different holder. makeSupersedingJob checks the tombstone at start and exits before running — so a job displaced while queued still aborts even if its lane key expired before it started (the pure-baton design missed that window entirely).

4. Delay-aware claim TTL

The claim happens at enqueue but is only refreshed once the job runs, so claimLane now stretches the TTL by the job's delay (enqueue and the drain pass both pass it through). Otherwise the baton expires mid-delay and the usurp is missed.

5. Vacant-lane reclaim at job start

Queue wait is unbounded, so a baton can still lapse. On start, reclaimLaneIfVacant re-asserts the claim with NX — a lapsed baton is restored (the run becomes visible to lane reads again), a live holder is never clobbered, so newer-claim-usurps semantics are untouched.

6. Rollbacks on every claim site + fenced tombstone cleanup

enqueueJob now claims at spill time too (the newest enqueue must hold the baton during outbox dwell) and rolls back if the spill fails; the drain pass rolls back its claim when the re-add fails. releaseLane takes the previousHolder returned by claimLane and clears the tombstone it created — only when it still points at us — and logs a failed fenced delete instead of swallowing it.

Validation

lanes.test.ts 10/10, makeSupersedingJob.test.ts 8/8 (new: tombstoned-before-start exits without running; lapsed baton re-asserted at start), drain suite 18/18, packages/db 260/260, apps/api 829/829, and full bun run check green.

Noted divergence from Zealot (not ported)

Zealot also force-mints a fresh jobId for superseding jobs (BullMQ dedupes re-adds by jobId) and skips the overflow buffer for delayed jobs. Template already mints uuidv7() per enqueue unless the caller passes a jobId, and its outbox admits delayed jobs — both are pre-existing template design choices, left alone here.

🤖 Generated with Claude Code

… the usurper

Re-claiming a buffered row that was displaced by a newer direct enqueue
(cron, bypass, or a concurrent instance) tombstoned the usurper back —
mutual tombstones meant neither job ran for 7 days. The drain now checks
the row's own tombstone first and deletes displaced rows without re-adding.

Also: the drain's rollback no longer releases a spill-time self-claim when
the re-add fails (the baton must survive while the row stays buffered), and
the spill-time claim stretches its TTL by the job delay like the other two
claim sites.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@agreenspan

Copy link
Copy Markdown
Contributor

Follow-up commit from an adversarial review of the backport:

Fixed — drain could counter-tombstone a legitimate usurper (mutual tombstones → neither job runs). If a buffered row's lane was taken by a newer direct enqueue (cron/cronTrigger go direct even while overflowing, so does bypass: true, or a concurrent instance), the drain's re-claim tombstoned that usurper right back. Both jobs then exited at start on their tombstones — durably, for the 7-day tombstone TTL. The drain now checks the row's own tombstone before claiming and deletes displaced rows without re-adding. Test: drops a buffered row displaced by a newer direct claim.

Fixed — drain rollback released the spill-time baton. When the re-add fails transiently, the rollback was releasing what was actually a self-claim of the baton taken at spill time, leaving the lane vacant until retry. The rollback now skips self-claims. Test: keeps the spill-time baton when re-enqueue fails.

Consistency — spill-time claim now stretches its TTL by the job delay, matching the direct-enqueue and drain claim sites.

Known limitation (not addressed here): if both an older and a newer job's batons lapse while queued (two >300s waits on one lane), the older job starts first, reclaims the vacant lane, and the newer one aborts — newest payload dropped. Since jobIds are uuidv7 (time-ordered), the watch/reclaim could compare ids and never let an older job usurp a newer one — but that breaks if a caller supplies a non-uuidv7 jobId, so leaving it as a design decision for review.

bun run check green (831 tests, lint, typecheck, CI rules).

🤖 Generated with Claude Code

reclaimLaneIfVacant let an older job re-assert a vacant lane and usurp a
newer queued job whose baton also lapsed — last-wins flipped to stale-wins
exactly under the congestion the lanes exist for (found independently by
two reviews). reclaimLane now claims over a vacant lane OR an older holder
(uuidv7 jobIds are time-ordered, so string compare = age compare) and
tombstones the displaced holder; a newer holder is never touched.

flushOutbox: spills accepted mid-drain after the shutdown flush gives up
sat unarmed in the accumulator and their awaits hung forever — they are
now rejected loudly. Also corrected the accumulate() comment that claimed
the finally re-arms stragglers (it deliberately does not).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@agreenspan

Copy link
Copy Markdown
Contributor

Closed the remaining open finding (independently confirmed by a second review on the Zealot side):

Fixed — baton-expiry inversion (stale-wins). reclaimLaneIfVacantreclaimLane: a starting job now re-asserts over a vacant lane or an older holder (uuidv7 jobIds are time-ordered, so a plain string compare is an age compare), tombstoning the displaced holder exactly like a claimLane eviction — but never touches a newer holder, so newer-claim-usurps semantics are unchanged. Previously, when both batons lapsed under congestion, the older queued job started first, re-asserted the vacant lane, and the newer job aborted itself as superseded. Tests: lane-level displacement/tombstone + never-steal-from-newer, and an end-to-end makeSupersedingJob test where the newer job displaces a running older holder and the older run is usurped. Caveat (as discussed): callers supplying a non-uuidv7 jobId lose the age semantics — the compare stays deterministic but arbitrary.

Also fixed — shutdown flush could strand spill awaits. Spills accepted mid-drain after flushOutbox exhausts its retries sat unarmed in the accumulator and their callers hung forever; they're now rejected loudly (narrow path — DB down during shutdown — but the hang class is closed). The accumulate() comment claiming the finally re-arms stragglers was wrong and now describes the real behavior.

bun run check green (834 tests, lint, typecheck, CI rules).

🤖 Generated with Claude Code

@agreenspan

Copy link
Copy Markdown
Contributor

Landed on main via merge-main-into-branch + fast-forward (87aa3ca2..029c82a1) — your four commits are preserved with authorship. Closing since this is a fork PR and can't be merged via the button.

One change applied on top (029c82a1): folded the superseded tombstone under the lane namespace — lane:superseded:<jobId> instead of a standalone superseded namespace. It's a per-job marker of the same supersede-lane primitive, used only in lanes.ts, and no namespace-scoped op ever treats it separately from lane, so the extra top-level namespace wasn't earning its keep.

Review notes (non-blocking):

  • The lanes.ts primitive is solid — uuidv7 age-compare in RECLAIM_LANE, fenced releaseLane, TTL refresh in watchLane, and the drain's drop-tombstoned-rows-instead-of-counter-tombstoning are all correct. Nice work.
  • releaseLane rollback vacates the lane rather than restoring the previous holder. It clears the phantom claim and un-tombstones the prior job (fixing the stated bug), but leaves the lane vacant — so a third claimant arriving in that window wouldn't supersede the still-running prior job (a double-run, edge-of-an-edge: queue.add throws AND a third job races AND prior still running). Acceptable given the rarity; flagging for awareness.
  • The removal of the test-mode delayed-job "return unrun" special-case is beyond the stated scope, but it's validated on current main.

Validation on current main + these changes: lanes 270/0, jobs 942/0, typecheck + biome clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants