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.
This commit is contained in:
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user