feat(selfhost): add automatic dead-letter job retry for self-host queues#2581
Conversation
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
gittensory-ui | ae2472b | Commit Preview URL Branch Preview URL |
Jul 02 2026, 11:50 AM |
|
Warning 🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨 ⏸️ Gittensory review result - manual review recommendedReview updated: 2026-07-02 11:52:16 UTC
⏸️ Suggested Action - Manual Review
Review summary Nits — 2 non-blocking
Review context
Contributor next steps
Signal definitions
🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2581 +/- ##
==========================================
+ Coverage 95.98% 96.02% +0.04%
==========================================
Files 229 233 +4
Lines 25813 26079 +266
Branches 9390 9475 +85
==========================================
+ Hits 24777 25043 +266
Misses 425 425
Partials 611 611
🚀 New features to boost your workflow:
|
Neither self-host queue backend (SQLite or Postgres) has a way to move a job out of status='dead' once it lands there. The Cloudflare Workers Queues DLQ redrive path (src/queue/dlq.ts) is wired exclusively to that platform's native queue consumer and is not reachable from the self-host tables. In practice this means: a job that dies from a transient bug, or a bug that's since been fixed and redeployed, requires a manual database UPDATE/DELETE to ever run again -- indefinitely. - Add reviveDeadLetterJobs() to both createSqliteQueue and createPgQueue: requeues dead jobs whose lifetime `attempts` is still under a ceiling (maxRetries + a small configurable extra-attempts budget, default 3), clearing last_error. `attempts` itself is left untouched, so a revived job gets exactly ONE more attempt before either succeeding or dying again -- not a fresh full retry budget -- bounding how many times a permanently- broken job can cycle before it stops being revived and requires manual intervention. - Runs on a separate, much slower timer (default 30 minutes, QUEUE_DEAD_LETTER_REVIVE_INTERVAL_MS) than the normal poll tick -- that interval IS the cooldown between auto-retry rounds, so no separate timestamp bookkeeping was needed. Reuses the existing attempts/job_key columns rather than a parallel table. - New gittensory_jobs_dead_letter_revived_total metric, distinct from the existing crash-recovery gittensory_jobs_recovered_total counter. - reviveDeadLetterJobs() is exposed on the public queue interface (not just wired to the internal timer) so it's directly testable without waiting on real intervals, and available for a future operator-triggered repair path. Validation: full local gate green; every new/changed line and branch across sqlite-queue.ts, pg-queue.ts, and queue-common.ts is covered per coverage/lcov.info (cross-checked against the diff hunks directly). New tests cover: revival under the ceiling with last_error cleared, the ceiling actually stopping further revival, no-op with nothing dead, the periodic timer firing a real revival end-to-end (sqlite, fake timers), the equivalent mocked-pool behavior for Postgres, and the two new env-var config knobs.
The AI review on this PR flagged a real defect: reviveEligibleDeadJobs selects candidate dead jobs, then updates each one back to 'pending' by id alone. That SELECT is a stale snapshot -- an overlapping reviver (a second self-host instance sharing the same Postgres database, or a slow prior revive tick still running when the next one fires) can move a row out of 'dead' between the SELECT and this row's own UPDATE. With no re-check, the UPDATE blindly flips whatever the row's CURRENT status is back to 'pending', including a row already claimed into 'processing' -- letting the job run a second time concurrently. Both backends now add "AND status='dead'" to the revive UPDATE, exactly mirroring the same defensive predicate already used elsewhere in both files for the same class of stale-snapshot sweep (reclaimExpiredProcessingJobs / deferPendingJobsForRateLimit). The revived count now reflects rows the UPDATE actually matched (pg's rowCount / sqlite's changes), not the raw SELECT count, so a row another reviver already claimed is correctly excluded rather than double-counted. Added a regression test per backend: Postgres via a mock pool with per-call configurable UPDATE rowCounts (proving a 0-rowCount UPDATE isn't counted as revived); SQLite via a driver.query spy that injects the competing status change at the exact point the real UPDATE would otherwise race against it (proving the row is left untouched, not reverted to pending).
57542a1 to
ae2472b
Compare
|
Resolved the flagged defect: `reviveEligibleDeadJobs`'s SELECT-then-UPDATE is a stale snapshot — an overlapping reviver (a second self-host instance sharing the same Postgres DB, or a slow prior revive tick still running when the next fires) could move a row out of `'dead'` between the SELECT and this row's own UPDATE, and the UPDATE (keyed on Both backends now add `AND status='dead'` to the revive UPDATE, mirroring the identical defensive predicate already used elsewhere in both files for the same class of stale-snapshot sweep (`reclaimExpiredProcessingJobs` / `deferPendingJobsForRateLimit`). The revived count now reflects rows the UPDATE actually matched, not the raw SELECT count. Added a regression test per backend:
Also rebased onto current `main` (two other PRs merged in the interim). Full local gate (`test:ci`, unsharded `test:coverage`, `npm audit`) green; every changed line/branch in `pg-queue.ts`/`sqlite-queue.ts` is covered (cross-checked the remaining uncovered branches directly against the diff — all pre-existing and unrelated). |
Summary
Closes #2534.
Neither self-host queue backend (SQLite or Postgres) has a way to move a job out of
status='dead'once it lands there. The Cloudflare Workers Queues DLQ redrive path (src/queue/dlq.ts) is wired exclusively to that platform's native queue consumer and is not reachable from the self-host tables. In practice this means: a job that dies from a transient bug, or a bug that's since been fixed and redeployed, requires a manual databaseUPDATE/DELETEto ever run again — indefinitely.What changed
reviveDeadLetterJobs()to bothcreateSqliteQueueandcreatePgQueue: requeues dead jobs whose lifetimeattemptsis still under a ceiling (maxRetries+ a small configurable extra-attempts budget, default 3), clearinglast_error.attemptsitself is left untouched, so a revived job gets exactly one more attempt before either succeeding or dying again — not a fresh full retry budget — bounding how many times a permanently-broken job can cycle before it stops being revived and requires manual intervention.QUEUE_DEAD_LETTER_REVIVE_INTERVAL_MS) than the normal poll tick — that interval is the cooldown between auto-retry rounds, so no separate timestamp bookkeeping was needed. Reuses the existingattempts/job_keycolumns rather than a parallel table, per the issue's requirement.gittensory_jobs_dead_letter_revived_totalmetric, distinct from the existing crash-recoverygittensory_jobs_recovered_totalcounter (a different scenario — process-restart recovery vs. dead-letter revival).reviveDeadLetterJobs()is exposed on the public queue interface (not just wired to the internal timer), so it's directly testable without waiting on real intervals, and available for a future operator-triggered repair path.Correctness/safety notes
attempts < ceilingfilter bounds how many total revivals a job can get before it's left dead permanently.Validation
npm run test:ci) green,npm audit --audit-level=moderateclean,git diff --checkclean.sqlite-queue.ts,pg-queue.ts, andqueue-common.tsis covered — cross-checked the diff hunks directly againstcoverage/lcov.info; the handful of uncovered lines/branches in those files are all pre-existing and unrelated to this change.last_errorcleared, the ceiling actually stopping further revival, no-op when nothing is dead, the periodic timer firing a real end-to-end revival (SQLite, fake timers), the equivalent mocked-pool behavior for Postgres, and both new env-var config knobs (defaults + overrides).Safety