From 88ab063b64770121c6491ae792b84990a18d200e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 23:09:35 -0500 Subject: [PATCH 1/5] fix(prune-cache): close the reader-marker check-then-delete window MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both eviction sites checked for a consumer's `.reading-` marker and then, seconds later — `usage_gb` runs `du -sk` over a multi-GB tree between the two — unlinked the directory. A consumer that started a clone inside that gap had its source removed mid-walk, which `cp -al` does not report: a subtree unlinked before its parent is listed is silently omitted. Unreachable today, and only by policy: snapshots belong to protected refs and protected refs never reach the marker check. `cargo-cache` and `cargo-cache-publish` take that ref list as two independent inputs, so a workflow listing a publisher in one and not the other arms this with no code change at all. Closed structurally, with publish-snapshot.sh's rotation rather than a new mechanism: the candidate is renamed aside and only then re-examined, so the scan the unlink rests on happens strictly after the rename. A consumer that resolved the directory published its marker before that scan and cannot be missed; one arriving after cannot resolve the path and starts cold, the same degrade the publisher's swap window already produces. Renaming disturbs no clone in flight — no entry is unlinked and the inode is unchanged — so a declined eviction costs a deferred eviction and nothing else. A reprieved cache is put back under its own name; one whose name a concurrent seed has retaken is left aside and swept by a later pass once its readers drain, since nothing else globs a dotted name. prune-cache-selftest gains three scenarios (21 -> 31 assertions). Scenario 12 is the one that bites: the `du` the pass runs on its candidate publishes the marker, placing it strictly after the check and strictly before the unlink. Against check-then-delete, 1-11 pass and 12 fails; breaking only the second look and leaving the rename fails it too. --- README.md | 27 +++++--- scripts/prune-cache-selftest.sh | 65 +++++++++++++++++++- scripts/prune-cache.sh | 105 ++++++++++++++++++++++++++++---- 3 files changed, 177 insertions(+), 20 deletions(-) diff --git a/README.md b/README.md index 63bd0a7..2daa71c 100644 --- a/README.md +++ b/README.md @@ -165,7 +165,7 @@ mechanisms, both required: walk each: measured on ext4 with a warm cache over a 78,554-entry tree, 44 ms per walk against 3,126 ms for the `cp -al` they guard — about 2.8%. -**What this does and does not guarantee.** Three separate claims, deliberately +**What this does and does not guarantee.** Four separate claims, deliberately not collapsed into one: - **A publisher rotating a snapshot cannot tear a clone of it — by @@ -174,12 +174,20 @@ not collapsed into one: consumer that resolved the old generation, and on timeout it defers the unlink rather than forcing it. On this path the consumer's own verification is a redundant second check, not the thing holding the guarantee up. +- **Nor can the eviction pass — by the same construction.** A cache chosen for + eviction is renamed aside and only then re-examined for readers, so the scan + the unlink rests on happens strictly after that rename, exactly as the + publisher's does. A consumer that resolved the directory published its + marker before the scan and so cannot be missed; one arriving after the + rename cannot resolve the path at all and starts cold instead. A cache + claimed inside that window is put back under its own name, and one whose + name a concurrent seed has taken in the meantime is left aside and reclaimed + by a later pass once its readers drain. Until this was structural it was + merely policy — snapshots belong to protected refs, protected refs are never + eviction candidates — which is a property held by vigilance rather than by + construction. - **Every other way the source can change mid-clone is detected, not - prevented.** The eviction pass's reader check is check-then-delete, so a - consumer publishing its marker inside that gap is narrowed but not excluded - — unreachable today only because snapshots belong to protected refs and - protected refs are never eviction candidates, which is policy rather than - structure. A `seed-fallback-dir` pointing at a directory something else + prevented.** A `seed-fallback-dir` pointing at a directory something else writes has no interlock at all. There, the per-attempt verification is what stands between a torn read and a corrupt cache: the clone is retried (`CACHE_CLONE_ATTEMPTS`, default 4) and then **fails the job loudly** — @@ -190,7 +198,10 @@ not collapsed into one: outright holds it until its marker passes `reader-stale-seconds`. The residual is capped at one deferred generation per publisher ref, and its real cost is close to inode count rather than byte count, since the - artifacts are hardlinked to whatever cloned them. + artifacts are hardlinked to whatever cloned them. A declined eviction is + bounded the same way: the cache stays, either under its own name or aside + awaiting the next pass, and is never unlinked under the job that claimed + it. **Eviction** runs three passes: caches for branches that no longer exist on origin are removed unconditionally; then, only if free space is under the @@ -308,7 +319,7 @@ bash scripts/selftest.sh --fast # fixture-only suites, no compiler | `hardlink-clone-selftest.sh` | that a build in a clone cannot mutate its source — with a control proving a raw `cp -al` does. Needs a real compiler. | | `seed-target-dir-selftest.sh` | seed-source preference, lock-file stripping, two jobs racing on one cache key, **and a seed racing a publisher's rotation of the source it is reading** — the race that actually truncates a tree | | `publish-snapshot-selftest.sh` | the atomic swap, that a live consumer survives a republish, and the publisher's side of the rotation race: deferred reclamation under a live reader, and its sweep once the reader is gone | -| `prune-cache-selftest.sh` | liveness, protection, locking, eviction order, self-clear — against a real scratch `origin` | +| `prune-cache-selftest.sh` | liveness, protection, locking, eviction order, self-clear, **and that a cache a job claims *inside* the check-to-unlink window survives it** — against a real scratch `origin` | | `restore-mtimes-selftest.sh` | the merge hazard and the watermark that closes it, including the two-jobs-one-namespace case. Needs a real compiler. | Every suite runs the actual script, not a reimplementation of its logic, and diff --git a/scripts/prune-cache-selftest.sh b/scripts/prune-cache-selftest.sh index d95fd10..8c66e31 100755 --- a/scripts/prune-cache-selftest.sh +++ b/scripts/prune-cache-selftest.sh @@ -29,6 +29,15 @@ # 9. OWN CACHE NEVER EVICTED by a sibling pass. # 10. SCOPED TO THE CACHE ROOT — a decoy outside it (standing in for another # project's volume) is never touched. +# 11. A LIVE READER MARKER PROTECTS A CACHE the same way a lock file does — a +# directory somebody is hardlink-cloning this instant is not a candidate, +# however dead and however tight the disk. +# 12. AND SO DOES ONE PUBLISHED INSIDE THE CHECK-TO-UNLINK WINDOW, which is +# the property a check-then-delete eviction does NOT have. This is the +# one that fails against the pre-fix script. +# 13. A DEFERRED EVICTION IS RECLAIMED, but not while its reader is live. +# Nothing else globs a dotted name, so an unswept one is disk lost for +# good on the volume whose whole problem is disk. set -euo pipefail script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) . "$script_dir/cache-lib.sh" @@ -112,7 +121,7 @@ mk "snapshot-$LIVE" '2020-01-02' # evictable — which is what makes the ORDER the observable property here, not # what survives. Assert the eviction order directly from the log. run_prune "1000000 1000" -order=$(grep -o "evicting \(target\|snapshot\)-$LIVE" "$scratch/log" | sed "s/evicting //") +order=$(grep -o "evicted \(target\|snapshot\)-$LIVE" "$scratch/log" | sed "s/evicted //") [ "$(printf '%s\n' "$order" | head -1)" = "target-$LIVE" ] \ || fail "expected target-$LIVE to be evicted before snapshot-$LIVE, got: $order" ok "target dirs are evicted before snapshots" @@ -170,5 +179,59 @@ decoy="$scratch/other-project"; mkdir -p "$decoy/target-$DEAD"; touch "$decoy/ta run_prune "1000000 1000" assert_kept "$decoy/target-$DEAD" "a cache outside the cache root is never touched" +echo +echo "=== 11: a live reader marker protects a cache, like a lock file does ===" +reset_cache +date +%s > "$root/.reading-target-$DEAD-job1" +run_prune "1000000 1000" +assert_kept "$root/target-$DEAD" "a cache being hardlink-cloned right now survives both passes" +assert_log "currently cloning it" "the reader is named in the log, not silently honoured" +rm -f "$root/.reading-target-$DEAD-job1" + +echo +echo "=== 12: a reader marker published INSIDE the check-to-unlink window ===" +reset_cache +# A consumer publishes its marker whenever it starts a clone, which can be at +# any instant — including after the pass has checked for markers and before it +# unlinks. That window is real time, not a theoretical interleaving: `du -sk` +# on a multi-GB cache runs for seconds, and the pass runs one on every +# candidate. It is reproduced deterministically here by making that very `du` +# publish the marker, which places it strictly after the check and strictly +# before the unlink — exactly where a check-then-delete eviction cannot see +# it. The candidate must still be standing afterwards, with its contents +# intact and nothing left renamed aside. +mkdir -p "$scratch/bin" +real_du=$(command -v du) +cat > "$scratch/bin/du" < "$root/.reading-target-$DEAD-racer" ;; esac +done +exec "$real_du" "\$@" +EOF +chmod +x "$scratch/bin/du" +( PATH="$scratch/bin:$PATH"; run_prune "1000000 900000" ) +[ -e "$root/.reading-target-$DEAD-racer" ] || fail "the racing marker was never published — scenario 12 proves nothing" +assert_kept "$root/target-$DEAD" "a cache claimed inside the eviction window is not unlinked" +assert_kept "$root/target-$DEAD/blob" "the reprieved cache still has its contents" +assert_log "restored, not evicted" "the reprieve is reported, not silent" +[ -z "$(ls -d "$root"/.evicting-* 2>/dev/null)" ] || fail "an aside directory was left behind after the reprieve" +ok "nothing left renamed aside once the eviction is declined" +rm -f "$root/.reading-target-$DEAD-racer" + +echo +echo "=== 13: a deferred eviction is reclaimed, but not under a live reader ===" +reset_cache +aside="$root/.evicting-target-$DEAD-9999" +mkdir -p "$aside"; head -c 4096 /dev/zero > "$aside/blob" +date +%s > "$root/.reading-target-$DEAD-job1" +run_prune "1000000 900000" +assert_kept "$aside" "a deferred eviction is not reclaimed while a job is still reading it" +assert_log "deferring its reclamation again" "the continued deferral is reported" +rm -f "$root/.reading-target-$DEAD-job1" +run_prune "1000000 900000" +assert_gone "$aside" "a deferred eviction is reclaimed once its reader is gone" +assert_log "reclaiming deferred eviction" "the reclamation is reported" + echo echo "prune-cache-selftest: ${pass_count} assertions passed" diff --git a/scripts/prune-cache.sh b/scripts/prune-cache.sh index 7e6efa6..08e0ebd 100755 --- a/scripts/prune-cache.sh +++ b/scripts/prune-cache.sh @@ -57,6 +57,11 @@ # actually-still-running job that somehow exceeds the threshold is visible # in the log rather than silently losing its cache mid-build. # +# That exclusion is decided TWICE per eviction — once as the cheap filter +# that keeps a held directory out of the pass at all, and once after the +# directory has been renamed aside, which is the decision the unlink +# actually rests on. See evict_dir. +# # Liveness is resolved by `git ls-remote --heads origin`, wrapped in a # timeout. A directory name cannot be inverted back to a branch name (the # sanitiser is lossy and the disambiguating suffix is a one-way hash), so this @@ -93,34 +98,90 @@ is_protected() { return 1 } +# is_locked [name] +# +# `name` is the directory's own name for reporting and for the reader-marker +# lookup, which matters when has been renamed aside for eviction: the +# markers a consumer publishes are keyed on the name it resolved, not on +# whatever the eviction pass has since called the directory. is_locked() { - local dir="$1" now lock_file lock_age locked=1 readers + local dir="$1" name="${2:-$(basename "$1")}" now lock_file lock_age locked=1 readers now=$(date +%s) # A directory being hardlink-cloned right now carries no .ci-lock-* of its # own — a snapshot has its locks stripped by construction — so the reader # markers are the only signal that unlinking it would truncate somebody's # in-flight clone. Today no reachable configuration prunes a snapshot (only # protected refs publish them, and protected refs are excluded from every - # pass), which makes this guard redundant *by policy*. It is here so that - # stops being the reason it is safe. - readers=$(live_reader_count "$ROOT" "$(basename "$dir")") + # pass), which makes this guard redundant *by policy* — evict_dir's second + # look is what stops policy being the reason it is safe. + readers=$(live_reader_count "$ROOT" "$name") if [ "$readers" -gt 0 ]; then - echo " $(basename "$dir"): ${readers} job(s) currently cloning it — not a candidate" + echo " ${name}: ${readers} job(s) currently cloning it — not a candidate" locked=0 fi for lock_file in "$dir"/.ci-lock-*; do [ -e "$lock_file" ] || continue lock_age=$(( now - $(stat -c '%Y' "$lock_file") )) if [ "$lock_age" -lt "$STALE_LOCK_SECONDS" ]; then - echo " $(basename "$dir"): held open by $(basename "$lock_file") (${lock_age}s old)" + echo " ${name}: held open by $(basename "$lock_file") (${lock_age}s old)" locked=0 else - echo " $(basename "$dir"): ignoring stale lock $(basename "$lock_file") (${lock_age}s old > ${STALE_LOCK_SECONDS}s) — treating as abandoned" + echo " ${name}: ignoring stale lock $(basename "$lock_file") (${lock_age}s old > ${STALE_LOCK_SECONDS}s) — treating as abandoned" fi done return "$locked" } +# evict_dir +# +# Unlinks , or declines to and says why. Returns 0 only if the directory +# is actually gone. +# +# The caller has already established that is a candidate, which is not +# the same as establishing that it is still one at the instant of the unlink: +# a consumer publishes its reader marker whenever it starts a clone, and the +# `du` that measures the directory between those two points runs for seconds +# on a multi-GB tree. Unlinking under a live clone truncates it silently — +# `cp -al` never reports a subtree that was removed before it read the +# parent's listing (see cache-lib.sh's reader-marker section). +# +# So the directory is renamed aside first and only then re-examined, which is +# what makes the second look conclusive rather than merely closer to the +# unlink. It is publish-snapshot.sh's rotation, and it rests on the same +# ordering proof: a consumer publishes its marker BEFORE it resolves the +# source path, so one that resolved this directory did so before the rename +# and therefore published its marker before the scan below, which happens +# strictly after that rename. A consumer arriving after the rename cannot +# resolve the path at all and falls through to its own cold-start path — the +# same safe degrade the publisher's swap window already produces. +# +# The rename disturbs nothing already in flight: it unlinks no entry and +# leaves the source inode unchanged, which is precisely why the publish side +# can rotate a snapshot out from under a live reader. A declined eviction +# therefore costs a deferred eviction and nothing else. +evict_dir() { + local dir="$1" name aside + name=$(basename "$dir") + aside="${ROOT}/.evicting-${name}-$$" + + mv -T "$dir" "$aside" 2>/dev/null || { + echo " ${name}: could not be set aside for eviction — skipped this pass" + return 1 + } + + if is_locked "$aside" "$name"; then + if mv -T "$aside" "$dir" 2>/dev/null; then + echo " ${name}: claimed by a job while its eviction was in flight — restored, not evicted" + else + echo "::warning::prune: ${name} was claimed mid-eviction and its own name is occupied again — leaving $(basename "$aside") for a later pass to reclaim once its readers drain" + fi + return 1 + fi + + rm -rf "$aside" + return 0 +} + # Directories oldest-first, target-* before snapshot-* (see the header). # The sort key is a rank digit followed by a zero-padded mtime, so the two # groups sort as blocks rather than interleaving by age. `.cache-last-used` @@ -141,6 +202,28 @@ list_by_lru() { done | sort | cut -f2- } +# Deferred reclamations from an earlier pass: a directory renamed aside for +# eviction that could not be unlinked, because a job claimed it inside the +# window and its own name was occupied again before it could be restored — or +# whose run was killed between the rename and the unlink. Nothing below would +# ever see one: every pass globs target-*/snapshot-*, which a dotted name does +# not match. Left unswept it is permanently unreclaimable disk on the one +# volume whose entire problem is disk. +# +# Safe under a concurrent pass mid-eviction for the reason evict_dir is: the +# aside name only exists after that pass's rename, so a consumer that resolved +# the directory published its marker before the count below. +for aside in "$ROOT"/.evicting-*; do + [ -d "$aside" ] || continue + aside_name=$(basename "$aside"); aside_name="${aside_name#.evicting-}"; aside_name="${aside_name%-*}" + if [ "$(live_reader_count "$ROOT" "$aside_name")" -gt 0 ]; then + echo "prune: $(basename "$aside") is still being read — deferring its reclamation again" + continue + fi + echo "prune: reclaiming deferred eviction $(basename "$aside")" + rm -rf "$aside" +done + echo "=== pass 1: liveness (unconditional, not gated on free space) ===" declare -A live_ns=() LIVENESS_AVAILABLE=0 @@ -174,9 +257,9 @@ if [ "$LIVENESS_AVAILABLE" = "1" ]; then [ -n "${live_ns[$name]:-}" ] && continue is_locked "$dir" && continue dir_gb=$(usage_gb "$dir") - echo "::warning::pruning dead-branch cache ${name} (${dir_gb} GB) — no matching branch on origin" + evict_dir "$dir" || continue + echo "::warning::pruned dead-branch cache ${name} (${dir_gb} GB) — no matching branch on origin" summary_line "- pruned dead-branch cache \`${name}\` (${dir_gb} GB) — branch no longer exists on origin" - rm -rf "$dir" pruned_any=1 done [ "$pruned_any" = "1" ] || echo "no dead-branch caches found" @@ -207,9 +290,9 @@ for dir in "${LRU[@]}"; do [ "$FREE_KB" -ge "$THRESHOLD_KB" ] && break is_locked "$dir" && continue dir_gb=$(usage_gb "$dir") - echo "::warning::evicting $(basename "$dir") (${dir_gb} GB, LRU under disk pressure)" + evict_dir "$dir" || continue + echo "::warning::evicted $(basename "$dir") (${dir_gb} GB, LRU under disk pressure)" summary_line "- evicted \`$(basename "$dir")\` (${dir_gb} GB, LRU under disk pressure)" - rm -rf "$dir" done read -r TOTAL_KB FREE_KB <<< "$(read_df "$ROOT")" -- 2.43.0 From c6a3fa6d97b09470e7f375a41b98817d60bcba9e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 23:13:25 -0500 Subject: [PATCH 2/5] docs(readme): say who moves the v1 tag, when, and what it promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Versioning section named the moving-major-tag model but not the release step that model implies. This is the first change where that gap has a consequence: `v1` and `origin/main` are the same commit today, so merging the eviction fix is the first thing that makes them diverge — at which point the fix is on `main` and every consumer is still fetching the old scripts. Records what was previously only implicit: that merging ships nothing, that re-pointing `v1` is a deliberate post-merge action because it changes what another repository's CI runs next, the exact commands and the check that it took, what a consumer is promised by pinning `@v1` and what forces a `v2`, and who is actually downstream. Also states why the moving pointer is the right model here rather than immutable release tags, since "safer in general" is the obvious objection and it deserves an answer. Docs only; no script or action definition is touched. --- README.md | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/README.md b/README.md index 2daa71c..6a07994 100644 --- a/README.md +++ b/README.md @@ -305,6 +305,45 @@ Pin `@v1`. It is a moving major tag: fixes and backward-compatible inputs move it forward, and anything that would break an existing consumer gets `v2` instead. Pin a commit SHA if you want a frozen version. +The alternative is immutable release tags — `v1.0.1`, `v1.0.2`, … — with every +consumer edited to point at the new one per fix. That is the safer model in +general and the wrong one here. What it buys is the ability to hold one +consumer back on a known-good version; what it costs is a PR in every consumer +repo per fix, and its real failure mode with two consumers and one operator is +that the second one is simply never updated and quietly runs a version nobody +is testing. The moving pointer makes a release one action with one blast +radius, which is the thing worth being deliberate about. Anyone who wants the +immutable behaviour already has it, by pinning a SHA. + +**What `@v1` promises** is that whatever it points at works with the inputs +documented above, spelled as they are documented. A change that renames or +removes an input, changes a default in a way that changes behaviour, or +requires something new of the consuming workflow — another `container.volumes` +entry, another permission — is a `v2`, not a `v1` move. Everything else moves +`v1`: correctness fixes, new optional inputs, and anything internal to +`scripts/`. + +**Moving the tag is a release step, and it is the operator's.** Merging to +`main` ships nothing to anybody. `v1` is a lightweight tag and does not follow +a branch, so until it is re-pointed every consumer keeps fetching the commit it +already named, whatever `main` now says. The gap is deliberate: re-pointing +`v1` changes what another repository's CI executes on its next run, so it is a +decision taken once, knowingly, after the merge — never something a merge does +by itself. + +```bash +git fetch origin +git tag -f v1 origin/main +git push -f origin v1 +git ls-remote --tags origin v1 # must equal git rev-parse origin/main +``` + +**Downstream** are emowheel, which pins `cargo-cache@v1` and +`cargo-cache-publish@v1` across its CI workflow, and zemyna, migrating to the +same pin. Both pick a move up on their next run with no change on their side, +which is the whole point of the moving pointer and also the reason the move is +not automatic. + --- ## Development -- 2.43.0 From 65f0782233ba418f7b54f56ec7a8bd48af4d138e Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 23:30:56 -0500 Subject: [PATCH 3/5] fix(prune-cache): stop the aside sweeper depending on timing it cannot see MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review findings on #2. The first is the one that mattered: the sweeper this PR added had the shape the PR exists to remove. Pass A renames a candidate aside; pass B's sweep sees an aside with no readers and reclaims it; A then finds a reader and restores. `rm -rf` traverses fd-relative, so the rename does not stop it and A can republish a half-emptied tree under a live cache name. `capacity: 1` bounds it today, which is exactly the kind of reason this PR was written to stop relying on. The unlink itself was never the problem — the ordering proof covers it under any interleaving, since the aside name only exists after the evicting pass's rename. What was missing is that an aside with no readers is indistinguishable from one a pass has just created and not yet decided about. The sweeper now leaves an aside alone until it has settled (EVICTION_ASIDE_SETTLE_SECONDS, default 60), which separates the two without having to identify the pass that created it — a PID is meaningless across the job containers these passes run in, and recycles. Read from ctime, not mtime: rename(2) updates the first and leaves the second at whenever the cache was last written, which is the signal list_by_lru wants and the wrong one here. That is a bound, not a construction, and both the code comment and the README now say which of the two properties is which instead of asserting the broader one. Also from the review: a pass that declined every dead cache it found no longer signs off with "no dead-branch caches found", and evict_dir no longer promises a later reclamation of an aside that is already gone. Scenario 14 covers the settle window against the script's own default, with nothing faked — the directory really was set aside a moment ago. Scenario 4 gains the summary assertion. 31 -> 37 assertions; each new gate verified red by defeating it alone in a scratch copy. --- README.md | 17 ++++++--- scripts/prune-cache-selftest.sh | 49 +++++++++++++++++++++++-- scripts/prune-cache.sh | 65 +++++++++++++++++++++++++++++---- 3 files changed, 114 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index 6a07994..68786fe 100644 --- a/README.md +++ b/README.md @@ -182,9 +182,14 @@ not collapsed into one: rename cannot resolve the path at all and starts cold instead. A cache claimed inside that window is put back under its own name, and one whose name a concurrent seed has taken in the meantime is left aside and reclaimed - by a later pass once its readers drain. Until this was structural it was - merely policy — snapshots belong to protected refs, protected refs are never - eviction candidates — which is a property held by vigilance rather than by + by a later pass once its readers drain. That later pass leaves an aside + directory alone until it has been set aside for a minute — not for the + unlink's sake, which the ordering proof above already covers, but so that a + pass still deciding about one is never mistaken for a pass that died holding + it. That settle window is a bound rather than a construction, and it is the + only part of this that is. Until the rest of it was structural it was merely + policy — snapshots belong to protected refs, protected refs are never + eviction candidates — a property held by vigilance rather than by construction. - **Every other way the source can change mid-clone is detected, not prevented.** A `seed-fallback-dir` pointing at a directory something else @@ -199,9 +204,9 @@ not collapsed into one: residual is capped at one deferred generation per publisher ref, and its real cost is close to inode count rather than byte count, since the artifacts are hardlinked to whatever cloned them. A declined eviction is - bounded the same way: the cache stays, either under its own name or aside - awaiting the next pass, and is never unlinked under the job that claimed - it. + never unlinked under the job that claimed it; what it costs meanwhile is + disk, normally as the cache restored under its own name and otherwise as one + set aside for a later pass to reclaim. **Eviction** runs three passes: caches for branches that no longer exist on origin are removed unconditionally; then, only if free space is under the diff --git a/scripts/prune-cache-selftest.sh b/scripts/prune-cache-selftest.sh index 8c66e31..8c6feac 100755 --- a/scripts/prune-cache-selftest.sh +++ b/scripts/prune-cache-selftest.sh @@ -13,7 +13,9 @@ # not age, is what decides pass 1. # 3. PROTECTED REFS NEVER EVICTED under forced disk pressure, even when # their caches are the oldest on disk and would rank first for LRU. -# 4. LOCKED CACHE PROTECTED even when dead, old, and under pressure. +# 4. LOCKED CACHE PROTECTED even when dead, old, and under pressure — and +# the pass's closing summary agrees with the decline it just logged, +# rather than reporting that it found nothing. # 5. STALE LOCK NOT HONOURED FOREVER — the same cache with a lock older than # STALE_LOCK_SECONDS is evicted, so a crashed job cannot pin a directory # permanently. @@ -38,6 +40,10 @@ # 13. A DEFERRED EVICTION IS RECLAIMED, but not while its reader is live. # Nothing else globs a dotted name, so an unswept one is disk lost for # good on the volume whose whole problem is disk. +# 14. AND NOT WHILE ANOTHER PASS MAY STILL BE EVICTING IT. An aside with no +# readers is indistinguishable from one a concurrent pass has just +# renamed and not yet decided about; reclaiming that one lets `rm -rf` +# empty a tree its owner may still restore under a live cache name. set -euo pipefail script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) . "$script_dir/cache-lib.sh" @@ -82,8 +88,14 @@ reset_cache() { mk "snapshot-$DEAD" '2030-01-01' mk "target-$OWN" '2025-01-01' } +# run_prune [settle-seconds] +# +# The settle window is only set when a scenario asks for it, so every other +# scenario — scenario 14 above all — runs against the script's own default +# rather than against a value this file chose. run_prune() { local free="${1:-}" + if [ -n "${2:-}" ]; then export EVICTION_ASIDE_SETTLE_SECONDS="$2"; else unset EVICTION_ASIDE_SETTLE_SECONDS; fi CACHE_DF_OVERRIDE="$free" GITHUB_STEP_SUMMARY="$scratch/summary" \ bash "$prune" "$root" "$root/target-$OWN" "dev main" 10 > "$scratch/log" 2>&1 \ || { cat "$scratch/log"; fail "prune-cache.sh exited non-zero"; } @@ -133,6 +145,17 @@ date +%s > "$root/target-$DEAD/.ci-lock-ci-1" run_prune "1000000 1000" assert_kept "$root/target-$DEAD" "locked cache survives both passes" assert_log "held open by" "lock reported in the log" +# With the locked one the only dead cache left, the pass has declined every +# dead cache it found — at which point "no dead-branch caches found" is a +# false summary of the decline logged two lines above it. +rm -rf "$root/snapshot-$DEAD" +run_prune "1000000 900000" +assert_kept "$root/target-$DEAD" "still not evicted when it is the only dead cache" +assert_log "none pruned this pass" "a pass that declined every dead cache reports that" +if grep -q "no dead-branch caches found" "$scratch/log"; then + fail "the closing summary contradicts the decline logged above it" +fi +ok "the summary does not claim it found nothing" echo echo "=== 5: a stale lock is not honoured forever ===" @@ -225,13 +248,33 @@ reset_cache aside="$root/.evicting-target-$DEAD-9999" mkdir -p "$aside"; head -c 4096 /dev/zero > "$aside/blob" date +%s > "$root/.reading-target-$DEAD-job1" -run_prune "1000000 900000" +# The settle window (scenario 14) gates this sweep first and would decide both +# runs on its own. A directory's ctime is what that window reads and cannot be +# backdated the way `touch -d` backdates an mtime, so the window is moved out +# of the way rather than the directory aged into it. +run_prune "1000000 900000" 0 assert_kept "$aside" "a deferred eviction is not reclaimed while a job is still reading it" assert_log "deferring its reclamation again" "the continued deferral is reported" rm -f "$root/.reading-target-$DEAD-job1" -run_prune "1000000 900000" +run_prune "1000000 900000" 0 assert_gone "$aside" "a deferred eviction is reclaimed once its reader is gone" assert_log "reclaiming deferred eviction" "the reclamation is reported" +echo +echo "=== 14: an aside another pass may still be evicting is left alone ===" +reset_cache +aside="$root/.evicting-target-$DEAD-9999" +mkdir -p "$aside"; head -c 4096 /dev/zero > "$aside/blob" +# Deliberately no reader marker: the reader gate would pass this straight +# through, which is the whole point. An aside with no readers is exactly what +# a pass that has just renamed one aside and not yet decided about it looks +# like, and `rm -rf` traverses fd-relative — so reclaiming it out from under +# that pass lets it republish a half-emptied tree under a live cache name. +# Nothing here fakes an age: the directory really was set aside a moment ago. +run_prune "1000000 900000" +assert_kept "$aside" "an aside younger than the settle window is not reclaimed" +assert_kept "$aside/blob" "and is left intact, not part-way emptied" +assert_log "may still be evicting it" "the deferral gives its actual reason" + echo echo "prune-cache-selftest: ${pass_count} assertions passed" diff --git a/scripts/prune-cache.sh b/scripts/prune-cache.sh index 08e0ebd..49419fa 100755 --- a/scripts/prune-cache.sh +++ b/scripts/prune-cache.sh @@ -9,6 +9,10 @@ # abandoned (default 7200) # CACHE_LIVENESS "false"/"0" to skip the liveness pass entirely # CACHE_DF_OVERRIDE " ", for the selftest +# EVICTION_ASIDE_SETTLE_SECONDS +# how long a directory renamed aside for eviction is +# left alone before another pass may reclaim it +# (default 60) # # Three passes, in order: # @@ -82,6 +86,15 @@ OWN_DIR="${2:?}" PROTECTED_REFS="${3:-}" MIN_FREE_PCT="${4:-10}" STALE_LOCK_SECONDS="${STALE_LOCK_SECONDS:-7200}" +# An aside directory is in flight for one rename plus one marker glob — +# milliseconds. Anything older belongs to a pass that died between the two, so +# an age is what separates "another pass is mid-eviction" from "a leftover", +# and it separates them without having to identify the pass that created it: a +# PID is meaningless across the job containers these passes run in, and +# recycles. Three orders of magnitude of headroom over the operation it covers, +# and short enough that a genuine leftover is reclaimed by the next run rather +# than lingering while the volume is under pressure. +EVICTION_ASIDE_SETTLE_SECONDS="${EVICTION_ASIDE_SETTLE_SECONDS:-60}" declare -A protected_ns=() for ref in $PROTECTED_REFS; do @@ -172,8 +185,10 @@ evict_dir() { if is_locked "$aside" "$name"; then if mv -T "$aside" "$dir" 2>/dev/null; then echo " ${name}: claimed by a job while its eviction was in flight — restored, not evicted" + elif [ -d "$aside" ]; then + echo "::warning::prune: ${name} was claimed mid-eviction and its own name is taken again — leaving $(basename "$aside") for a later pass to reclaim once its readers drain" else - echo "::warning::prune: ${name} was claimed mid-eviction and its own name is occupied again — leaving $(basename "$aside") for a later pass to reclaim once its readers drain" + echo "::warning::prune: ${name} was claimed mid-eviction and is already gone — another pass reclaimed it after its readers drained" fi return 1 fi @@ -204,17 +219,41 @@ list_by_lru() { # Deferred reclamations from an earlier pass: a directory renamed aside for # eviction that could not be unlinked, because a job claimed it inside the -# window and its own name was occupied again before it could be restored — or +# window and its own name was taken again before it could be restored — or # whose run was killed between the rename and the unlink. Nothing below would # ever see one: every pass globs target-*/snapshot-*, which a dotted name does # not match. Left unswept it is permanently unreclaimable disk on the one # volume whose entire problem is disk. # -# Safe under a concurrent pass mid-eviction for the reason evict_dir is: the -# aside name only exists after that pass's rename, so a consumer that resolved -# the directory published its marker before the count below. +# TWO DISTINCT PROPERTIES HOLD HERE, and neither implies the other. +# +# No clone can be truncated by the unlink below, by construction: the aside +# name only comes into existence after the evicting pass's rename, so a +# consumer that resolved the directory published its marker strictly before +# that rename and therefore before the count below — it cannot be missed. A +# consumer that arrives later cannot resolve the path at all. This holds under +# any interleaving and needs no settle window. +# +# No pass mid-eviction is mistaken for a leftover, by bound rather than by +# construction, and this is what the settle window is for. Without it a +# concurrent pass could unlink an aside its owner is still deciding about, and +# `rm -rf` traverses fd-relative: the owner's restore can then republish a +# half-emptied tree under a live cache name. What the window guarantees is that +# the two cannot be confused within it. What it does not guarantee is the +# pathological case beyond it — an evicting pass suspended past the window and +# then resumed finds its aside reclaimed and fails its restore, saying so. +now=$(date +%s) for aside in "$ROOT"/.evicting-*; do [ -d "$aside" ] || continue + # Change time, not modification time. `mv` leaves a directory's mtime alone + # (a cache last written days ago keeps a days-old mtime, which is what + # list_by_lru wants and exactly the wrong signal here) but rename(2) does + # update ctime, so %Z is when this directory was set aside and %Y is not. + aside_age=$(( now - $(stat -c '%Z' "$aside" 2>/dev/null || echo "$now") )) + if [ "$aside_age" -lt "$EVICTION_ASIDE_SETTLE_SECONDS" ]; then + echo "prune: $(basename "$aside") was set aside ${aside_age}s ago — another pass may still be evicting it, leaving it alone" + continue + fi aside_name=$(basename "$aside"); aside_name="${aside_name#.evicting-}"; aside_name="${aside_name%-*}" if [ "$(live_reader_count "$ROOT" "$aside_name")" -gt 0 ]; then echo "prune: $(basename "$aside") is still being read — deferring its reclamation again" @@ -250,19 +289,29 @@ fi if [ "$LIVENESS_AVAILABLE" = "1" ]; then pruned_any=0 + # Tracked separately so the line below cannot contradict the decline lines + # above it: "none pruned" and "none found" are different outcomes, and a + # pass that declined every dead cache it found has not found none. + spared_any=0 for dir in "$ROOT"/target-* "$ROOT"/snapshot-*; do [ -d "$dir" ] || continue name=$(basename "$dir") is_protected "$dir" && continue [ -n "${live_ns[$name]:-}" ] && continue - is_locked "$dir" && continue + is_locked "$dir" && { spared_any=1; continue; } dir_gb=$(usage_gb "$dir") - evict_dir "$dir" || continue + evict_dir "$dir" || { spared_any=1; continue; } echo "::warning::pruned dead-branch cache ${name} (${dir_gb} GB) — no matching branch on origin" summary_line "- pruned dead-branch cache \`${name}\` (${dir_gb} GB) — branch no longer exists on origin" pruned_any=1 done - [ "$pruned_any" = "1" ] || echo "no dead-branch caches found" + if [ "$pruned_any" = "0" ]; then + if [ "$spared_any" = "1" ]; then + echo "every dead-branch cache found is still in use — none pruned this pass" + else + echo "no dead-branch caches found" + fi + fi else echo "liveness: ${LIVENESS_REASON} — treating as UNAVAILABLE (not as \"no branches\"); pass 1 skipped" fi -- 2.43.0 From dcd73dd82df8e1ee4ed39dda744ba12329552305 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 23 Aug 2026 23:46:03 -0500 Subject: [PATCH 4/5] test(prune-cache): build scenario 14's aside the way the pass builds one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review finding on #2. Swapping the settle window's `stat -c %Z` for `%Y` — the exact substitution the comment beside it calls the wrong signal — left the suite green at 37/37. The scenario built its aside with `mkdir`, so the fixture's mtime was also ~now and the two timestamps agreed; a fixture whose clocks agree cannot tell them apart. Every real aside is the opposite shape: a cache last written days ago, renamed a moment ago. Under `%Y` the window would never fire for one, the sweeper would silently return to reclaiming asides another pass is still deciding about, and nothing would say so — the mechanism guarded by a comment again, which is what the previous finding was about. So the fixture is now built the way the pass builds one: an old directory `mv`d into the aside name. `%Z` -> `%Y` now fails scenario 14, as does deleting the guard outright. Also sharpens why the PID alternative was rejected: the `$$` in an aside's name was that pass's PID inside its own job container, so testing it from another one is not unreliable, it is meaningless. No change to prune-cache.sh's behaviour; the assertions are untouched. --- scripts/prune-cache-selftest.sh | 16 +++++++++++++--- scripts/prune-cache.sh | 7 ++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/scripts/prune-cache-selftest.sh b/scripts/prune-cache-selftest.sh index 8c6feac..06585c9 100755 --- a/scripts/prune-cache-selftest.sh +++ b/scripts/prune-cache-selftest.sh @@ -43,7 +43,9 @@ # 14. AND NOT WHILE ANOTHER PASS MAY STILL BE EVICTING IT. An aside with no # readers is indistinguishable from one a concurrent pass has just # renamed and not yet decided about; reclaiming that one lets `rm -rf` -# empty a tree its owner may still restore under a live cache name. +# empty a tree its owner may still restore under a live cache name. Its +# fixture is an OLD directory renamed a moment ago — production's shape, +# and the only shape that can tell the two timestamps apart. set -euo pipefail script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) . "$script_dir/cache-lib.sh" @@ -264,13 +266,21 @@ echo echo "=== 14: an aside another pass may still be evicting is left alone ===" reset_cache aside="$root/.evicting-target-$DEAD-9999" -mkdir -p "$aside"; head -c 4096 /dev/zero > "$aside/blob" +# Built the way the pass builds one — an old cache directory renamed a moment +# ago — because the two ages that describe it disagree, and which of them the +# window reads is the whole mechanism. `mkdir`-ing the aside directly would +# give it a fresh mtime as well as a fresh ctime, and a fixture whose two +# clocks agree cannot tell %Z from %Y: the settle window would read the wrong +# one, never fire for any real aside, and this scenario would not notice. +victim="$root/target-$DEAD-victim" +mkdir -p "$victim"; head -c 4096 /dev/zero > "$victim/blob" +touch -d '2020-01-01' "$victim" # an old cache, which is every cache +mv -T "$victim" "$aside" # set aside a moment ago # Deliberately no reader marker: the reader gate would pass this straight # through, which is the whole point. An aside with no readers is exactly what # a pass that has just renamed one aside and not yet decided about it looks # like, and `rm -rf` traverses fd-relative — so reclaiming it out from under # that pass lets it republish a half-emptied tree under a live cache name. -# Nothing here fakes an age: the directory really was set aside a moment ago. run_prune "1000000 900000" assert_kept "$aside" "an aside younger than the settle window is not reclaimed" assert_kept "$aside/blob" "and is left intact, not part-way emptied" diff --git a/scripts/prune-cache.sh b/scripts/prune-cache.sh index 49419fa..432b452 100755 --- a/scripts/prune-cache.sh +++ b/scripts/prune-cache.sh @@ -89,9 +89,10 @@ STALE_LOCK_SECONDS="${STALE_LOCK_SECONDS:-7200}" # An aside directory is in flight for one rename plus one marker glob — # milliseconds. Anything older belongs to a pass that died between the two, so # an age is what separates "another pass is mid-eviction" from "a leftover", -# and it separates them without having to identify the pass that created it: a -# PID is meaningless across the job containers these passes run in, and -# recycles. Three orders of magnitude of headroom over the operation it covers, +# and it separates them without having to identify the pass that created it. +# The `$$` in an aside's name was that pass's PID inside its own job +# container, so testing it with `kill -0` from a different one is not +# unreliable, it is meaningless — and PIDs recycle besides. Three orders of magnitude of headroom over the operation it covers, # and short enough that a genuine leftover is reclaimed by the next run rather # than lingering while the volume is under pressure. EVICTION_ASIDE_SETTLE_SECONDS="${EVICTION_ASIDE_SETTLE_SECONDS:-60}" -- 2.43.0 From b0d63c807bf7258b6d8951afabb42176ab61f9a7 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 24 Aug 2026 09:10:43 -0500 Subject: [PATCH 5/5] style(prune-cache): rewrap the settle-window comment; drop an overclaim MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review nits on #2, both cosmetic. The sentence added last round left its paragraph at 127 characters in a file that otherwise wraps comments at 78-79 — the rewrap after the insert simply did not happen. `awk 'length($0)>80'` over both files now reports one comment line, the pre-existing 93-character usage string at :4. Scenario 14's header called its fixture "the only shape that can tell the two timestamps apart". The property it needs is old mtime with fresh ctime; an old directory renamed a moment ago is the production instance of that, not the only construction of it. "Production's shape" was already carrying the argument. No behaviour change and no assertion change. --- scripts/prune-cache-selftest.sh | 2 +- scripts/prune-cache.sh | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/scripts/prune-cache-selftest.sh b/scripts/prune-cache-selftest.sh index 06585c9..1ffa885 100755 --- a/scripts/prune-cache-selftest.sh +++ b/scripts/prune-cache-selftest.sh @@ -45,7 +45,7 @@ # renamed and not yet decided about; reclaiming that one lets `rm -rf` # empty a tree its owner may still restore under a live cache name. Its # fixture is an OLD directory renamed a moment ago — production's shape, -# and the only shape that can tell the two timestamps apart. +# and what lets it tell the two timestamps apart. set -euo pipefail script_dir=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd) . "$script_dir/cache-lib.sh" diff --git a/scripts/prune-cache.sh b/scripts/prune-cache.sh index 432b452..23f07a5 100755 --- a/scripts/prune-cache.sh +++ b/scripts/prune-cache.sh @@ -92,9 +92,10 @@ STALE_LOCK_SECONDS="${STALE_LOCK_SECONDS:-7200}" # and it separates them without having to identify the pass that created it. # The `$$` in an aside's name was that pass's PID inside its own job # container, so testing it with `kill -0` from a different one is not -# unreliable, it is meaningless — and PIDs recycle besides. Three orders of magnitude of headroom over the operation it covers, -# and short enough that a genuine leftover is reclaimed by the next run rather -# than lingering while the volume is under pressure. +# unreliable, it is meaningless — and PIDs recycle besides. Three orders of +# magnitude of headroom over the operation it covers, and short enough that a +# genuine leftover is reclaimed by the next run rather than lingering while +# the volume is under pressure. EVICTION_ASIDE_SETTLE_SECONDS="${EVICTION_ASIDE_SETTLE_SECONDS:-60}" declare -A protected_ns=() -- 2.43.0