imagecache: eviction engine — root set, EvictUnused, startup orphan recovery - #735
Open
igooch wants to merge 4 commits into
Open
imagecache: eviction engine — root set, EvictUnused, startup orphan recovery#735igooch wants to merge 4 commits into
igooch wants to merge 4 commits into
Conversation
…ecovery The engine that reclaims cache disk, built on the primitives from the size/last-use, spec-digest, record-first-pull, and layer-retirement PRs. Inert in production: nothing calls EvictUnused yet (the watermark GC loop and its flags come separately); the only behavior New gains is the startup orphan scan. - Store.InUse: scans bundle overlay specs under the actors dir (opt-in via WithActorsDir) for the root set — image digests, layer hexes, and exact layer-set signatures. The exact-set rule roots the multi-arch twin record and digestless (pre-ImageDigest) specs' records, without making every superset image unevictable. - Store.EvictUnused: LRU eviction of unprotected images and the layers their removal leaves unreferenced, with min-age vetoes, per-victim re-checks under hitMu held exclusive against the cache-hit touch, and a restore-on-keep commit protocol: if any layer of a deleted record must be kept (still referenced, rooted, fresh, or failed to retire), the record is rewritten from bytes captured at listing time so the kept layer is never stranded unreachable. Freed bytes are credited optimistically from recorded sizes; dry-run mutates nothing. - Store.RecoverOrphans, called once from New: reclaims layer dirs no record references. Runs only at startup, the one moment the scan is race-free (no pull in flight), and skips itself entirely when the record enumeration is incomplete — refcounts from partial data make referenced layers look like garbage. Failure never fails New. - Per-item log lines (root-set entries, per-layer keep/retire, skips) are Debug and gated on Enabled so suppressed passes don't pay for attr construction; pass summaries, evictions, and restores stay Info. - pull_gated_test.go: the slow-pull progress-touch test now asserts the freshened record vetoes a concurrent EvictUnused pass (deferred from the record-first-pull PR, which had no engine to call). Tests cover every veto, shared-layer survival, both restore-on-keep shapes (wedged-pull-fresh-layer and rooted-subset), orphan reclaimed at startup but ignored by the periodic pass, the enumeration completeness gate, dry-run purity, and an ensure-vs-evict race loop under -race.
…erated Review fixes, all in the error paths around unreadable records: - EvictUnused now gates on complete record enumeration, like the startup scan already did. Refcounts from a partial listing understate references - a layer shared with an unreadable record hits zero and is retired while that record still names it - so the doc claim that a partial listing "only ever skips work" was inverted. The pass skips entirely, logs at ERROR naming the records, and returns the error; every later pass retries. Skipping only layer retirement instead would strand the evicted records' layers as orphans until restart, so the whole pass fails toward retention. - An undecodable record is never an eviction candidate: deleting it would strand its (unknown) layers the same way. It stays in place, surfaced in the enumeration error for the operator. - Dry-run no longer writes size-file backfills: dryRunRetire sizes via a read-only variant of layerSize (split into recordedLayerSize + walkLayerSize). The affected population is exactly the pre-size-file layers a dry-run soak on upgraded nodes exists to observe. - restoreRecord documents the cost of its mtime bump: the record's true last-use is lost, accepted because preserving it would churn the same doomed candidate through delete-and-restore every pass. One test per finding: gated pass with an unreadable shared-layer record (root-gated on euid 0), undecodable record survival, and dry-run leaving no size file behind.
The bulleted summary duplicated the per-test comments; sibling test files carry at most a short note on what distinguishes the file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 2 of #463 (image-cache GC + observability): this is the eviction engine itself — the root set, the eviction pass, and startup orphan recovery — assembled from the foundations merged in #650 (sizes/last-use), #656 (spec digest), #658 (retirement primitives), and #717 (record-first pull). What remains after this is the watermark loop + flags that turn it on, then metrics and e2e.
Inert in production: nothing calls
EvictUnusedyet — the watermark GC loop and its flags come in the next PR. The only behaviorNewgains is the startup orphan scan.What
Store.InUse()— the root set: scans bundle overlay specs under the actors dir (opt-in viaWithActorsDir) for image digests, layer dirs, and exact layer-set signatures. The exact-set rule roots the multi-arch twin record and digestless (pre-imageDigest) specs' records without making every subset image unevictable.Store.EvictUnused(ctx, targetBytes, dryRun)— LRU eviction of unprotected images and the layers their removal leaves unreferenced. Min-age vetoes; per-victim re-checks underhitMuheld exclusive against the cache-hit touch; two-phase deletion viaretireLayer, with the slowRemoveAlloutside all locks. If any layer of a deleted record must be kept (still referenced, rooted, fresh, or failed to retire), the record is atomically restored from bytes captured at listing time — a kept layer is never left unreachable.Store.RecoverOrphans, called once fromNew— reclaims layers no record references. Startup is the one moment this scan is race-free (no pull can be in flight); a failure never failsNew.Enabledgate (level control via serverboot: dynamic --log-level flag for the server binaries #677); pass summaries, evictions, and restores are Info.Notes for reviewers
backdate,mustEnsure) and imagecache: write the image record before unpacking layers #717's gated-registry harness — no duplicated symbols. The slow-pull gated test gains the eviction-veto assertion deferred from imagecache: write the image record before unpacking layers #717.layerSizesplit into read/walk/backfill parts; dry-run uses the read-only path).Storebuilt withoutWithActorsDirhas an empty root set (deliberate for tools/tests); the wiring PR must pass it in atelet.Testing
go test -race ./internal/imagecache/: every veto, shared-layer survival, both restore-on-keep shapes, orphan reclaimed at startup and ignored by the periodic pass, enumeration gate (unreadable + undecodable), dry-run purity, 25-iteration ensure-vs-evict race loop.