fix: complete the keys a killed sibling held, and stop waiting to find out it is dead

[sotashimozono]

A job that follows one the wall clock killed cannot complete the campaign, does not report that it failed to, and when it eventually can, waits ten minutes for permission.

This PR was reviewed after being declared ready, and was not ready. A multi-agent review found three Critical defects, all in the direction the section below originally claimed was impossible. They are fixed, each with a regression test, and the review is summarised at the bottom. Read that section before the rest.

---

1. run_loop! abandoned keys a killed sibling held

acquire_running! already reclaims a lock whose heartbeat is older than stale_after, so the stack can recover from a master killed mid-key: something just has to keep trying until the lock expires.

run_loop! stopped before that. Its exit test was max_empty_rounds rounds with done == 0, and a round where every remaining key came back :lock_busy counted as empty. With the defaults that is 90 s against a stale_after of 600 s.

the tail of a campaign, one key held by a killed master	before	after
run_loop! returned after	5.3 s	62.1 s
work_fn ran on the key	no	yes
key done at exit	NO	yes

A busy round no longer counts toward max_empty_rounds until stale_after has been waited out. busy is now in the return, so "finished" is distinguishable from "someone else still has work out". Bounded by stale_after + 2 * idle_sleep, so a sibling that keeps refreshing is not waited on forever.

The cost, stated: holding an allocation up to 11 min instead of 90 s when a live sibling holds the remainder. Exiting early and having the sibling then die leaves those keys with nobody to finish them.

2. Ask whether the holder is gone, instead of waiting

holder_liveness(owner) -> :alive | :dead | :unknown

A key whose holder is a dead pid on this host completes in under 30 s with stale_after = 600.0.

source	reach
/proc/<pid>	the holder is on this host. A recycled pid reads :alive, the safe side.
squeue	across hosts, when the holder carries a Slurm job id and this process is itself inside an allocation and the queue can see this process

:slurm is one of four worker modes, so none of this is load-bearing. Layer 1 completes the campaign on its own in all four; this only removes the wait.

SweepRunner also now actually uses DataVault 0.8.1's owner-stamped locks (it had been calling the two-argument forms), so the heartbeat's loss detection is no longer best-effort. Compat floor moves to 0.8.1.

3. A defect layer 2 surfaced

_run_affinity! gave a key back on ProcessExitedException with no bound, where the pmap path it replaced uses ExponentialBackOff(; n=2). Invisible while a dead holder's lock sat until stale_after; reclaiming immediately turned it into a cascade. Measured on a key that kills every worker: 3 deaths, err = 1, other 23 keys still complete.

---

Review findings

Five specialised agents plus an independent pass. Every agent that looked at Liveness.jl reproduced at least one Critical.

Critical: three ways :dead was answered for a live master

The original body claimed ":dead is returned only on positive evidence. Every error path, unparseable token, and un-askable holder is :unknown." That was false three ways.

#	condition	why it answered :dead
1	hostname begins with slurm	the Slurm field was found by scanning every token field, so slurm-node-01:12345:ab01cd23 had -node-01 read out of its hostname as a job id, found absent, and returned :dead without reaching the pid check. Clusters name nodes slurm* routinely.
2	any Slurm array task but the first	Slurm gives each task its own raw SLURM_JOB_ID (36, 37, 38 …) while squeue -o %i prints <ARRAY_JOB_ID>_<TASK_ID> (36_0, 36_1 …). Stamping the raw id made every task unfindable.
3	squeue pointed at another cluster	it answers successfully and lists none of our ids, so every holder was absent.

Fixes: the Slurm field is read only at position 4 (where owner_token writes it); owner_token stamps the id the queue prints; and the queue must be shown to see this process before its silence about anyone else counts.

#2 was invisible to the test suite by construction. The test hand-wrote the token with the ARRAY id, sharing the implementation's own assumption about what SLURM_JOB_ID holds. An oracle that shares the convention under test cannot see it.

Important

• _reap_if_dead! was fatal. An unlink that failed escaped run! and took the whole round. Measured: one lock in a read-only directory, run! threw, 0 of 3 healthy keys attempted; now returns with 3 done. Reaping is an optimisation over stale_after; nothing in it may be fatal.
• squeue had no timeout, on the hot path of every contended key, holding the cache lock, where the binary can be an SSH wrapper. Bounded to 10 s, killed, then :unknown.
• :gave_up acquired a second source independent of opts.max_attempts, contradicting three docstrings. Split out as :worker_died.
• One bound, two mechanisms. ExponentialBackOff(; n=2) and a hand counter. Now one _WORKER_DEATH_REDISPATCHES feeds both, since expressing one bound twice is how the unbounded give-back got in.
• owner_token did not say to call it fresh. Exported and in api.md; a caller caching it per process defeats the nonce.
• Four .cov artifacts (528 lines) committed by a git add -A. Removed, .gitignore updated.
• Comments narrating how a trap was found, rather than the trap, cut. One named a contributor's machine.

Test gaps found, now closed

• The real fetch-and-parse had no value-level test. Every testset seeded the cache, so the parser never ran; the one "real fetch" test asserted only that the result was nothing or a Set, which an always-nothing stub satisfies. Now exercised against the shapes squeue -o %i actually prints, via a fake squeue on PATH.
• That removes a live network call from CI. The squeue on the machine this was written on is exec ssh -o BatchMode=yes <front> -- squeue, and CI runs on that machine.
• :lock_reaped was emitted but never asserted.
• Slurm evidence outranking the pid on the same host was never exercised; every Slurm test used a foreign hostname, so the branches were only tested apart.
• test_eventlog.jl's enumerated-kinds contract was missing four kinds.
• The deaths bound asserted a range where the count is structurally exact.

Round 2 — a second review, of this PR

Two more findings from reviewing the published 0.6.2, plus one this PR's own fix exposed. All three
are a control answering for a condition it was not measuring.

Prerequisite.opts replaced the caller's RunOpts wholesale

deadline defaults to nothing, so a prerequisite built for the documented reason — raising
stale_after, because the shared setup is the slow half — ran with no deadline at all. The
barrier then waits for a sibling past the end of the allocation running it, which is exactly the
failure a deadline exists to prevent.

stop_flag and deadline are now inherited from the caller when the prerequisite leaves them
unset, and still take precedence when it sets them. They bound the job, not the stage.

stopped_by was read off the clock at return time

In run! and again in run_loop!. That answers "is a stop condition true now", not "why did
this stop", and the two diverge in both directions:

	reported	actually
loop sleeps idle_sleep between rounds, crosses the deadline, then gives up on max_empty_rounds	:deadline — retryable	a key that cannot be produced, and will fail again next allocation
flag file removed after the stop was observed	nothing	:flag

The reason now travels back with the outcome that carried it (:stop_flag / :stop_deadline) and
is recorded where the stop happened. run!'s docstring promised this already: "so a short stage is
attributable without re-reading the clock".

Exposed by that fix: the two dispatchers disagreed on what a stop costs

The sequential path breaks on a stop and emitted no outcome for the keys it dropped, while
pmap hands every remaining key back as stopped. The same stop reported a different stop count
depending on which dispatcher ran. Both now attribute them, and done + stop + err + busy == length(keys) holds on either path.

The recipe the docs recommend had never been run

Prerequisite's docstring says to build keys with ParamIO.project "so the two spaces cannot
drift apart by hand". Every test passed DataVault.keys(prep) instead — the hand-written
projection that is prep.toml. The recipe now runs end to end, with the fixture as its oracle and
a control that grows the sweep's axis to show the projection follows where the hand-written file
does not.

Round 3 — a review of round 2

Round 2's fix reopened the failure it was named for, from the other side.

Critical: a stage could still loosen a bound the job set

_merged_opts inherited stop_flag/deadline when the Prerequisite left them unset, but let an
explicitly set one win outright. A deadline is not a stage preference: it names when Slurm kills
this process tree. A Prerequisite built once with a generous ceiling therefore discarded a caller's
tighter, freshly computed one, and run_prerequisite!'s busy-wait is unbounded except by that
deadline.

Round 2's own labelled "control" asserted this as correct:

p2 = Prerequisite(work, prep, keys; opts=RunOpts(deadline=time() + 3600))
r2 = run_prerequisite!(p2; opts=RunOpts(deadline=time() - 1), poll=0.01)
@test r2.complete == true          # caller's deadline ALREADY EXPIRED, barrier runs anyway

It now takes the tighter of the two, and that test asserts the opposite.

Important: === nothing is not "unset" for stop_flag

RunOpts resolves its default from ENV["SWEEPRUNNER_STOP_FLAG"], which the shipped
submit_slurm.sh exports for the whole job. So a Prerequisite built for stale_after alone carries
a flag it never asked for, and that ambient value outranked the flag the campaign was actually
configured with. An operator raising the one they know about would never be seen by the barrier.
The caller's now governs whenever it has one.

Invisible in CI, which does not set that variable, and only reachable in the deployment this
feature exists for.

complete=false beside remaining=0

run_prerequisite! hardcoded complete=false the moment a stop was observed, computing remaining
on the same line. A setup finished by an earlier run reported a self-contradictory tuple, and
run_loop! gates the dependent stage on exactly that field: a fully provisioned barrier refused to
let the work start.

Also

• _merged_opts rebuilds RunOpts by name over fieldnames, so a field added later cannot be
  silently reset to its constructor default.
• The :flag/:deadline to :stop_flag/:stop_deadline map was two hand-written ternaries with a
  third site re-implementing the precedence rule. It is written once in _stop_outcome and
  throws on an unmapped reason instead of relabelling it as a deadline.
• _is_stopped is removed, having become dead.
• The two new wall-clock margins are an order of magnitude wider than the work they bound, since CI
  here is self-hosted and shares the box.

mutation	result
explicit deadline wins again	RED
prerequisite stop_flag wins again	RED
complete hardcoded false again	RED
drop the round's own reason at the no-progress exit	RED
control, unmutated	GREEN

Not changed, with reasons: run_loop! keeps stopped_by as "why the loop exited" rather than
"was a stop ever observed" (a flag raised and then cleared should not tell a resubmitter to stop);
and the worker-death vs deterministic-failure conflation in the stopped_by=nothing bucket is a
pre-existing design question, not a patch-release change.

Verification

24 of 24 files under test/ pass, judged by exit code. Round 2's three fixes were then reverted individually in a disposable copy: each time the intended testset goes red and the unmutated control stays green. Three times during this work a grep over test output reported green where the run was red, or red where it was expected noise; the suite is now judged by whether Julia exits non-zero.

Patch bump, 0.6.2 -> 0.6.3.

🤖 Generated with Claude Code

[github-actions]

📚 Docs preview: https://qatlashub.github.io/SweepRunner.jl/previews/PR52/

(updates on each push to this PR)

[codecov]

Codecov Report

❌ Patch coverage is 93.49593% with 8 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/Liveness.jl	92.72%	4 Missing ⚠️
src/Run.jl	93.33%	4 Missing ⚠️

📢 Thoughts on this report? Let us know!