Skip to content

atelet: watermark-driven image cache GC loop - #836

Open
igooch wants to merge 3 commits into
agent-substrate:mainfrom
igooch:imagecache-gc-loop
Open

atelet: watermark-driven image cache GC loop#836
igooch wants to merge 3 commits into
agent-substrate:mainfrom
igooch:imagecache-gc-loop

Conversation

@igooch

@igooch igooch commented Aug 10, 2026

Copy link
Copy Markdown
Collaborator

Phase 2 of #463, continuing from #735 (the eviction engine): the watermark-driven loop in atelet that turns eviction on, plus the flags that govern it. After this: observability (metrics/spans), the ops-tool swap, and the e2e suite.

What

  • A serialized, panic-recovered pass every --image-cache-gc-period (default 5m; 0 disables the loop entirely). Each tick: statfs the cache volume + sum the pool's recorded sizes, compute a byte target — down to --image-cache-low-percent when usage crossed --image-cache-high-percent, and/or down to --image-cache-max-bytes — and hand it to Store.EvictUnused.
  • The target is capped at the pool's own size. The cache is one tenant of a shared volume; uncapped kubelet-style watermark math on a pressured volume asks a tiny cache to fix pressure it didn't cause, evicting everything every tick.
  • Gated passes are detected by contract, not inference: the engine exports ErrIncompleteEnumeration (wrapped into every gate return), and classifyGCPass — a pure, table-tested function — maps each pass to skipped/shortfall/complete/quiet. A gated pass logs ERROR "pass skipped" and never feeds the shortfall backoff.
  • Genuine shortfall warns with backoff then decays to a periodic reminder: on a volume under foreign pressure, shortfall is the steady state and must not ERROR every tick.
  • atelet passes WithActorsDir(ateompath.ActorsDir) so the root set sees placed actors (the wiring imagecache: eviction engine — root set, EvictUnused, startup orphan recovery #735's notes required); ateompath gains the ActorsDir constant.
  • The first pass runs immediately at boot (a node starting under disk pressure shouldn't wait a full period), and a CacheSize failure skips the pass explicitly rather than silently zeroing the target.
  • --image-cache-gc-dry-run computes and logs every decision while deleting nothing — the production soak mechanism. A cache dir outside BasePath logs a warning (its watermarks would measure a different volume than actor state).
  • README: the GC section's "loop lands next" intro replaced with the loop/flag documentation.

For reviewers: the enabled-by-default question

Defaults ship enabled (5m / 85% / 80%, mirroring kubelet so operator intuition transfers), with dry-run as the opt-out soak path. The alternative is dry-run-by-default for a release. The target cap removes the known pathological case; happy to flip the default if you'd rather soak first.

Testing

  • imagegc_test.go: watermark/cap target-math table (including the capped-target cases), flag validation.
  • Loop + engine together, Kind: full gVisor e2e suite green under aggressive flags (15s period, 1-byte cap); planted orphan ignored by periodic passes and reclaimed at startup; shortfall backoff observed against an impossible cap; no enumeration-gate false positives.
  • GKE (6 nodes): idle pool evicted under aggressive watermarks, subsequent resumes cold-pulled cleanly with snapshot memory continuity; flags restored to defaults after.

Comment thread cmd/atelet/imagegc.go
Comment thread cmd/atelet/imagegc.go Outdated
Comment thread cmd/atelet/imagegc.go
Comment thread cmd/atelet/imagegc.go
Comment thread cmd/atelet/imagegc_test.go Outdated
Comment thread cmd/atelet/imagegc.go Outdated
igooch added a commit to igooch/substrate that referenced this pull request Aug 11, 2026
…gent-substrate#837 @ b0ead0b) into the POC

Brings the POC to parity with both open PRs in one merge (the tool
branch stacks on the loop branch): ErrIncompleteEnumeration sentinel
with engine-side gate logs dropped (single ERROR per gated pass, at
the caller), classifyGCPass + imageCacheGC receiver with snapshotted
flags, immediate first pass, honest CacheSize skip, negative-min-age
rejection, cleaned-path BasePath check, and the tool driving eviction
through the engine with WithActorsDir rooting live actors.
@igooch
igooch force-pushed the imagecache-gc-loop branch from fac02b8 to 515a5a3 Compare August 11, 2026 16:29
igooch added a commit to igooch/substrate that referenced this pull request Aug 11, 2026
igooch added 2 commits August 11, 2026 10:18
The loop that turns the eviction engine on. Every --image-cache-gc-period
(default 5m; 0 disables), a serialized, panic-recovered pass measures the
cache volume with statfs and the pool's recorded size, computes a byte
target - down to --image-cache-low-percent when volume usage crossed
--image-cache-high-percent, and/or down to --image-cache-max-bytes -
capped at the pool's own size (the cache is one tenant of a shared
volume; an uncapped target would evict everything chasing pressure it
did not cause), and hands it to Store.EvictUnused.

- Gated passes (unreadable records, garbled diffIDs, unenumerable root
  set) are handled before the shortfall accounting: zero stats plus an
  error means nothing was attempted, logged ERROR "pass skipped", never
  counted as a shortfall.
- Genuine shortfall warns with backoff, then a periodic reminder: on a
  volume under foreign pressure, shortfall is the steady state and must
  not ERROR every tick.
- atelet passes WithActorsDir(ateompath.ActorsDir) so the root set sees
  placed actors; ateompath gains the ActorsDir constant.
- --image-cache-gc-dry-run computes and logs every decision, deleting
  nothing: the production soak mechanism.
- A cache dir outside BasePath logs a warning (its watermarks would
  measure a different volume than actor state).
- README: the GC section's "loop lands next" intro replaced with the
  loop and flag documentation.

Defaults ship enabled (5m / 85% / 80%), matching kubelet so operator
intuition transfers; dry-run is the opt-out soak. Target math and flag
validation covered in imagegc_test.go.
Review fixes on the loop:

- The engine exports ErrIncompleteEnumeration, wrapped into all four
  gate returns (EvictUnused and RecoverOrphans), and the loop detects a
  gated pass with errors.Is instead of inferring it from zero stats -
  the inference was correct today but nothing in the contract promised
  it, and a future error path before the candidate loop would have
  silently reclassified real failures as skips.
- classifyGCPass extracts the skipped/shortfall/complete/quiet decision
  as a pure function with a table test, including the cases the old
  shape could not test: gated-vs-per-item errors, per-item errors with
  and without shortfall.
- A CacheSize failure now skips the pass and says so; the old path set
  cacheSize=0, which the target cap silently drove to zero while the
  log claimed "watermark target only".
- The first pass runs immediately instead of one full period after
  boot: a node starting under disk pressure should not wait 5 minutes
  (startup recovery reclaims debris, not pressure).
- Stale comment dropped from the quiet branch (it cited an orphan
  branch and gauges that do not exist here); long doc comments on
  imageCacheGCTarget and the out-of-BasePath warning tightened.
@igooch
igooch force-pushed the imagecache-gc-loop branch from 515a5a3 to 99e0dc8 Compare August 11, 2026 17:18
- The engine's gates no longer log: a gated pass produced two ERROR
  lines per tick (engine + loop), and the loop's line carries the full
  attrs plus the wrapped error. New logs the startup scan's gate once,
  at ERROR (was WARN over a second engine ERROR).
- Loop state moves onto an imageCacheGC receiver with flag values
  snapshotted at construction: runPass reads no globals (testable
  without flag juggling), and the observability phase adds its
  instruments as fields instead of more out-params. The pure decision
  functions stay free functions.
- validateImageCacheGCFlags rejects a negative --image-cache-min-age
  (it inverts the veto: the cutoff lands in the future) and cleans the
  cache-dir path before the BasePath prefix check.
- Comments: the ~1%-early watermark floor; why the pass runs even at
  target 0 (gate corruption should surface before disk pressure does).
- Flag validation test: table with t.Cleanup and the legal boundary
  high=100/low=0.
@igooch
igooch force-pushed the imagecache-gc-loop branch from 99e0dc8 to 3024a23 Compare August 11, 2026 17:28
igooch added a commit to igooch/substrate that referenced this pull request Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants