scheduler: claim a run when dequeuing it, not after
dequeue-run selected a pending run and returned it. The row stayed pending until the goroutine run-loop spawns wrote starting, so everything between those two points was a window in which the same row was dispatchable again -- and count-active-runs, which counts starting/running, could not see the dispatch either, so max_concurrent did not bound it.
The cooperative scheduler makes that window arbitrarily wide. sigil-sqlite forces busy_timeout=0 in its native layer and retries in Scheme with a sleep, which SUSPENDS the fiber. So while any other connection holds the SQLite write lock, the goroutine's claim-write suspends, the drain loop resumes, re-selects the same pending row, and spawns another goroutine for it. No backoff, no bound.
MEASURED, with the mechanism isolated from anything that found it: six queued runs, max_concurrent: 2, and one external connection holding the write lock for eight seconds via BEGIN IMMEDIATE.
before 66 x Starting node run=5 controller alive: NO
1 x Starting node run=1..4 health: 000
after 1 x Starting node run=1..4 controller alive: yes, health 200The same measurement at a twenty-second hold -- past sigil-sqlite's five-second busy budget, so the claim itself raises rather than waiting -- also stays at one dispatch per run with the controller alive: the raise is caught by run-loop's guard, logged, and retried on the next one-second tick, which is a bounded retry rather than a spin.
This is not a test-only fault. Any second writer on the kiln database -- a kiln CLI subcommand that writes, a backup, a maintenance script, or a slow write on a loaded controller -- can livelock the live controller and kill it. It has been latent since the run loop was written; kiln's own CI surfaced it on the first outing of a gate that seeds 700 rows.
THE LOAD-BEARING PROPERTY IS NOT THE SQL, IT IS WHERE THE CLAIM HAPPENS. UPDATE ... WHERE status = 'pending' RETURNING id is one statement so that no second statement's changes() can be clobbered by another fiber, which is worth having; but what closes the window is that the claim completes in the DEQUEUING fiber before the goroutine is spawned. Any suspension inside the goroutine -- a DB lock, worker provisioning, an SSH round trip -- is now harmless, not just this one.
run-loop's existing comment already described this shape and guarded only the EXCEPTION route ("If run-job! crashed before marking the run terminal ... Otherwise the run stays 'pending', gets re-dequeued, and respawns in a tight CPU spin"). The route found here raises nothing, which is why 247 log lines of the dying controller held zero ERROR lines.
No new stranding class: a controller that dies between the claim and the execution leaves the row starting, and init-database! already errors orphaned starting rows on the next startup, for exactly this reason.
The regression test asks the question the drain loop asks -- can a second dequeue-run hand back the same run -- rather than asserting the row's new status, because the second call is what actually went wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0183EGLt1MaPpvhHCSZD4ma9
src/kiln/db.sgl | 41 +++++++++++++++++++++++++++++++++++++++++
src/kiln/scheduler.sgl | 24 ++++++++++++++++++++++--
test/test-scheduler.sgl | 34 ++++++++++++++++++++++++++++++++++
3 files changed, 97 insertions(+), 2 deletions(-)src/kiln/db.sglmodified
db-get-run-by-name db-list-runs db-update-run-status! db-claim-pending-run! db-update-run-server! db-next-run-number db-runs-for-sha (else (sqlite-run db "UPDATE runs SET status = ? WHERE id = ?" status id)))) ;;; CLAIM a pending run, atomically. #t if this caller got it. ;;; ;;; WHY THIS IS ONE STATEMENT AND NOT A READ FOLLOWED BY A WRITE. The ;;; dispatcher used to select a pending run and leave it `pending` until ;;; the goroutine it spawned wrote `starting`. Everything between those two ;;; points is a window in which the run is dispatchable again, and the ;;; cooperative scheduler makes that window arbitrarily wide: the status ;;; write goes through sigil-sqlite's busy retry, which SLEEPS -- suspends ;;; the fiber -- whenever another connection holds the SQLite write lock. ;;; The drain loop then resumes, re-selects the same still-`pending` row, ;;; and spawns another goroutine for it, with no backoff and no bound. ;;; ;;; Measured on 2026-09-01: an external writer holding the write lock for ;;; eight seconds produced 66 dispatches of one run against one dispatch ;;; each for its five neighbours, under `max_concurrent: 2`, and the ;;; controller was dead by the end of it. See ;;; [[topics/kiln-dequeue-claim-race]]. ;;; ;;; `WHERE status = 'pending'` is what makes it a claim rather than a ;;; write: the row moves out of the dispatchable set in the same statement ;;; that decides it should. RETURNING reports whether this caller was the ;;; one that moved it, without a second statement whose `changes()` another ;;; fiber could have clobbered in between. ;;; ;;; It stamps `started_at` exactly as `db-update-run-status!` does for ;;; `starting`, so a claimed run is indistinguishable from one the runner ;;; had already begun -- which it is about to be. A controller that dies ;;; between the claim and the execution leaves the row `starting`, and ;;; `init-database!` already errors those on the next startup for exactly ;;; this reason; this adds no new stranding class. (define (db-claim-pending-run! db id) (let ((row (sqlite-query-row db "UPDATE runs SET status = 'starting', started_at = COALESCE(started_at, datetime('now')) WHERE id = ? AND status = 'pending' RETURNING id" id))) (and row #t))) ;;; Update a run's Hetzner server ID. (define (db-update-run-server! db id server-id) (sqlite-run db "UPDATE runs SET server_id = ? WHERE id = ?" server-id id))src/kiln/scheduler.sglmodified
;; Queue Management ;; ============================================================ ;;; Get the next pending run to execute. ;;; Get the next pending run to execute, AND CLAIM IT. ;;; ;;; Returns the run record or #f if no runs are pending or the ;;; concurrency limit is reached. Group-aware: a pending node run is ;;; skipped (and the next candidate considered) when its group already ;;; has `max-concurrent` node runs active, so one big group can't ;;; starve everything while still honoring the global cap. ;;; ;;; IT CLAIMS, AND THAT IS PART OF THE CONTRACT, NOT AN OPTIMISATION. ;;; A run this returns has already moved to `starting`; a second call ;;; cannot return it, and `count-active-runs` counts it from this moment ;;; rather than from whenever the spawned goroutine gets around to writing ;;; its status. Selecting without claiming left a window in which the same ;;; `pending` row was dispatched repeatedly -- unbounded, with no backoff, ;;; and past `max-concurrent`, because the concurrency count could not see ;;; the dispatches either. Full mechanism and the measurement in ;;; `db-claim-pending-run!`. ;;; ;;; The RECORD returned still reads `status = "pending"`: it is the row as ;;; it was selected, and callers use its id / group_id / plan_form, never ;;; its status. Returning the pre-claim row keeps the claim invisible to ;;; everything except the queue. (define (dequeue-run db max-concurrent) (let ((active (count-active-runs db))) (if (>= active max-concurrent) (>= (db-active-node-count-for-group db gid) max-concurrent)) (loop (cdr cs)) run))))))))) ;; A candidate that is no longer `pending` by the time ;; we get here was taken by someone else (or cancelled ;; mid-scan): move on rather than dispatch it. (if (db-claim-pending-run! db (assoc-ref 'id run)) run (loop (cdr cs)))))))))))) ;;; Count the number of currently running jobs (node runs included). (define (count-active-runs db)test/test-scheduler.sglmodified
(let ((run (dequeue-run db 3))) (assert-true (pair? run)) ;; The RECORD is the row as it was selected, so it still reads ;; "pending". The DATABASE does not -- see the claim test below. (assert-equal "pending" (assoc-ref 'status run)) (assert-equal "sigil-linux" (assoc-ref 'job_name run))) (sqlite-close db))) ;; THE REGRESSION TEST FOR THE DISPATCH RACE. ;; ;; Before `dequeue-run` claimed, this second call returned the SAME run: the ;; row stayed `pending` until the goroutine the dispatcher spawned got round ;; to writing `starting`, and under write-lock contention that write suspends ;; the fiber and lets the drain loop dispatch the row again -- 66 times for ;; one run in the measurement that found this, ending with a dead controller. ;; ;; It is written as "the second call must not hand back the first run" ;; rather than "the row is now starting", because the second call is what the ;; drain loop actually does and what actually went wrong. (test "claims the run, so a second dequeue cannot return it" (let ((db (make-test-env))) (db-create-job! db "sigil-linux" "codeberg/daviwil/sigil" "main") (write-job-script! "sigil-linux.run" "#!/bin/sh\necho test\n") (schedule-runs! db test-jobs-dir "codeberg/daviwil/sigil" "refs/heads/main" "sha-claim" 'push #f #f) (let ((first (dequeue-run db 3))) (assert-true (pair? first)) ;; The claim is visible in the database, with started_at stamped ;; exactly as the `starting` transition stamps it. (let ((row (db-get-run db (assoc-ref 'id first)))) (assert-equal "starting" (assoc-ref 'status row)) (assert-true (string? (assoc-ref 'started_at row)))) ;; And the queue no longer offers it. With max-concurrent 3 the ;; concurrency cap is nowhere near binding, so a #f here is the claim ;; talking and nothing else. (assert-false (dequeue-run db 3))) (sqlite-close db))) (test "returns #f when at concurrency limit" (let ((db (make-test-env))) (db-create-job! db "sigil-linux" "codeberg/daviwil/sigil" "main")