chadtmiller.com/knotless

Switch repository

Loading…

What changes

A stop hands over. On SIGTERM, knotless-run drops each run not started that it will not run, as before. It now keeps two kinds to run itself: a merge-queue batch's runs, and those at a running deploy's commit. Once every waiting run is dropped or kept, it writes its state file a last time. From then on it writes a note of its own under <state>.handover/ instead, every 2 seconds and on each change. The note lists its runs going, the repositories it is checking a batch of, and the passes, runs dropped, workflows asked for again and step time it has kept since. Meanwhile its caches hold still: they make no base and remove none. Its discovery, schedules and container sweeps stop.

The next runner starts beside it. A runner that finds a fresh note at its start skips the leftover container and overlay cleanup, and pins every cache base on disk. It also:

• leaves the old runner's checks out of recover;
• lends it one job per run it has going, so together they never exceed --jobs;
• lands no batch of a repository the old one is checking;
• takes in the passes and minutes the note records.

Once the note says done, or goes 20 s without a write, the new runner takes in the step time, removes the note, gives the jobs back, unpins the bases and rings every door. Turns::lend, Caches::pin/unpin/hold_still and crates/knotless-run/src/handover.rs are the new pieces.

Deploys use it. deploy/handover.sh replaces the stop-then-start restart. It stops docker restarting the leading runner and sends it SIGTERM. Once a new note appears, it runs docker compose up --scale run=N+1 --no-recreate, and removes the old container in the background once it exits. A runner that writes no note within 60 s, one built before this, is stopped first as before. deploy.sh compares only the leading runner, the newest one docker still restarts.

Why

Every restart cancelled the waiting runs. Each one could also cut off a batch, which then had to be checked again from scratch. While the old runner waited for its slowest run, nothing new started on prod's two jobs. On 2026-11-03, runner restarts cancelled 44 batch checks, and queued→landed p95 reached 230 min. A cheaper restart fixes this, and so does restarting less often (the fingerprint pull). This pull makes the restart cheap. The other options were adopting the old runner's containers mid-step, which isn't possible since steps are docker exec sessions the runner owns, and sharing one state file between two runners. A note with a single writer, merged by the runner that reads it, avoids both problems.

How it was checked

New unit tests:

• handover::tests: what a note records since the handover, what is taken in and when, and when a note counts as gone.
• turns::tests::lent_jobs_are_not_taken.
• caches::tests::pinned_bases_stay_until_unpinned, including holding still.

New integration test a_stop_hands_over_to_the_next_runner: the new runner leaves the old one's running check alone and runs nothing in the lent job. It runs the dropped run once the old one is done, and the note goes. I updated a_stop_drops_what_has_not_started: the run kept for the deploy now runs once the deploy ends instead of being dropped. The unit tests ran here; the integration tests and coverage are left to CI. handover.sh was only checked with sh -n. Its first real run is the first deploy after this lands, which falls back to the old restart because the running runner predates notes. The deploy after that is the first real handover.

Shortcomings

• While the old runner finishes a batch, the new one builds no batch of that repository, and with --jobs 2 it may get no job until the old one's runs end. That is what keeps memory within the host.
• A runner killed without writing done leaves its checks running until the next start ends them, as before.
• The trusted pull's cache layers a batch writes while handing over are not kept. A push that lands it still takes its checks, but warms its caches again.

Review

The design, then Runner::stop, dropping, heed and the start-up branch in start_as in crates/knotless-run/src/lib.rs, and deploy/handover.sh.

🤖 Generated with Claude Code

This pull makes a stopping runner hand over to the next one through a single-writer note. It keeps batch and deploy-commit runs, lends jobs and pins cache bases, and deploy/handover.sh drives the whole thing. The design is sound and the lock order looks consistent (state, then handing, then going). I read all of lib.rs, handover.rs, turns.rs, caches.rs and the deploy scripts. I skipped the tests, docs and changeset files. check and test were still queued or running, so I have no check results.

Must fix: work done after the handover can be lost. stop waits for deciding to reach 0, then hand_over writes the state file a last time. After that, save writes only into the note, and the note carries only passed and used (see handover::since). A reconcile already in flight when stopping was set is not stopped. reconcile checks stopping only on entry (lib.rs:1337). It can still create jobs after hand_over. Those jobs see stopping and are dropped, and drop_run records state.dropped and ask_again records state.wanted through save. Both land in a note that never carries them, so the next runner never learns of that run, and the push or pull is never checked. Before this change those writes reached the state file. Either make reconcile and job creation bail out once stopping is set (and re-check right before Deciding::new), or have the note carry dropped and wanted and have take merge them.

Minor: bases can stay pinned. start_as decides beside and calls pin(). heed then unpins only when was is non-empty and before becomes empty. If the old note goes stale or done between the two reads, was is empty, so unpin never runs. The bases then stay pinned until the next restart. Track "pinned" explicitly and unpin when no live notes remain.

Minor: over-lending. heed lends note.going.len(), which counts kept runs still waiting for a turn on the old runner, not only runs holding a job. The new runner may therefore sit idle longer than needed. This is safe, so it is only worth noting.

The earlier-described limits (a runner killed without writing done, a trusted pull's cache layers lost) are acknowledged in the body and fine.

Fixed in c69e9d913d7e. On the first point: stop takes the reconciling write lock first, which waits for any reconcile in flight. So every job that reconcile spawns is counted in deciding before the handover, and no drop comes after it from that path. The real hole was ask_again. A run asked for by name is cancelled at the stop, and one cut off past --stop-wait is cancelled later. Either one finishes after the handover, so its re-ask went into a note without wanted, which CI's a_workflow_asked_for_by_name caught. The note now carries wanted and dropped, and take takes each in once, skipping what the note said before, so one run since is not brought back. Second point: heed now unpins whenever no live note is left and bases are pinned, and rings every door when it takes anything in. Third: lending for the kept runs still waiting is deliberate. They take the old runner's jobs as its started runs end, and lending for them is what keeps the two within --jobs. A comment there now says so.

Both fixes I asked for last round are in, so I approve.

• Work lost after the handover: the note now carries wanted and dropped as well as passed and used. handover::since writes them and take merges them with the seen de-duplication. A run dropped or asked again after hand_over therefore reaches the next runner. The unit test covers both.
• Bases staying pinned: heed now unpins whenever no live notes remain and caches.pinned() is true. It no longer depends on was being non-empty.
• Over-lending: heed still lends going.len(), which counts kept runs waiting for a turn on the old runner. It is capped at --jobs, and the failure is only idleness, not exceeding the limit. I'm leaving it alone.

Every check on this head failed with the same error: it conflicts with main in deploy/README.md. No test or build ran, so merge main into the branch and make sure the checks pass before landing. The pull itself notes that the integration test a_stop_hands_over_to_the_next_runner was left to CI.

This head only merges main into c69e9d9, the commit I approved last round. The merge fixes the deploy/README.md conflict that had failed every check. I didn't re-read the code. The pull is already queued, and check, test and docs are still queued, so no test results exist yet. The integration test a_stop_hands_over_to_the_next_runner is left to CI, so the merge queue's checks need to pass before it lands. My earlier asks (the note carrying wanted and dropped, and unpinning whenever no live note remains) were done, and I'm not asking for anything more.

This head adds one commit on top of the merge I approved last round. It is coverage work plus a small refactor, and I found nothing to change.

• caches::runs_in is the directory listing moved out of start_as so a test can reach it. The behaviour is the same.
• their_batches is a separate set that heed rewrites on every read, in the same place that lends jobs. checking_theirs now reads that set, so it no longer takes the before lock. It empties when no live note remains, so a batch is not blocked after the old runner is done.
• The new quietly test only adds coverage.

The pull is already queued, and check, test and docs are the gate, including the integration test a_stop_hands_over_to_the_next_runner that the author left to CI. My earlier asks were done, and I'm asking for nothing more.

This head merges main into 1eba813, which adds one small commit on top of 753372e. I approved 753372e last round. I found nothing to change.

• Caches::pinned() is removed. unpin() now takes the pinned set and sweeps only if it was non-empty, so calling it with nothing pinned does nothing.
• heed calls unpin() whenever no live note remains, which removes the check-then-act gap on pinned(). The test drops its pinned() assertions, and unpin is still covered through theirs.exists().

On this head, check, test and docs were still queued, so no results exist yet. The integration test a_stop_hands_over_to_the_next_runner has not run in CI, so the merge queue's checks need to pass before this lands. My earlier asks were done, and I'm asking for nothing more.