c6a3fa6d97b09470e7f375a41b98817d60bcba9e
5
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 claude
|
88ab063b64
|
fix(prune-cache): close the reader-marker check-then-delete window
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. |
||
 claudeandClaude Opus 5
|
3b2dec6a50
|
test(cargo-cache): cover the stale-reader-marker sweep and its bound
The staleness path decides whether a publisher may reclaim disk, so getting it
wrong means an abandoned marker pins a snapshot generation forever — the exact
outcome the bound exists to prevent. It was previously covered only by
analogy to prune-cache.sh's .ci-lock-* staleness, which is not the bar.
New publish-snapshot-selftest.sh scenario 8 asserts both directions against
the SAME backdated marker, which is what separates "honours the bound" from
"ignores anything that looks old":
* under CACHE_READ_STALE_SECONDS=86400 a three-hour-old marker is left alone
and still defers reclamation, exactly as a live reader does;
* under the 7200s default the same marker is swept, reported as swept, and the
generation it was pinning — plus the one deferred by the first half — is
reclaimed.
Backdated with `touch -d`, not slept for; the suite stays fast.
Red-proven by mutation rather than against the pre-fix scripts, since the
whole mechanism is new there and "it does not exist yet" proves nothing about
the threshold logic. Mutating live_reader_count's bound test to `true` (never
sweep) fails scenario 8:
ASSERTION FAILED: the stale marker was not reported as swept
and to `false` (sweep everything, bound ignored) fails scenario 6 instead,
which is the right blast radius — ignoring the bound means unlinking under a
LIVE reader:
ASSERTION FAILED: the previous generation was unlinked while a reader
still held it
Also documents the entry-count check's measured cost in the README: on ext4
with a warm cache over 78,554 entries, 44 ms per metadata walk against 3,126 ms
for the `cp -al` it guards — about 2.8%. Not a perf-claiming change; the number
is there so the next reader does not have to wonder.
The reviewer's other nit — scenarios 8/9 of the seed suite exercising
publish-snapshot.sh's interlock — was already covered by the cross-reference
in this file's header, so no move.
Verification: `bash scripts/selftest.sh` — 5 suites, exit 0, 88 assertions
(82 before this commit, 63 at baseline). shellcheck: no new findings.
Refs: daniel/gitdan#11, zemyna#911
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L
|
||
|
claudeandClaude Opus 5
|
719475831b
|
fix(cargo-cache): scope the safety claim to what the code actually prevents
Follow-up to |
||
|
claudeandClaude Opus 5
|
f57e2a6013
|
fix(cargo-cache): close the seed-vs-republish race the design claimed to close
The shared action's justification over zemyna's and emowheel's schemes was that hardlink-cloning from a published snapshot closes gitdan #911 "by construction, not by the single job slot". Review disproved that. This makes the claim true, and corrects the README where it could only be bounded. Finding 1 (verdict-level) — silent partial clone ------------------------------------------------ `hardlink_clone_into` ran `cp -al` with no exit-status check, and both call sites invoked it as a condition, which suppresses `set -e` for the whole call. A publisher's `rm -rf` of the generation it rotated away therefore unlinked entries beneath an in-flight consumer walk, and the truncated tree was renamed into place and reported as success. Both layers are fixed: * The consumer verifies its own clone. Every attempt checks `cp -al`'s status explicitly, the source directory's inode before and after (a wholesale replacement mid-walk splices two generations), and the entry count — the only signal for a subtree unlinked before its parent was listed, since `cp -al` reports no error for one it never saw. Any failure discards the staging tree and retries; exhausting the attempts returns a distinct status 2 and fails the job rather than seeding a partial cache. `unshare_subtree` / `_unshare_files` now propagate failure too — a swallowed unshare leaves the clone aliasing its source, the exact corruption that step exists to prevent. * The publisher does not unlink under a reader. A consumer publishes a `.reading-<snapshot>-<tag>` marker before it resolves the snapshot path; the publisher scans for markers after its first rename. A consumer holding the old generation therefore published its marker before that scan and cannot be missed; one arriving after the scan necessarily resolves to the new generation. The publisher waits for readers to drain and, on timeout, DEFERS reclamation rather than forcing it — the old generation is left as `.publish-old-<key>-<tag>` and swept by a later publish. So correctness is closed by construction; disk reclamation is bounded, not immediate. The residual is capped at one deferred generation per publisher ref, and the README now says exactly that instead of the disproved claim. Finding 2 — restore-mtimes.sh ran with no errexit ------------------------------------------------- `set -euo pipefail` was glued to the end of a comment (`# soundness.set -euo pipefail`), so it was entirely commented out: a partial failure of the `git log | awk` pipeline would have produced wrong mtimes across the whole restore instead of failing loudly. Moved to its own line. Audited every other script for the same defect — this was the only instance. Independent confirmation: shellcheck's two SC2164 warnings on this file's `cd "$repo_root"` disappear now that errexit is actually in effect. Finding 3 — lock-acquire window ------------------------------- A just-seeded directory was unlocked until a later action step, so a concurrent job's prune pass could evict it. `seed-target-dir.sh` now takes an optional lock-id and writes the lock marker on every path out of the script, including into the staging tree before its rename, so the directory carries a lock the instant it appears under its final name. The action's acquire step stays (it is idempotent and stamps the LRU marker). Also hardened `prune-cache.sh` to treat a directory with live reader markers as locked. Today no reachable configuration prunes a snapshot — only protected refs publish them and protected refs are excluded from every pass — so this is redundant by policy; it is here so that stops being the reason it is safe. Verification ------------ New selftest scenario 8 races a real seed against a real publish rotation, gating the rotation on the seed's *observed* clone progress so the window is hit deterministically rather than on a fast machine's coin flip. Red-proven against the unguarded scripts, three consecutive runs: ASSERTION FAILED: the seeded tree is truncated: 15443 entries against the snapshot's 493 (was 48805 before the rotation) (15443 / 16986 / 16498) Green after the fix, six consecutive runs, catching the clone mid-walk at ~10.5k of 48805 entries each time. Scenario 9 covers deferred reclamation and its later sweep; scenario 10 covers an unreadable source failing loudly. `bash scripts/selftest.sh`: 5 suites, exit 0, 75 assertions (was 63). shellcheck over `scripts/`: no new findings, two SC2164 warnings resolved. Docs: README's republish-safety paragraph replaced with what the code now guarantees, including the bounded disk residual stated explicitly; new `read-grace-seconds` / `reader-stale-seconds` inputs documented in the `cargo-cache-publish` table; the selftest table names the new race. Refs: daniel/gitdan#11, zemyna#911 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L |
||
|
claudeandClaude Opus 5
|
248af3061e
|
feat(cargo-cache): hardlink-clone a per-ref Cargo cache from a published snapshot
Replaces the phase-0 resolution probe with the real actions, merging the two independent per-branch Cargo cache implementations on this forge into the design neither of them had. ## The merge - zemyna seeds a PR branch by `cp -al` hardlink clone (near-free: cost scales with inode count, not bytes) from the base branch's LIVE target dir — a torn read waiting for a second job slot (its own #911). - emowheel seeds from a PUBLISHED IMMUTABLE SNAPSHOT (no race by construction) but with `cp -a`, duplicating ~35 GB per branch. This ships hardlink-clone FROM a published snapshot: zemyna's cost profile, emowheel's soundness, and #911 closed structurally rather than by the runner happening to have one execution slot. ## The bug both implementations have A build inside a `cp -al` clone DOES mutate the directory it was cloned from. Cargo replaces real artifacts, but writes its metadata — and build scripts write their OUT_DIR — with a plain truncating write, straight through the shared inode. Measured set: `.fingerprint/<unit>/dep-<target>` (under CARGO_UNSTABLE_CHECKSUM_FRESHNESS), `build/<pkg>/{output,root-output,out/**}`, `deps/*.d` and `<profile>/*.d`. The checksum-freshness case is a wrong answer, not a slow build: a PR clone rewrites the base's dep-info to describe the PR's sources while the base's cache still holds the artifact built from the base's; once the PR merges, the base's next run finds the checksums match, reports `Fresh`, and links a binary built from the pre-merge code. Reproduced end to end. Fix: hardlink the artifacts (the GB), real-copy the metadata (the MB) — about 3.7% of a 6.9 GB Bevy target dir, against 100% for a full copy. ## Contents - `cargo-cache/action.yml` — consume: resolve keys, seed from the base's snapshot via staging + one atomic rename, strip Cargo lock files, unshare the mutable paths, restore mtimes from git history, lock, prune. - `cargo-cache-publish/action.yml` — publish: record the build watermark, atomically republish the snapshot on a protected branch, release the lock (`mode: release-lock` for the `if: always()` step). - `scripts/` — all logic, so it is testable standalone; the YAML is wiring. - `scripts/*selftest.sh` + `selftest.sh` — five suites, 63 assertions, every fix paired with a control that reproduces the bug. All green locally. Eviction merges emowheel's liveness pass (dead branches pruned unconditionally, not gated on disk pressure) with LRU-under-pressure, but inverts the order within the pressure pass: `target-*` before `snapshot-*`, because a snapshot is hardlinked to everything cloned from it, so evicting one frees almost no real bytes while costing every future PR its warm start. restore-mtimes.sh is ported from emowheel (the watermark variant, which closes the merge hazard zemyna's copy still has) with its provenance de-projectised. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Sqh2vscfzisk83VuPVQX9L |