Skip to content

Latest commit

 

History

History
581 lines (328 loc) · 68.3 KB

DESIGN_DECISIONS.md

File metadata and controls

581 lines (328 loc) · 68.3 KB

DESIGN_DECISIONS.md

Format for each: Context → Options → Decision → Trade-off accepted → Revisit when.

Every number quoted here is measured, not predicted, and cites where it came from: EXPLAIN.md for planner output against exactly 100,000 job rows, and the session entries in PROGRESS.md for everything else. Several of these decisions were made after being wrong first; where that happened the earlier position is stated rather than quietly replaced, because the correction is the part worth reading. ADR-015 through ADR-018 exist only because measurement or a browser contradicted a document.


ADR-001 — Postgres as the queue

Context. Needed durable, ordered, atomically-claimable job storage across multiple workers.

Options. (a) Postgres with FOR UPDATE SKIP LOCKED; (b) Redis lists/streams; (c) RabbitMQ; (d) SQS.

Decision. Postgres.

Rationale. SKIP LOCKED gives correct atomic claiming with no additional infrastructure. Job state and metadata mutate in one transaction, eliminating the dual-write problem — with Redis, a completed job whose Postgres update fails leaves the two stores disagreeing, and reconciling that is a whole subsystem. Durability, backups, and PITR come free.

Trade-off accepted. Polling latency exceeds a blocking pop; throughput ceiling in the thousands/sec rather than hundreds of thousands; long claim transactions cause bloat requiring autovacuum tuning at scale.

Revisit when. >5k jobs/sec sustained or p99 pickup must be sub-50ms. Postgres remains the system of record; Redis Streams becomes the dispatch layer.


ADR-002 — SKIP LOCKED over the alternatives

Context. N workers poll the same table and each row must be handed to exactly one of them. The claim is the only place in this system where a correctness bug is invisible in development and catastrophic in production, because a single worker never races with anyone.

Options. (a) SELECT candidate ids, then UPDATE ... WHERE id = ANY(...) in a second statement. (b) SELECT ... FOR UPDATE without SKIP LOCKED. (c) pg_advisory_lock per job id. (d) One statement: UPDATE ... WHERE id IN (SELECT ... FOR UPDATE SKIP LOCKED).

Decision. (d). The claim is a single statement — selection, locking and mutation in one round trip. Verbatim from packages/core/src/claim.ts:

UPDATE jobs j
SET status          = 'claimed',
    worker_id       = $1,
    claimed_at      = now(),
    lock_expires_at = now() + ($2 || ' seconds')::interval,
    attempt         = j.attempt + 1,
    updated_at      = now()
FROM (
  SELECT j2.id
  FROM jobs j2
  JOIN queues q ON q.id = j2.queue_id
  WHERE j2.queue_id = $3
    AND j2.status   = 'queued'
    AND j2.run_at  <= now()
    AND q.is_paused = false
  ORDER BY j2.priority DESC, j2.run_at ASC, j2.id ASC
  FOR UPDATE OF j2 SKIP LOCKED
  LIMIT $4
) sel
WHERE j.id = sel.id
RETURNING j.*

FOR UPDATE OF j2 rather than a bare FOR UPDATE is deliberate: without the alias Postgres would also try to lock the joined queues row, so every worker claiming from the same queue would serialise on it — reintroducing option (b)'s problem through the back door.

Rationale, and the negative control. (a) is the design most people write, and its failure mode is not theoretical. The same 1,000-job / 10-worker harness that passes against the current claim was run against a SELECT-then-UPDATE implementation: 2,560 jobs handed out for 1,000 rows — 1,560 duplicates, 750 rows with attempt <> 1, one row claimed seven times — and it deadlocked repeatedly. The window between the two statements is where every one of those duplicates lives. One worker sees the row as queued, so does the next, and both proceed. Nothing in a single-worker test can produce this, which is exactly why it ships.

That run is the reason the passing test is worth anything. A concurrency test that has never been observed failing proves only that it executes. This one was watched failing against the naive implementation before being trusted against the correct one, and both duplicate detectors fire hard: Set(ids).size === 1000 and count(*) FILTER (attempt <> 1) = 0.

(b) is correct but serializes the fleet: worker 2 blocks on the rows worker 1 has locked instead of moving to the next available ones, so throughput collapses to roughly one worker's worth regardless of how many you run. SKIP LOCKED is the difference between "wait your turn" and "take the next free row".

(c) needs a lock per job id, and the locks are held on the session rather than the transaction, so a crashed worker leaks them until its connection dies. Worse, it does not compose with the ordering: you cannot express "the highest-priority row that is not currently locked" as an advisory lock predicate — you would have to fetch candidates, try to lock each, and fall back, which reintroduces (a)'s window and adds a round trip per attempt. It is also application-level locking for something the storage engine already does correctly, which CLAUDE.md forbids for this reason.

Evidence it is genuinely concurrent rather than accidentally serialised: 10 workers on independent connection pools via Promise.all claimed 100/120/100/110/90/100/80/90/110/100 — every worker got work, so the loops really did overlap.

Trade-off accepted. The claim holds row locks for the duration of its transaction, so a long claim transaction blocks the reaper from touching those rows and contributes to bloat; mitigated by the statement being a single round trip that touches exactly LIMIT rows. SKIP LOCKED also gives up strict priority ordering under contention — a worker skips a locked high-priority row and takes a lower-priority one — which is the correct trade for a work queue and would be wrong for a ledger.

Revisit when. Never, for this storage engine. If Postgres stops being the queue (ADR-001's revisit condition) the whole mechanism is replaced rather than adjusted.


ADR-002a — Claiming from one queue, not ANY(queue_ids)

Context. A worker serves several queues. The obvious claim takes them all at once with queue_id = ANY($3), which was the original spec.

Options. (a) One claim across all the worker's queues with ANY; (b) one claim per queue, iterated in queue-priority order.

Decision. (b), and this was a correction forced by measurement, not a preference.

Rationale. idx_jobs_claim is (queue_id, priority DESC, run_at ASC, id ASC). An equality prefix on queue_id lets the index supply the ordering, so LIMIT terminates after reading exactly LIMIT rows. ANY turns that prefix into a set membership test, the index can no longer supply a global ordering across queues, and the planner sorts every claimable row before applying LIMIT. Measured on the 100k dataset: the ANY-plus-aging shape scanned 5,065 rows with a 366 kB sort at 7.641 ms; the single-queue shape scans 10 rows with no sort node at 0.110 ms (EXPLAIN.md findings 3 and 5). The cost of the old shape also scaled with queue depth, which is the property that matters — it degrades exactly as the system gets busy.

Precedence is preserved by the worker iterating its queues in priority order instead, which gives the same overall ordering across queues while keeping each individual claim on the indexed path.

Trade-off accepted. A worker serving ten queues issues up to ten statements per poll rather than one. At 0.110 ms each that is a good trade, but it is more round trips and the loop, not the database, now owns cross-queue precedence — so a bug in the iteration order is a fairness bug the SQL cannot catch.

Revisit when. A worker routinely serves enough queues that the per-poll round trips dominate. The fix is a shard key rather than a return to ANY.


ADR-003 — Per-queue concurrency: correctness over throughput

Context. queues.max_concurrency caps how many of a queue's jobs run at once — the point of it is to protect a downstream dependency that cannot take more. A cap that is exceeded under load is worse than no cap, because it is relied upon.

Options. (a) Count in-flight jobs, then claim max_concurrency - running. (b) Lock the queue row with SELECT ... FOR UPDATE, then count, then claim. (c) A counter column incremented on claim and decremented on completion. (d) No enforcement; treat max_concurrency as advisory.

Decision. (b). claimJobsWithinQueueLimit takes a row lock on the queue, counts jobs in claimed/running, then claims LEAST(workerCapacity, max_concurrency - running).

Rationale. (a) has the same read-then-write window as ADR-002's negative control, one level up: ten workers each read running = 0 against a cap of 5 and each claim 5, so 50 jobs run against a limit of five. The count is only meaningful if nothing else can claim between reading it and acting on it, and the queue row is the natural thing to serialise on because it is the object the limit belongs to. (c) is tempting and is wrong on the failure path — a worker that is SIGKILLed never decrements, so the counter drifts upward and permanently under-admits until someone reconciles it; deriving the count from job rows cannot drift because the job rows are the truth. (d) was rejected because the feature is specified.

Trade-off accepted. This is the deliberate correctness-over-throughput choice, and the cost is real: claims against a single queue are serialised. Two workers claiming from emails at the same instant queue behind one another on that row lock. Cross-queue claims stay fully parallel, because they lock different rows — so the ceiling is per-queue, not global. On a queue with a high cap and many workers, that lock becomes the bottleneck before max_concurrency does.

Stated plainly because it is easy to hide: the unlimited claim path (claimJobs) has no such serialisation, so a queue that needs maximum throughput should not set max_concurrency at all rather than set it high.

Revisit when. A single queue needs both a concurrency cap and more throughput than one serialised claim path allows. The answer is sharding, not a cleverer lock: split the queue into N shards each with max_concurrency / N, which restores parallelism because the shards lock separate rows. shard_id on queues is listed as future work for this reason. An advisory-lock-free alternative — SELECT count(*) ... FOR UPDATE on the jobs rather than the queue — was not pursued because it locks a growing set instead of one row.


ADR-004 — Separate scheduler process, no leader election

Context. Three jobs are cluster-wide rather than per-worker: materialising due cron schedules into jobs, reaping expired leases, and marking silent workers dead. Each must happen once per due item, not once per worker.

Options. (a) Run them inside every worker. (b) Run them in one worker elected leader (Raft, or a lock table, or a leader lease row). (c) A separate process, single instance, and accept it as a single point of failure. (d) A separate process, N replicas, made safe by the queries themselves.

Decision. (d). apps/scheduler runs three independent 10s loops. There is no Raft, no lock table, no leader lease and no if (isLeader) anywhere in the process.

Rationale. Safety is a property of the queries, not of coordination code. Cron and the reaper both select FOR UPDATE SKIP LOCKED, so concurrent replicas partition the due set instead of duplicating it; the sweeper is a single idempotent UPDATE. Given that, leader election buys nothing and costs a consensus dependency plus a failure mode of its own — a leader that is alive but partitioned. (a) does the work N times, which for cron means N duplicate jobs per fire. (c) removes the duplication but makes cron stop when one process dies.

The cron pass does the whole selection and the next_run_at advance in one transaction, and that is load-bearing rather than tidy: FOR UPDATE locks are held until commit, so if the selection committed before the advance, replica B would pick up a row replica A had already decided to fire.

Proven twice. In the suite: two and three replicas on independent pools firing the same due schedule via Promise.all produce exactly one job, repeated across ten simulated minutes, and with 20 due schedules all 20 fire exactly once with the work partitioned. Then in Docker with --scale scheduler=3 for eight real minutes: one job per minute for eight consecutive minutes, zero minutes with more than one.

One precision about what that proves: scheduler-2 won every race and the other two did no cron work. That is correct — the guarantee is no duplication, not even distribution. With one due schedule and a 10s poll, whichever replica ticks first takes the row. Distribution is what the 20-schedule test covers.

The three loops are staggered by 0, TICK/3 and 2·TICK/3. Not cosmetic: without it, N replicas issue 3N simultaneous queries every 10s — a synchronised herd — and a replica restarted mid-deploy re-aligns with its peers.

Trade-off accepted. Work distribution is arbitrary rather than balanced, and a replica can idle while another does everything. No leader also means no single place to put work that genuinely must be singleton — if such a task appears, this ADR has to be reopened rather than worked around. And three loops on one 10s timer means the worst-case latency for a due item is one tick, not zero; that is why lease recovery and job promotion share the loop with the real deadline (ADR-018).

Revisit when. A cluster-wide task appears that cannot be made idempotent or partitionable by SKIP LOCKED. Then the honest answer is a real lease-based leader (a row with an expiry, renewed) — still not Raft.


ADR-005 — At-least-once, not exactly-once

Context. A job must run. The question is what happens when a worker completes the job's side effect and then dies before it can record that it did.

Options. (a) At-most-once: mark the job done before running it, so a crash loses work silently. (b) At-least-once: mark it done after running it, so a crash re-runs it. (c) Exactly-once.

Decision. (b), at-least-once, with two mechanisms that let a cooperating handler behave as if it were exactly-once.

Why (c) is not on the table. Exactly-once delivery is unachievable here, and the reason is not a limitation of Postgres or of this implementation. The handler's side effect and the status update are in two different systems — an HTTP call to a payment provider, and a row in our database — and there is no transaction spanning both. Whatever order they happen in, there is an instant where one has committed and the other has not, and a process killed in that instant is indistinguishable from one killed just before it. Moving the window does not close it: commit-then-run loses work, run-then-commit duplicates it. No amount of database cleverness fixes that, because the problem is that a distributed commit needs the other side to participate.

What is achievable is exactly-once effect, and only with consumer cooperation — the handler has to be able to recognise work it has already done. So the system's job is not to promise exactly-once; it is to give handlers what they need to deduplicate, and to be honest that the guarantee stops at our boundary.

The two mechanisms:

  • idempotency_key on enqueue, with a partial unique index. The same key twice produces one job, returning 202 then 200 and preserving the original payload — verified in the API suite. This closes the producer side: a client that retries a network-failed enqueue does not create a second job.
  • UNIQUE (job_id, execution_seq) on execution. Two executors racing to record the same execution fail at the database rather than in application code, so the guarantee does not depend on the worker being correct.

And handlers receive {jobId, attempt} so they can key their own deduplication on it.

A note on the execution-side constraint, because it was UNIQUE (job_id, attempt) first and that was wrong. It was replaced in migration 0005_job_execution_seq, and the reason belongs in this ADR because it is ADR-007 arriving from the other direction.

UNIQUE (job_id, attempt) is the obvious design and it is incompatible with refunding an attempt. ADR-007 refunds one on infrastructure failure — the reaper does attempt = attempt - 1 when a lease expires, because a SIGKILLed worker is not a failed job. That refund makes attempt numbers reusable: a job killed on attempt 1 returns to attempt 0, is re-claimed as attempt 1 again, and now needs a second execution row charged to the same attempt. Under the old constraint the insert for that retry violated uniqueness, so every post-crash retry failed — the two documented rules were in direct conflict.

The first resolution was ON CONFLICT (job_id, attempt) DO UPDATE, which kept the constraint enforceable but overwrote the abandoned row — erasing the record that a worker died mid-execution, which is precisely the event the audit trail exists to capture. That trade was reverted. Separating identity from accounting removes the conflict instead of paying for it:

  • execution_seq is monotonic per job, allocated fresh per execution and never reused. It is the execution's identity, and it is what the unique constraint keys on.
  • attempt records which attempt the execution was charged to. It is a plain, non-unique column, and it is what the retry policy counts against max_attempts.

Both invariants now hold at once. The refund still happens, and every execution keeps its own immutable row — including the one nobody finished. After a crash-and-retry a job correctly shows (seq 1, attempt 1) still in running and (seq 2, attempt 1) completed, which is the state the audit trail is supposed to record.

The constraint also still does its real job. UNIQUE (job_id, execution_seq) means two executors racing to record the same execution fail at the database, not merely in application code — which is the defense-in-depth point worth making, since the guarantee does not depend on the worker being correct.

One consequence to state honestly rather than gloss: because attempt can repeat, the {jobId, attempt} pair handlers receive is stable across a crash-and-retry, not unique per physical run. For downstream deduplication that is usually the semantics you want — an interrupted attempt that gets retried should not charge a card twice. But it means a handler cannot use that pair to distinguish "first run of attempt 1" from "re-run of attempt 1 after a crash". A handler that genuinely needs per-run identity needs execution_seq plumbed into HandlerContext, which is not currently passed.

Full reasoning in DATABASE.md ("Why execution_seq and attempt are separate columns"), CONCURRENCY.md section 5, and the Session 3 correction block in PROGRESS.md, which records how the conflict surfaced and why the first fix was backed out.

Trade-off accepted. Handlers can be invoked more than once for the same job, and the system does not prevent it — a handler with a non-idempotent side effect and no deduplication of its own will double-charge under the right crash. That is a real cost pushed onto the handler author, and the only mitigations are the two mechanisms above plus documentation. http_request is the honest example: it cannot know whether its POST arrived before the worker died, so it is at-least-once in the strict sense.

Two further costs worth naming. idempotency_key is optional, so the producer-side guarantee only exists for clients that opt in. And because attempt repeats after a refund, {jobId, attempt} is stable across a crash-and-retry but is not unique per physical run — usually the semantics you want, but a handler needing per-run identity has no way to get it today.

Revisit when. A handler genuinely needs per-run identity — then plumb execution_seq into HandlerContext, which is a small change and deliberately not made speculatively. If transactional outbox semantics are ever needed for a specific downstream (writing the intent and the job in one transaction, with a relay), that is a per-handler pattern rather than a change to this guarantee.


ADR-006 — Partial indexes

Context. jobs is append-mostly and overwhelmingly terminal: on the measured dataset, 100,000 rows of which 5,065 (5.1%) are queued. The claim only ever reads that 5%. A full index over all 100,000 rows would be 95% dead weight in the buffer cache, and that ratio gets worse forever as completed jobs accumulate.

Options. (a) Full indexes on the columns the claim orders by; (b) partial indexes with WHERE status = 'queued'; (c) move terminal jobs to an archive table.

Decision. (b). idx_jobs_claim is (queue_id, priority DESC, run_at ASC, id ASC) WHERE status = 'queued', with the same pattern for the other hot paths (idx_jobs_scheduled, the DLQ and idempotency indexes) — 11 named indexes, 7 of them partial.

Rationale. The index contains only claimable rows, so the claim is O(log n) over the pending set rather than the total set, and ten million completed jobs leave the resident index the same size it is today. Because status = 'queued' is also in the claim's WHERE, the planner can prove the index covers the predicate and drops the recheck.

Measured, all from EXPLAIN.md against 100,000 rows with the queue under test holding 1,740 claimable:

variant inner selection full claim rows read sort
all indexes present 0.110 ms 0.441 ms 10 none
idx_jobs_claim dropped 1.802 ms (16×) 2.250 ms (5.1×) 1,740 157 kB quicksort
no jobs indexes at all 7.643 ms (69×) 8.286 ms (19×) 1,740 157 kB quicksort

The row counts matter more than the milliseconds. With the index the plan is Limit → LockRows → Nested Loop → Index Scan and reads exactly LIMIT rows; without it, it reads every claimable row in the queue and sorts them before LIMIT applies. That is the difference between a query whose cost is constant and one whose cost scales with backlog.

A fourth measurement is worth keeping because it justifies a column that looks redundant: id is the last term in the index purely as a tie-break. Removing it still uses the index but adds an Incremental Sort and reads 33 rows instead of 10 — 0.149 ms against 0.110 ms. Small, but it is the difference between "reads LIMIT rows" and "reads however many share the leading key", and it was applied as migration 0004 after being measured rather than assumed.

(c) was rejected as premature: partial indexes give most of the benefit with none of the operational cost of moving rows between tables, and the archive only becomes necessary when the heap rather than the index stops fitting.

Trade-off accepted. Partial indexes are fragile in a specific way: the predicate must match the query's predicate or the planner silently ignores the index — no error, no warning, just a slower plan. ADR-016 is that failure actually happening. So every one of these indexes is a coupling between a migration and a query that nothing type-checks, and the only defence is verifying the plan rather than reading the schema. Writes also pay to maintain several partial indexes, and a status transition moves a row into or out of an index rather than updating it in place.

Revisit when. The claimable fraction stops being small — if a queue is chronically backlogged, "partial" stops meaning "tiny" and the index approaches the table. Also re-run pnpm db:explain after any change to the claim or the indexes; the numbers above are a snapshot of one dataset on one machine and EXPLAIN.md is the source of truth.


ADR-007 — Attempt not charged on infrastructure failure; aborted ≠ failed

Context. attempt is what the retry policy counts against max_attempts to decide between another try and the DLQ. So the question "what counts as an attempt" is the question "what can send a healthy job to the dead letter queue".

Options. (a) Every claim increments attempt; a job that disappears has used one up. (b) Distinguish job failure from infrastructure failure and only charge the former. (c) A separate infra_failures counter with its own, higher limit.

Decision. (b), in two places that are easy to conflate but are different code paths.

1. Lease expiry refunds the attempt. The reaper's requeue does attempt = attempt - 1. A worker that was SIGKILLed, OOM-killed, or lost its network never delivered a verdict on the job — the job did not fail, the machine did.

2. A handler aborted by shutdown is not a failed handler. executeJob checks signal.aborted before interpreting a rejection and returns interrupted, leaving the claim for drain() to release with the attempt refunded.

Rationale. Under (a) a rolling deploy charges an attempt to every in-flight job simultaneously, so three deploys against max_attempts = 3 dead-letter healthy work — and the last_error on those DLQ entries reads like a genuine application fault, so the operator debugs the wrong thing. The blast radius scales with fleet size, which is the wrong direction.

The second path was found in a container, not in a test, and that detail is the useful part. The first real SIGTERM run showed the in-flight sleep job going down the retry path: err: sleep aborted → job scheduled for retry, released: 0. The unit test had passed throughout, because it used a promise that never settles — exercising the grace-expiry branch while leaving the handler-rejects-on-abort branch entirely uncovered. After the fix: running|attempt 1 → queued|attempt 0, released: 4. A test for that branch now exists.

(c) was rejected as more state to keep correct for no additional guarantee; the refund achieves the same thing by making the counter mean one thing.

Trade-off accepted. Two real costs. A job whose handler reliably kills its worker — an OOM caused by the payload — is refunded forever and retries indefinitely, because the system cannot distinguish "worker died" from "this job kills workers". There is no poison-pill detection; the mitigation is the lease reaper's visibility (lease.expired climbing steadily is the alert) rather than a limit. Second, attempt numbers become reusable, which broke the execution-side unique constraint and forced the execution_seq separation — see ADR-005.

Verified by removing it: dropping attempt = attempt - 1 fails 5 tests, including the three-crash-cycle test that exists specifically to prove a rolling deploy cannot dead-letter healthy jobs, and the attempt-ceiling test added later.

Revisit when. Poison-pill jobs appear in practice. The fix is a separate infra_failures column with its own generous ceiling — (c) above — so an endlessly worker-killing job eventually stops rather than cycling forever.


ADR-008 — Full jitter on retry backoff

Context. Failures in this system are correlated, not independent. One downstream outage fails every job that talks to it, at nearly the same instant, so their retries are scheduled at nearly the same instant too.

Options. (a) Deterministic exponential backoff; (b) exponential with a small random ± percentage; (c) full jitter — random(0, computed_delay); (d) decorrelated jitter.

Decision. (c), applied to all three strategies (fixed, linear, exponential), each clamped to max_delay_ms.

Rationale. With (a), 500 jobs that failed together retry together — a thundering herd aimed at a service that is, by definition, already unhealthy. The retry storm is then indistinguishable from the original outage and can extend it indefinitely. (b) narrows the spike but does not remove it: the distribution is still a spike, just a wider one. Full jitter spreads retries uniformly across the whole interval, which is the cheapest possible admission control.

The same argument appears twice more in the codebase, which is why it is an ADR rather than a line of code: the worker's idle poll backoff is jittered so N idle workers do not poll in lockstep, and the scheduler's three loops are staggered for the same reason.

backoff.ts is pure — injectable RandomSource, clock passed as a parameter, no I/O — precisely so the distribution can be tested deterministically rather than by observing timings.

Trade-off accepted. Full jitter makes the expected delay half the computed delay, so the effective backoff curve is gentler than the nominal one, and an individual job's retry timing is unpredictable — you cannot promise "retried in exactly 4s", only "within 4s". For a job queue that is the right trade; for anything with a latency SLA per attempt it would not be.

Revisit when. Retries need to be predictable for an external contract, or measurement shows retries clustering anyway (which would mean the jitter range is too narrow relative to the failure arrival rate — then decorrelated jitter, option (d)).


ADR-009 — Polling over WebSockets

Context. The dashboard shows live queue depth, job status and worker heartbeats. The brief permits either transport.

Options. (a) refetchInterval polling; (b) WebSockets fed by Postgres LISTEN/NOTIFY; (c) Server-Sent Events; (d) long polling.

Decision. (a), at 2s on live views, opted into per hook.

Rationale. A 2s delay on a monitoring dashboard is imperceptible — the human reading it cannot act faster than that, and the underlying data (a heartbeat every 5s, a poll loop every 500ms) is not fresher than 2s anyway. Against that, WebSockets add a stateful connection to a stateless API, a reconnect/backfill story, and fanout state that has to be per-tenant-correct or it becomes an isolation bug. TanStack Query gives polling with cache dedup, background refetch and error handling for one config line.

Two details that make polling behave rather than merely work:

  • refetchInterval is deliberately not global. A job list wants 2s; a retry-policy list wants never. Set globally, the dashboard makes sixty requests a minute for data nobody is looking at.
  • refetchOnReconnect is off, because polling already covers recovery and without this every live query refetches simultaneously when a laptop wakes — a self-inflicted thundering herd, the same shape as ADR-008.

The scale-out design, if it is ever needed: a trigger on jobs issues NOTIFY jobs_changed, '<project_id>'; one API instance holds the LISTEN connection and fans out over WebSockets to subscribers filtered by project; the client keeps the existing query cache and applies deltas, with a full refetch on reconnect to close gaps. The subscription filter is the security-sensitive part and would need the same treatment as ADR-013's route table.

Trade-off accepted. Up to 2s of staleness, and a fixed request floor per open tab regardless of whether anything changed. The floor is the real cost at scale: 100 operators with a dashboard open is ~3,000 requests/minute of mostly-unchanged data.

That cost bit once already and needed a fix rather than a shrug. Infinite queries refetch every loaded page on each poll, so twenty "Load more" clicks turn a 2s poll into twenty-one sequential requests every 2s — which cannot complete inside its own interval, and the rate limiter then starts rejecting the operator's real actions. The interval now backs off with page depth. Invisible on a small dataset because nobody loads a second page.

Revisit when. Concurrent dashboard sessions make the request floor a real load problem, or a feature needs sub-second latency — a live log tail is the obvious one, and SSE (option (c)) would be sufficient for that alone without adopting bidirectional sockets everywhere.


ADR-010 — Missed cron runs: fire once, skip the backlog

Context. The scheduler was down for an hour. A * * * * * schedule has 60 missed occurrences. What should happen when it comes back?

Options. (a) Fire all 60; (b) fire once and rejoin the cadence; (c) fire once with the skipped count recorded; (d) make it configurable per schedule.

Decision. (c). The next occurrence is computed from now, never from the stored next_run_at, and the number skipped is reported on the cron.materialized event.

Rationale. It is one line and it is the whole ADR:

// from now, not from row.next_run_at
const next = nextOccurrence(expression, timezone, asOf);

Advancing from next_run_at looks more correct and is worse in a way that is hard to see. On a per-minute schedule after an hour of downtime, next_run_at + 1min is still 59 minutes in the past — so the row is due again on the very next pass, and the dispatcher replays the backlog one job per tick. It looks like a working scheduler while it happens: jobs are flowing, the loop is healthy, nothing errors. They just arrive an hour late and in the wrong order relative to everything else, which for anything time-sensitive (a report "for the last minute", a reminder) is worse than not running at all. Advancing from now fires once and rejoins the cadence.

(a) is the same failure without the disguise: 60 simultaneous jobs immediately after a deploy, when the system is least able to absorb them, is how a scheduler causes the outage it was recovering from. (d) is the right long-term answer but there is no column for it, so it is not built — and the default has to be the safe one either way.

Skipped runs are counted rather than discarded silently, saturating at 1,000 so that a long outage on a fast schedule cannot spend the whole pass counting. Without that number the policy is invisible: an operator cannot tell "fired once, skipped 59" from "fired once, nothing was missed", and it is the difference between a healthy recovery and a data gap someone needs to know about.

A pass also reads its own clock once, server-side, and uses that single instant for the due predicate, the job's run_at and the cron computation. Two reasons: replicas on skewed hosts must not disagree about whether a row is due, and a pass straddling a minute boundary would otherwise fire a job dated before the boundary while computing a next_run_at after it — silently dropping a run.

Verified by inverting it: advancing from the missed occurrence instead of now reproduces the backlog-replay bug exactly — 10:01 where 11:01 was expected, i.e. still an hour in the past.

Trade-off accepted. Missed runs are genuinely lost. For a schedule that must not skip — a nightly billing job — this policy is wrong and there is currently no way to opt out short of a manual enqueue. The skipped_runs figure makes the loss visible and alertable, which is the mitigation, not a fix.

Revisit when. A schedule needs catch-up. Add catchup_policy (skip | fire_once | fire_all with a bounded window) to scheduled_jobs, defaulting to skip. Bounded, because unbounded catch-up is option (a).


ADR-011 — Denormalizing workers.last_heartbeat_at

Context. worker_heartbeats is an append-only time series (one row per beat per worker, carrying running_jobs and mem_mb) used for charts and history. workers.last_heartbeat_at is a single column holding the most recent beat. The same fact is in two places, which normally means one of them is wrong.

Options. (a) The time-series table only, with MAX(logged_at) where liveness is needed; (b) the column only, discarding history; (c) both, with the column as a maintained cache.

Decision. (c), and it is called out here because an unexplained duplicate reads as an error while an explained one reads as a decision.

Rationale. The two have different access patterns and only one is hot. The sweeper asks "which workers are past their deadline" every 10 seconds across the whole fleet; answering that from the time series is a grouped aggregate over a table that grows by (workers × 12) rows per minute forever — the cost of a liveness check would scale with retained history, which is the wrong shape for a query on a fixed interval. As a column it is an indexable scalar and the sweep is a single UPDATE ... WHERE last_heartbeat_at < now() - interval. (b) loses the per-beat running_jobs and mem_mb series the dashboard charts.

The duplication is safe because there is exactly one writer and it writes both in the same beat: beatOnce updates the column and inserts the sample together. The column is a cache of the series' maximum, never an independent fact, so it cannot disagree except transiently.

Trade-off accepted. Two writes per beat instead of one, and a genuine possibility of drift if any future code path writes one without the other — the invariant lives in one function and is not enforced by the schema. Nothing checks it.

Revisit when. A second writer of heartbeats appears (a sidecar, a health proxy). At that point either funnel both through one function or derive the column with a trigger, which trades application discipline for database machinery.


ADR-012 — Drizzle over Prisma

Context. The most important query in this system cannot be expressed by any ORM's query builder. UPDATE ... WHERE id IN (SELECT ... FOR UPDATE SKIP LOCKED) is raw SQL, and it is the query the project is graded on.

Options. (a) Prisma; (b) Drizzle; (c) postgres.js alone with hand-written types; (d) TypeORM/Sequelize.

Decision. (b), with postgres.js underneath and raw sql templates used freely for the claim, the reaper and the promoter.

Rationale. Prisma's value is the typed client, and reaching SKIP LOCKED means $queryRaw, which returns unknown and forfeits exactly the property that justified adopting it — so the most important query in the system would be the least type-safe. Prisma also owns migrations through its own schema language, so the partial indexes of ADR-006 become @@index escape hatches or raw migration files, and the schema stops being the source of truth precisely where it matters most. Drizzle is SQL-shaped: the schema is TypeScript that maps 1:1 onto DDL, db.select() is typed where a builder suffices, and dropping to sql\...`` for one statement is unremarkable rather than an escape hatch. (c) was the honest alternative and was rejected only for migrations and the typed schema; it would have been fine.

Two costs discovered in use, recorded because they are not obvious from the README and each cost real time:

  • .desc() emits DESC NULLS LAST. That silently disabled the claim's index. See ADR-016 — it is the single most expensive thing the ORM did.
  • The sql template expands a JS array into separate placeholders, so ANY(${ids}::uuid[]) becomes ANY(($2,$3,…)::uuid[]) and fails. Array parameters have to go through the postgres.js tag or sql.param(). This bit twice, in different sessions.

Also worth stating: the driver returns timestamptz as a string from raw templates and as a Date only through typed db.select(). Declaring a raw row's field as Date compiles and throws toISOString is not a function at runtime.

Trade-off accepted. Drizzle is younger and less documented than Prisma; the generated migration SQL needs reading rather than trusting (the execution_seq migration was emitted as a single ADD COLUMN ... NOT NULL, which fails on a populated table and had to be hand-split into add-nullable / backfill / set-not-null). And because raw SQL is easy, the codebase has more of it than an ORM-first project would — which is the point for the claim and a liability everywhere else.

Revisit when. Never, for this project. The decision is downstream of SKIP LOCKED, and that is not changing.


ADR-013 — Routes as data, so tenancy cannot be forgotten

Context. SECURITY_GUIDELINES.md section 2 makes a specific prediction about how multi-tenant isolation fails in practice: the org_id filter gets applied to most routes and silently dropped on a few. That is not a failure of intent — everyone agrees the guard should be called — it is a failure of enumeration. With 35 project-scoped routes spread over eight files, "every route calls assertProjectAccess" is a claim nobody can verify by reading, and the omission surfaces when a customer reads another customer's data rather than in testing.

The usual mitigations are weak for exactly this reason. A code-review checklist depends on the reviewer remembering. A lint rule can see that a handler mentions the guard, not that the guard runs before the query. A test suite that covers "a representative sample" of routes proves nothing about the route someone adds next week.

Options. (a) Call assertProjectAccess at the top of each handler, enforced by review. (b) Express middleware applied per router with router.use, inheriting down a path prefix. (c) Global middleware that inspects the URL and infers the tenancy check. (d) Make each route a data declaration, and derive the middleware chain from it.

Decision. (d). Every route is an entry in ALL_ROUTES declaring method, path, scope, zod schemas, rate-limit bucket and handler. mountRoutes is the only function in the codebase that turns a route into an Express handler chain, and it attaches requireProjectAccess to every entry whose scope.kind is project.

Rationale. The property this buys is not "the guard is applied consistently" — it is that the set of routes is a value the test suite can iterate. Three consumers read the same array: the router, the OpenAPI generator, and test/isolation.test.ts. The test therefore covers all 35 project-scoped routes by construction rather than by a list someone maintains by hand, and a new route is covered the moment it is declared.

That closes the enumeration gap, and four checks close the ways around it:

  • Drift — the test walks the live Express router stack and fails if any mounted method+path is absent from ALL_ROUTES, so bypassing the table with a direct router.get breaks CI.
  • Structural — the guard middleware is a named function, and the test asserts its presence on each project-scoped route's layer stack. This holds even if a behavioural assertion is wrong.
  • Classification — every route not scoped project must appear in an explicit reviewed allowlist, so declaring a tenant-data route as user-scoped to dodge the guard fails loudly.
  • Fixtures — validation runs before the guard, so a route with a body schema needs a valid sample body or its 403 assertion would be masked by a 422. Missing fixtures fail their own test rather than silently weakening a case.

Option (a) was rejected because a handler that forgets the call is indistinguishable from one that does not need it. (b) is the conventional answer and is genuinely decent, but path prefixes do not match tenancy boundaries here — /queues/:id, /jobs/:id, /dlq/:id and /retry-policies/:id are all project-scoped while sharing no prefix, and /workers/:id shares the shape of all of them while being deliberately global. (c) infers authorization from a string, which fails the moment a path is added that the pattern does not anticipate.

RBAC rides the same mechanism: minRole is a field on the declaration, resolved in middleware, never in a handler — which is what section 3 asks for.

Trade-off accepted. Three real costs. First, indirection: reading a route file no longer shows the middleware order, which lives in mountRoutes and has to be understood separately — mitigated by that order being documented in one place instead of implied in forty. Second, the chain is uniform, so a route needing a genuinely unusual order has to change the mounting function rather than express it locally; no route has needed this yet, and if one does, that is a signal worth seeing. Third, RouteDef is a small framework of our own, and a bespoke abstraction is a liability when the next person expects plain Express — accepted because it is roughly 100 lines and buys a verifiable security property.

Verified by deliberately breaking it, three ways: dropping the userId predicate from the guard's join fails 37 of 50 isolation tests; reclassifying one route as user-scoped fails the classification check (and the total silently drops 50 → 49, which is precisely the lapse the check exists to catch); mounting a route directly with router.get fails the drift check.

Revisit when. The route count grows past roughly a hundred and ALL_ROUTES becomes unwieldy — split per domain while keeping one flat export, since the flatness is the property that matters. Or if the API moves to a framework with a first-class typed route registry (Fastify's schema-first routes, tRPC, NestJS guards), in which case that registry replaces this table and the isolation test should be re-pointed at it rather than deleted.


ADR-014 — /workers is deployment-scoped, not tenant-scoped

Context. Every other authenticated route in the API is project- or org-scoped. The three worker routes — GET /workers, GET /workers/:id, GET /workers/:id/heartbeats — are not, and this needs stating rather than leaving as an apparent oversight next to a document that insists every project-scoped query filters by org_id.

The reason is in the schema: workers has no org_id and no project_id column (DATABASE.md). That is not an omission either. A worker is an OS process that registers itself and claims from queues by name across the whole deployment; it is infrastructure, like a database replica. It has no owner to filter by, and workers.queues is a text[] of names, not a foreign key, so there is no join that would yield one.

Options. (a) Leave the routes deployment-scoped and authenticated, and say so. (b) Add a nullable org_id to workers and filter on it. (c) Derive visibility from the queues a worker serves — show a worker to a tenant if any name in workers.queues matches a queue in one of their projects. (d) Restrict the routes to a platform-operator role. (e) Drop the routes from the tenant API and expose the fleet only through /metrics.

Decision. (a) for now, with the exclusion made explicit in code and in the test suite rather than left implicit. The routes require authentication; they are listed in test/isolation.test.ts's reviewed allowlist, so a reviewer meets the decision instead of discovering an unguarded route; and the header of routes/workers.ts states the limitation.

Rationale. What is actually exposed is hostname, PID, version, capacity, status, heartbeat time and in-flight count. No job payloads, no queue contents, no project or organization identifiers — the serializer emits a fixed field list, so this is a property of the code and not of the current schema. The disclosure is therefore a description of the operator's own infrastructure, which every authenticated user of a single-tenant deployment already knows, and it is the same class of information a status page publishes.

(c) is the interesting option and is the likely long-term answer, because it needs no schema change and lines up with how workers actually relate to tenants. It was rejected for now on honesty grounds: matching on a text[] of queue names across projects is a fuzzy join — two tenants may legitimately both have a queue called emails, so a name match does not establish that a worker is serving your queue. Shipping that would produce a filter that looks authoritative and is not, which is worse than a documented absence of one. (b) is the clean fix but attributes a shared process to one organization, which is false whenever a worker serves several tenants' queues — the normal case. (d) needs a platform-operator role the system does not have; adding one to solve a read-only infrastructure view is disproportionate. (e) loses the fleet view the dashboard is specified to show (FRONTEND_SPEC.md).

Worth being precise about the residual risk, since "infrastructure metadata" can be made to sound harmless: hostnames and versions are reconnaissance material, and a version number tells an attacker which CVEs to try. In a genuinely multi-tenant hosted deployment that is not acceptable and (c) or (b) becomes required. In a single-tenant or self-hosted deployment — what this system targets — it is not a meaningful exposure.

Trade-off accepted. Any authenticated user, including a viewer in an unrelated organization, can enumerate the worker fleet and read its hostnames and versions. Accepted because the data is not tenant data, the field list is a closed allowlist, and the deployment model is single-tenant. Explicitly not accepted as a permanent answer for a hosted multi-tenant product.

Revisit when. The platform hosts more than one paying organization, or a worker's queues array becomes a foreign key rather than a name list. At that point implement (c): resolve workers.queues against queues.id per project and filter, which also makes the fleet view more useful — an operator wants the workers serving their queues, not all of them. Adding a shard_id to queues (already listed as future work) would supply the same join.


ADR-015 — Quoting measured numbers, not predicted ones

Context. An early draft of DATABASE.md claimed the claim query went from Seq Scan 240ms → Index Scan 0.4ms. It was a plausible headline and it was in a document presented as fact.

Options. (a) Keep the prediction, it makes the point; (b) measure and quote whatever comes out; (c) measure, and if the numbers are less impressive, quote the prediction anyway as "typical".

Decision. (b). pnpm db:explain generates EXPLAIN.md from real EXPLAIN ANALYZE output against exactly 100,000 rows, DATABASE.md's prediction is deleted, and every performance figure in this repo points at the generated document.

Rationale. Neither endpoint reproduced. The real figures are 7.643 ms without any jobs index and 0.110 ms with the full set — a 69× improvement, not 600×. And the reason is instructive rather than embarrassing: at 100,000 rows the heap is 18 MB and fits inside shared_buffers (128 MB), so even the sequential scan is served from memory and never touches disk. The prediction was describing a disk-bound scan on a dataset larger than the one being measured. The gap does widen with scale, so the prediction was not wrong in kind — it was wrong for this dataset, which is the only dataset anyone can check.

Two things fell out of measuring instead of predicting, and neither would have been found otherwise. The 240ms figure was hiding the fact that the claim was not using its index at all (ADR-016). And the 0.4ms figure was close enough to the eventual 0.441 ms full-claim number to look like a successful prediction, while the inner selection was actually 0.110 ms — so a reader who accepted it would have drawn the right conclusion from the wrong evidence. Being accidentally nearly right is worse than being measurably wrong, because nothing prompts you to look again.

The habit generalises to the rest of this document: 2,560 handouts for 1,000 rows (ADR-002), 1,740 rows and a 366 kB sort (ADR-002a), 2,510 stranded jobs (ADR-018) are all observed outputs, and where a claim is reasoned rather than measured this document says so.

Trade-off accepted. The numbers are specific to one dataset, one Postgres configuration and one machine, so they are not portable and will drift — the honest cost of measuring rather than asserting. Mitigated by the harness being re-runnable rather than transcribed, so the numbers can be regenerated instead of trusted.

Revisit when. Any change to the claim query or the indexes; re-run the harness rather than editing the figures. The dataset also needs re-seeding first — a running worker consumes it, so numbers taken from a partly-drained database are not comparable.


ADR-016 — Rejecting DESC NULLS LAST, and verifying index shape against the catalogue

Context. Drizzle's .desc() helper emits DESC NULLS LAST. A plain ORDER BY priority DESC means DESC NULLS FIRST, which is the SQL default. The schema used the helper; the query in CONCURRENCY.md used plain DESC.

Options. (a) Change the query to NULLS LAST to match the index; (b) change the index to NULLS FIRST to match the documented query; (c) make the column NOT NULL and assume the distinction cannot matter.

Decision. (b), pinned explicitly with nullsFirst() on every descending index column (migration 0003).

Rationale. (c) is the intuition and it is false, which is the whole reason this ADR exists. Postgres will not use an index whose null placement differs from the requested ordering even on a NOT NULL column, where the difference is unobservable in any result. There is no error and no warning. The planner quietly abandoned idx_jobs_claim, fell back to idx_jobs_explorer, and sorted every claimable row in the queue: 1,740 rows at roughly 4.9 ms instead of 10 rows at 0.110 ms — same index columns, same query text, a silently different plan.

This is the failure mode partial and composite indexes have in general (noted in ADR-006's trade-off): the index is either usable by the query or decorative, and nothing in the type system, the migration or the ORM tells you which. It cost measurable time to find and was only found because the plan was being read.

The operational rule that came out of it, and the reason this is an ADR rather than a bug fix: verify index shape with pg_indexes, not with the ORM source. The schema file is what you intended; the catalogue is what exists. The same check caught the migration for execution_seq being emitted in a form that fails on a populated table.

(a) would also have worked, and was rejected because it makes the documented query carry a non-obvious clause to satisfy an ORM helper's default — the coupling would survive but point the wrong way.

Trade-off accepted. Every descending index column now carries an explicit nullsFirst() that looks redundant to a reader who does not know this story, and a future contributor using .desc() out of habit reintroduces the bug silently. There is no lint rule and no test that would catch it; only re-reading the plan would.

Revisit when. Drizzle changes its default, or a genuinely nullable column needs an index — at which point the null placement becomes semantically meaningful and has to be chosen rather than pinned.


ADR-017 — Priority aging is opt-in, because it costs the index

Context. A low-priority job behind a permanently busy high-priority queue starves. The standard fix is priority aging: let waiting time accrue into effective priority.

Options. (a) Always on; (b) never; (c) per-queue opt-in.

Decision. (c), via queues.enable_priority_aging, default false. Both ORDER BY variants are spelled out in CONCURRENCY.md sections 1 and 1a and the claim switches on the column.

Rationale. Aging is (priority + EXTRACT(EPOCH FROM (now() - created_at)) / 3600.0) DESC, and that expression is computed per row against now(), so no B-tree can supply it. The index stops providing the ordering, the planner sorts the whole claimable set, and LIMIT no longer terminates early. Measured: 0.110 ms → 2.362 ms on the selection (21.5×) and 0.441 ms → 2.795 ms on the full claim (6.34×), reading 1,740 rows with a 171 kB sort instead of 10 rows with none (EXPLAIN.md finding 3).

So the choice is not "fairness or not" but "fairness on this queue, at a cost that scales with its depth". A queue with uniform priorities gains nothing from aging and would pay 21× for it, which is why the default is off; a queue that genuinely mixes priorities and has starving work can accept the cost knowingly. The dashboard's toggle states the measured figure next to the checkbox rather than describing it as an optimisation setting.

Trade-off accepted. Two claim shapes exist, so the hot path has a branch and both variants need testing. The ORDER BY fragments are the only interpolated SQL in the claim — static, containing no caller input, but interpolated nonetheless. And starvation is real on queues that leave it off; nothing warns about it.

Revisit when. Aging is wanted broadly enough that the cost matters. A materialised effective_priority column, recomputed on a schedule, would be indexable and would move the cost off the claim path — that is the design if this stops being niche.


ADR-018 — scheduled was a terminal state: the index for a reader never written

Context. jobs.status = 'scheduled' means "delayed or awaiting retry", documented in lifecycle.ts as "becomes claimable when run_at passes". Three code paths write it: every retry backoff in recordFailure, the API's enqueue for a future run_at, and the lifecycle table's own transition. The claim requires status = 'queued', and it must, because idx_jobs_claim is a partial index on exactly that predicate.

Nothing implemented "becomes claimable when run_at passes."

Found by querying the deployed database during a pre-deploy check: 2,510 jobs in scheduled with run_at in the past across three queues, none of them waiting for anything. A job that failed once was finished — permanently invisible to every worker. The retry system, which is the headline feature, did not work in a deployment.

idx_jobs_scheduled ON jobs (run_at) WHERE status = 'scheduled' had been in the schema since Session 1, as an index for a reader nobody wrote.

Options. (a) Widen the claim to accept scheduled where run_at <= now(). (b) Have the worker promote rows it finds due, before claiming. (c) A promotion pass in the scheduler, alongside the reaper. (d) Drop the scheduled status and use queued with a future run_at.

Decision. (c). promoteDueScheduledJobs in packages/core/src/lease.ts, run first in the reaper loop: FOR UPDATE SKIP LOCKED with a 500-row batch, reporting whether a backlog remains.

Rationale. (a) is the smallest diff and destroys ADR-006 — the claim's predicate would no longer match the partial index, so the index becomes decorative and the claim starts scanning. That is exactly the trap ADR-016 documents, and CLAUDE.md forbids "simplifying" the indexes for this reason. (b) puts cluster-wide work in every worker, duplicating it N times (ADR-004). (d) is defensible and would remove the whole class of bug by removing the state, but it discards the distinction the dashboard needs — "waiting for its time" versus "ready and not picked up" — which is the split ADR-019 relies on.

It lives in lease.ts because that module is already the time-driven repair surface and already owns the other two statements that write status = 'queued'; it needs no new file and keeps M6's "consumes core, never writes claim SQL of its own" boundary. It runs before the lease sweep because retry latency has the same deadline lease recovery does, and the two touch disjoint status sets so the order is latency rather than correctness. run_at is deliberately left untouched — it records when the job became due and is what the backoff is measured against.

WHY EIGHT GREEN GATES MISSED IT. This is the part worth keeping. Newly enqueued and cron-materialized jobs go straight to queued, so a demo where nothing fails looks perfectly healthy. And the retry test performed the transition by hand:

/** Makes a 'scheduled' retry claimable without waiting out the backoff. */
UPDATE jobs SET status = 'queued', run_at = now() - interval '1 second'
WHERE id = ${jobId} AND status = 'scheduled'

Framed as a clock skip, it also performed a state transition that no production code performed. The suite proved the backoff arithmetic while standing in for the missing component. It now calls the real promoter and only skips the clock.

This is the third instance of one pattern in this project: Session 3's never-settling promise passed while the handler-rejects-on-abort branch was broken (ADR-007); Session 5's single-instant cron test passed under a missing SKIP LOCKED because the two passes happened to serialise. A fixture that substitutes for production behaviour, rather than only for the passage of time, will make a suite pass over a missing feature. That is the lesson, and it is cheap to check for: ask of every test helper whether production has an equivalent.

Verified on the running stack, not only in tests: job.promoted count=500 backlog=true every 10s, backlog cleared in five passes, scheduled 2,511 → single digits, and all three queues drained to zero. Sensitivity checked by dropping the run_at <= now() predicate, which fails the new test with expected 5 to be 3.

Trade-off accepted. Retry latency is now bounded below by the reaper's 10s tick, so a 1s backoff can take up to 11s to become claimable — the backoff is a minimum, not a schedule, and that should be stated wherever backoff delays are documented. The reaper loop also has two responsibilities now, named in its result type rather than hidden. And 500 is a judgement call: large enough to clear a 2,500-row backlog in five passes, small enough that one transaction does not lock a significant fraction of the table.

Revisit when. Retry latency of up to one tick is too coarse. The fix is LISTEN/NOTIFY on the promotion — or a shorter dedicated loop — rather than a bigger batch.


ADR-019 — depth split into claimable and scheduled

Context. A reviewer opened the dashboard and read Depth 850 / Running 0 on three queues, and reasonably concluded the scheduler was broken.

Options. (a) Leave depth as the sum and explain it in a tooltip; (b) redefine depth to mean claimable only; (c) keep depth as the total and add the breakdown.

Decision. (c). /queues/:id/stats returns depth_claimable (the claim's predicate verbatim: status = 'queued' AND run_at <= now()) and depth_scheduled (the remainder), so the two always sum to depth.

Rationale. depth summed two things an operator needs to distinguish: work a poll would pick up right now, and work waiting for its run_at. Those have opposite diagnoses. Claimable non-zero while Running stays 0 means the workers are not draining and something is wrong; a large scheduled count with Running 0 is a queue whose retries are simply not due yet, and is normal. One number cannot say which, so it invited the wrong conclusion — and in this case the wrong conclusion happened to be right, which is luck rather than instrumentation.

(b) was rejected because depth as total backlog is what a capacity alert wants. Defining it as the remainder rather than as its own filter is deliberate: the two cannot drift apart or fail to reconcile with the total.

Trade-off accepted. Three numbers where there was one, on a panel that is already dense. Mitigated by rendering the breakdown indented under the total rather than as siblings, so the relationship is visual.

Not fixed and worth flagging: services/metrics.ts still computes the Prometheus queue-depth gauge as queued + scheduled, the same conflation. Left alone because an alerting gauge plausibly wants total backlog, but it should probably become two series.

Revisit when. The Prometheus gauge is used for autoscaling, where scaling on total backlog rather than claimable backlog would add workers for jobs that are not yet runnable.


Deliberately not built

Queue sharding. Split a queue into N shards, each with max_concurrency / N, and have workers claim per shard. Design: a shard_id column on queues (or a shard_count on a logical queue plus a derived shard key on jobs), the claim unchanged because each shard is already a queue by another name. Out of scope because it is the scale-out answer to ADR-003's serialised claim path, and nothing measured here needs it — the per-queue lock is not the bottleneck at the throughput this system was tested at. It is the first thing to build if it becomes one.

Distributed locking / leader election. A lease row with an expiry, renewed by the holder, giving one replica the right to run singleton work. Deliberately not built because ADR-004 removed the need for it: every cluster-wide task here is either idempotent or partitioned by SKIP LOCKED, so a leader would add a dependency and a new failure mode (alive-but-partitioned) in exchange for nothing. This is an omission with a reason, not a gap — if a genuinely singleton task appears, ADR-004 gets reopened.

Event-driven execution via LISTEN/NOTIFY. A trigger on jobs issues NOTIFY jobs_ready, '<queue_id>'; each worker holds a LISTEN connection and polls immediately on notification instead of waiting out its backoff. Removes most of the idle-poll latency and most of the idle-poll load. Out of scope because polling with jittered backoff is correct and the latency it costs (up to 500 ms idle, one 10 s tick for a promoted retry) is within budget for this workload. The honest caution: NOTIFY is fire-and-forget and delivers only to currently-connected listeners, so it must remain an optimisation over polling rather than a replacement — a worker that reconnects must not need a missed notification. Any implementation keeps the poll loop as the correctness path.

WebSocket live updates. Design sketched in ADR-009: LISTEN/NOTIFY → one API instance holding the listen connection → per-project fanout → client applies deltas to the existing query cache, full refetch on reconnect. Out of scope because 2 s polling is imperceptible on a monitoring dashboard and the subscription filter is security-sensitive — a fanout that leaks another tenant's events is an isolation bug of the exact kind ADR-013 exists to prevent, and it would need the same enumerable treatment.

Job DAGs / dependencies. jobs.depends_on is a uuid[] in the schema and is serialized by the API, but nothing enforces it — the claim does not check whether dependencies are satisfied, so a job with depends_on set is claimable immediately. That is a schema field without behaviour, and it is the most misleading thing in the data model, so it is stated here rather than left to be discovered. Design: extend the claim's predicate with NOT EXISTS (SELECT 1 FROM jobs d WHERE d.id = ANY(j2.depends_on) AND d.status NOT IN ('completed', 'cancelled')), which is correct and costs an anti-join on the hot path — so it would want its own index and its own EXPLAIN pass before being believed. A DAG also needs cycle detection at enqueue, a decision about whether a failed dependency cancels or blocks its dependents forever, and a way to reason about a dependency that is itself retrying. Out of scope because a partial implementation of dependencies is worse than none: it would look like it worked in the common case where the dependency happened to finish first.


What I would do differently with more time

Verify against a browser and a real database far earlier. Sessions 6 and 7 built the whole frontend against green typecheck, lint, build and 241 passing tests, and the first browser pass found a crash on three screens, a chart rendering as two coloured blocks, and a double scrollbar on every page. The first SELECT against the deployed database found ADR-018 — a retry system that had never worked. None of those needed a sophisticated tool; they needed opening the app and querying the data, which are the two cheapest checks available and were the two left until last.

Treat test fixtures with the same suspicion as production code. Three separate bugs shipped behind a passing suite because a helper stood in for behaviour that did not exist (ADR-018 collects them). I would add one review question — "does production have an equivalent of this helper?" — and apply it to every UPDATE inside a test.

Write the frontend's one E2E test. TESTING_RULES.md allocates a single Playwright test (login → create queue → enqueue → see it complete) and it does not exist. That one test would have caught the Radix Slot crash on its first run and would have caught ADR-018 too, because a job that never completes fails the last assertion. It is the highest-value test in the plan and it is the one that was skipped.

Put a stated invariant behind an assertion when the database will not. attempt <= max_attempts is load-bearing — it decides DLQ versus retry — and was enforced only by code that happened to be correct. The test now exists, but it was written in response to a report rather than alongside the refund logic that makes it non-trivial.

Be more sceptical of my own documentation. DATABASE.md asserted a performance improvement nobody had measured (ADR-015), CONCURRENCY.md cited a constraint that had been replaced (ADR-005), and lifecycle.ts documented a transition that was never implemented (ADR-018). In each case the prose was confident and wrong, and confident prose is harder to doubt than code. Generating the numbers (pnpm db:explain) rather than transcribing them is the pattern that worked; it should have been applied to behaviour as well as to timings.