From 67012bc8629879ec043868544294ddfbab00fab3 Mon Sep 17 00:00:00 2001 From: Ivy Date: Mon, 10 Aug 2026 14:38:17 -0700 Subject: [PATCH 1/3] atelet: watermark-driven image cache GC loop 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. --- cmd/atelet/imagegc.go | 228 ++++++++++++++++++++++++++++++++ cmd/atelet/imagegc_test.go | 134 +++++++++++++++++++ cmd/atelet/main.go | 8 ++ internal/ateompath/ateompath.go | 8 +- internal/imagecache/README.md | 20 ++- 5 files changed, 389 insertions(+), 9 deletions(-) create mode 100644 cmd/atelet/imagegc.go create mode 100644 cmd/atelet/imagegc_test.go diff --git a/cmd/atelet/imagegc.go b/cmd/atelet/imagegc.go new file mode 100644 index 000000000..34652a934 --- /dev/null +++ b/cmd/atelet/imagegc.go @@ -0,0 +1,228 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +// The image-cache GC loop. +// +// A single serialized pass on a fixed period (the kubelet's shape — the +// heavy deletion work happens outside the pull path's locks, so there is +// nothing to duty-cycle). Each pass measures the cache volume with statfs +// and the pool's own recorded size, computes how many bytes to free — +// down to the low watermark when volume usage crossed the high one, and/or +// down to --image-cache-max-bytes when the pool outgrew it — and hands the +// larger target to Store.EvictUnused. + +import ( + "context" + "fmt" + "log/slog" + "os" + "runtime/debug" + "strings" + "time" + + "github.com/agent-substrate/substrate/internal/ateompath" + "github.com/agent-substrate/substrate/internal/imagecache" + "github.com/spf13/pflag" + "golang.org/x/sys/unix" +) + +var ( + imageCacheGCPeriod = pflag.Duration("image-cache-gc-period", 5*time.Minute, "How often to run the image cache eviction pass. 0 disables eviction entirely.") + imageCacheHighPct = pflag.Int("image-cache-high-percent", 85, "Cache-volume usage percentage above which eviction starts.") + imageCacheLowPct = pflag.Int("image-cache-low-percent", 80, "Cache-volume usage percentage eviction frees down to. Must be lower than --image-cache-high-percent.") + imageCacheMaxBytes = pflag.Int64("image-cache-max-bytes", 0, "Absolute cap on the summed size of cached layers, evicted down to independently of the volume watermarks. 0 means no cap.") + imageCacheMinAge = pflag.Duration("image-cache-min-age", 2*time.Minute, "Layers and image records younger than this are never evicted (protects images pulled but not yet mounted).") + imageCacheGCDryRun = pflag.Bool("image-cache-gc-dry-run", false, "Compute and log eviction decisions without deleting anything.") +) + +const ( + // shortfallWarnLimit is how many consecutive shortfalls warn before the + // loop backs off to shortfallReminderEvery. + shortfallWarnLimit = 3 + // shortfallReminderEvery keeps a persistent shortfall visible without a + // line per tick (at the 5m default: roughly hourly). + shortfallReminderEvery = 12 +) + +func validateImageCacheGCFlags() error { + if *imageCacheHighPct < 1 || *imageCacheHighPct > 100 { + return fmt.Errorf("--image-cache-high-percent %d out of range [1,100]", *imageCacheHighPct) + } + if *imageCacheLowPct < 0 || *imageCacheLowPct >= *imageCacheHighPct { + return fmt.Errorf("--image-cache-low-percent %d must be in [0,%d)", *imageCacheLowPct, *imageCacheHighPct) + } + // The watermark is measured on the cache dir's filesystem while the root + // set is read from the actors dir. If an operator points the cache at a + // different volume than BasePath, those are different filesystems: the + // pass would evict based on disk pressure the cache doesn't contribute + // to. Warn rather than fail — a separate cache volume is a legitimate + // (and recommended, for IOPS) configuration, it just wants its own + // watermarks. + if !strings.HasPrefix(*imageCacheDir, ateompath.BasePath+string(os.PathSeparator)) { + slog.Warn("Image cache dir is outside the ateom base path; its volume watermarks are measured separately from actor state", + slog.String("image_cache_dir", *imageCacheDir), + slog.String("actors_dir", ateompath.ActorsDir)) + } + return nil +} + +// imageCacheGCTarget computes the bytes an eviction pass should free. +// +// Watermark half (kubelet's formula): when volume usage is at or above the +// high watermark, free down to the low one. Cap half: when the pool's +// recorded size exceeds maxBytes, free the difference. The pass pursues the +// larger — but never more than the cache actually holds. +// +// That cache-size ceiling is the important difference from kubelet, which +// owns its imagefs and can therefore assume the whole shortfall is its to +// free. Our cache is one tenant of a volume it shares with containerd's +// image store, kubelet, logs, actor uppers and local snapshots, so the raw +// watermark target asks the cache to free far more than it holds (measured: +// a 105 GiB volume at 98% yields an 18.9 GiB target against an 11 MiB +// cache). The pass would then evict every unrooted image on every tick — +// a permanent 0% hit rate, turning every actor start back into a full +// re-pull — while barely moving disk usage. Capped at its own size, the +// cache gives back everything it can and no more; the residual shortfall +// is reported (it is someone else's disk), not chased. +func imageCacheGCTarget(capacity, available uint64, cacheSize, maxBytes int64, highPct, lowPct int) int64 { + var target int64 + if capacity > 0 { + usedPct := 100 - int(available*100/capacity) + if usedPct >= highPct { + // Free enough that available climbs back to (100-lowPct)% of + // capacity. + target = int64(capacity)*int64(100-lowPct)/100 - int64(available) + } + } + if maxBytes > 0 && cacheSize > maxBytes { + if over := cacheSize - maxBytes; over > target { + target = over + } + } + if target > cacheSize { + target = cacheSize + } + if target < 0 { + target = 0 + } + return target +} + +// runImageCacheGC runs eviction passes on the configured period until ctx +// is done. Passes are strictly serialized: a slow pass delays the next +// tick rather than overlapping it. +func runImageCacheGC(ctx context.Context, store *imagecache.Store, cacheDir string) { + ticker := time.NewTicker(*imageCacheGCPeriod) + defer ticker.Stop() + + consecutiveShortfalls := 0 + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + } + + runImageCacheGCPass(ctx, store, cacheDir, &consecutiveShortfalls) + } +} + +// runImageCacheGCPass performs one pass. It recovers from panics: this is a +// background janitor, and a bug here (or a malformed directory an operator +// dropped into the pool) must not take atelet down with it and strand every +// actor on the node. +func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir string, consecutiveShortfalls *int) { + defer func() { + if r := recover(); r != nil { + slog.ErrorContext(ctx, "Image cache GC pass panicked; skipping this pass", + slog.Any("panic", r), slog.String("stack", string(debug.Stack()))) + } + }() + + var st unix.Statfs_t + if err := unix.Statfs(cacheDir, &st); err != nil { + slog.WarnContext(ctx, "Image cache GC: statfs failed", slog.String("dir", cacheDir), slog.Any("err", err)) + return + } + capacity := st.Blocks * uint64(st.Bsize) + available := st.Bavail * uint64(st.Bsize) + + // A sizing error is not fatal to the pass: the watermark half of the + // target needs only statfs. + cacheSize, err := store.CacheSize() + if err != nil { + slog.WarnContext(ctx, "Image cache GC: sizing the pool failed; watermark target only this pass", + slog.Any("err", err)) + cacheSize = 0 + } + + target := imageCacheGCTarget(capacity, available, cacheSize, *imageCacheMaxBytes, *imageCacheHighPct, *imageCacheLowPct) + + tStart := time.Now() + stats, err := store.EvictUnused(ctx, target, *imageCacheGCDryRun) + attrs := []any{ + slog.Int64("target_bytes", target), + slog.Int64("freed_bytes", stats.FreedBytes), + slog.Int("evicted_images", stats.EvictedImages), + slog.Int("evicted_layers", stats.EvictedLayers), + slog.Int("candidates", stats.Candidates), + slog.Int("rooted_images", stats.RootedImages), + slog.Int("orphan_layers", stats.OrphanLayers), + slog.Int("skipped_rooted", stats.SkippedRooted), + slog.Int("skipped_fresh", stats.SkippedFresh), + slog.Int64("cache_size_bytes", cacheSize), + slog.Bool("dry_run", *imageCacheGCDryRun), + slog.Duration("took", time.Since(tStart)), + } + if err != nil { + if stats.Candidates == 0 && stats.EvictedImages == 0 { + // Enumeration-gated pass: nothing was attempted, so this is a + // wedged pass, not a shortfall — the shortfall accounting below + // would misread the zero stats as "cache cannot give more". + slog.ErrorContext(ctx, "Image cache GC pass skipped", append(attrs, slog.Any("err", err))...) + return + } + // Per-item failures were already skipped inside the pass; log the + // aggregate and let the next pass retry. + slog.WarnContext(ctx, "Image cache GC pass finished with errors", append(attrs, slog.Any("err", err))...) + } + switch { + case target > 0 && stats.FreedBytes < target: + // Everything eligible was evicted and the target still wasn't + // met: the remainder is rooted, pinned, or fresh. Now that the + // target is capped at the cache's own size, a shortfall means + // the cache genuinely cannot give back more — which on a volume + // under foreign pressure is the steady state, so this must not + // log at ERROR every tick. Warn on the first few, then drop to a + // periodic reminder. + *consecutiveShortfalls++ + switch { + case *consecutiveShortfalls <= shortfallWarnLimit: + slog.WarnContext(ctx, "Image cache GC could not reach target", + append(attrs, slog.Int("consecutive", *consecutiveShortfalls))...) + case *consecutiveShortfalls%shortfallReminderEvery == 0: + slog.WarnContext(ctx, "Image cache GC still short of target; the remaining pressure is not the image cache's to free", + append(attrs, slog.Int("consecutive", *consecutiveShortfalls))...) + } + case target > 0: + *consecutiveShortfalls = 0 + slog.InfoContext(ctx, "Image cache GC pass complete", attrs...) + default: + // No disk pressure and no orphans: stay quiet. The gauges are + // the "GC is alive" signal, not a log line per tick. + *consecutiveShortfalls = 0 + } +} diff --git a/cmd/atelet/imagegc_test.go b/cmd/atelet/imagegc_test.go new file mode 100644 index 000000000..0772af96e --- /dev/null +++ b/cmd/atelet/imagegc_test.go @@ -0,0 +1,134 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package main + +import "testing" + +func TestImageCacheGCTarget(t *testing.T) { + const gib = int64(1 << 30) + tests := []struct { + name string + capacity, available uint64 + cacheSize, maxBytes int64 + highPct, lowPct int + want int64 + }{ + { + name: "below high watermark: no target", + capacity: 100 * uint64(gib), available: 30 * uint64(gib), // 70% used + highPct: 85, lowPct: 80, + want: 0, + }, + { + name: "at high watermark: free down to low", + capacity: 100 * uint64(gib), available: 10 * uint64(gib), // 90% used + cacheSize: 50 * gib, // cache is big enough to cover the shortfall + highPct: 85, lowPct: 80, + // available must climb to 20% of capacity: free 20GiB - 10GiB. + want: 10 * gib, + }, + { + name: "exactly high watermark triggers", + capacity: 100 * uint64(gib), available: 15 * uint64(gib), // 85% used + cacheSize: 50 * gib, + highPct: 85, lowPct: 80, + want: 5 * gib, + }, + { + // The kubelet formula assumes it owns the filesystem; we don't. + // A near-full boot disk shared with containerd/kubelet/logs must + // not ask an 11 MiB cache to free 18.9 GiB — uncapped, that + // evicts the entire cache on every tick forever (0% hit rate) + // without materially moving disk usage. + name: "watermark target capped at what the cache holds", + capacity: 105 * uint64(gib), available: 2 * uint64(gib), // ~98% used + cacheSize: 11 << 20, + highPct: 85, lowPct: 80, + want: 11 << 20, + }, + { + name: "empty cache under volume pressure: nothing to free", + capacity: 100 * uint64(gib), available: 1 * uint64(gib), + cacheSize: 0, + highPct: 85, lowPct: 80, + want: 0, + }, + { + name: "max-bytes cap independent of watermarks", + capacity: 100 * uint64(gib), available: 90 * uint64(gib), // 10% used + cacheSize: 8 * gib, maxBytes: 5 * gib, + highPct: 85, lowPct: 80, + want: 3 * gib, + }, + { + name: "both: larger target wins", + capacity: 100 * uint64(gib), available: 10 * uint64(gib), // watermark target 10GiB + cacheSize: 60 * gib, maxBytes: 40 * gib, // cap target 20GiB + highPct: 85, lowPct: 80, + want: 20 * gib, + }, + { + name: "max-bytes zero means no cap", + capacity: 100 * uint64(gib), available: 90 * uint64(gib), + cacheSize: 500 * gib, maxBytes: 0, + highPct: 85, lowPct: 80, + want: 0, + }, + { + name: "zero capacity: watermark half disabled", + capacity: 0, available: 0, + cacheSize: 2 * gib, maxBytes: gib, + highPct: 85, lowPct: 80, + want: gib, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := imageCacheGCTarget(tc.capacity, tc.available, tc.cacheSize, tc.maxBytes, tc.highPct, tc.lowPct) + if got != tc.want { + t.Errorf("imageCacheGCTarget() = %d, want %d", got, tc.want) + } + }) + } +} + +func TestValidateImageCacheGCFlags(t *testing.T) { + setFlags := func(high, low int) { + *imageCacheHighPct = high + *imageCacheLowPct = low + } + defer setFlags(85, 80) + + setFlags(85, 80) + if err := validateImageCacheGCFlags(); err != nil { + t.Errorf("defaults rejected: %v", err) + } + setFlags(101, 80) + if err := validateImageCacheGCFlags(); err == nil { + t.Error("high=101 accepted") + } + setFlags(85, 85) + if err := validateImageCacheGCFlags(); err == nil { + t.Error("low == high accepted") + } + setFlags(85, 90) + if err := validateImageCacheGCFlags(); err == nil { + t.Error("low > high accepted") + } + setFlags(85, -1) + if err := validateImageCacheGCFlags(); err == nil { + t.Error("negative low accepted") + } +} diff --git a/cmd/atelet/main.go b/cmd/atelet/main.go index c59deadd6..bb1e5d5dd 100644 --- a/cmd/atelet/main.go +++ b/cmd/atelet/main.go @@ -164,13 +164,21 @@ func main() { } } + if err := validateImageCacheGCFlags(); err != nil { + serverboot.Fatal(ctx, "Invalid image cache GC flags", err) + } imageCache, err := imagecache.New(*imageCacheDir, imagecache.WithAuthenticator(gcpRegistryAuthn), imagecache.WithLocalhostRegistryReplacement(*localhostRegistryReplacement), + imagecache.WithActorsDir(ateompath.ActorsDir), + imagecache.WithMinAge(*imageCacheMinAge), ) if err != nil { serverboot.Fatal(ctx, "Failed to open image cache", err) } + if *imageCacheGCPeriod > 0 { + go runImageCacheGC(ctx, imageCache, *imageCacheDir) + } anonGCSClient, err := storage.NewClient(ctx, option.WithoutAuthentication()) if err != nil { diff --git a/internal/ateompath/ateompath.go b/internal/ateompath/ateompath.go index 8cdac4f10..5c47c2d49 100644 --- a/internal/ateompath/ateompath.go +++ b/internal/ateompath/ateompath.go @@ -35,6 +35,11 @@ var ( // and in every ateom pod (which mounts them as overlay lowerdirs). ImageCacheDir = filepath.Join(BasePath, "image-cache") + // ActorsDir holds the per-actor state directories (see ActorPath). The + // image cache's eviction root-set scan reads the bundle overlay specs + // under it. + ActorsDir = filepath.Join(BasePath, "actors") + // CredentialBrokerSocket is the node-local atelet socket used by atunnel // to request credentials for the worker's current actor assignment. CredentialBrokerSocket = filepath.Join(BasePath, "credential-broker.sock") @@ -81,8 +86,7 @@ func AteomNetNSPath(podUID string) string { func ActorPath(actorUID string) string { return filepath.Join( - BasePath, - "actors", + ActorsDir, actorUID, ) } diff --git a/internal/imagecache/README.md b/internal/imagecache/README.md index 46e52c909..428edeb97 100644 --- a/internal/imagecache/README.md +++ b/internal/imagecache/README.md @@ -164,13 +164,19 @@ an actor's bundle directory (via `/proc/self/mountinfo`) before atelet wipes it — called from the checkpoint cleanup path in ateom-gvisor and `teardownActor` in ateom-microvm. -## Garbage collection (engine; the periodic loop lands next) - -`gc.go` holds the eviction engine. Nothing in production calls -`Store.EvictUnused` yet — the watermark-driven periodic loop and its -flags (period, high/low watermarks, max-bytes cap, min-age, dry-run) -arrive with the next change — so cached data still only grows for now. -The one behavior `New` gains today is the startup orphan scan below. +## Garbage collection + +`gc.go` holds the eviction engine; atelet drives it as a periodic pass +(`--image-cache-gc-period`, default 5m; `0` disables it). Each tick +measures the cache volume with `statfs` and the pool's own size from the +per-layer `size` files, then computes a byte target: free down to +`--image-cache-low-percent` when volume usage reaches +`--image-cache-high-percent`, and/or down to `--image-cache-max-bytes` — +**capped at the pool's own size**, because this cache is one tenant of a +shared volume and an uncapped target would evict the whole cache trying +to fix disk pressure it didn't cause. `--image-cache-gc-dry-run` +computes and logs every decision while mutating nothing: the +recommended way to soak the policy on a live fleet. **One pass** (`Store.EvictUnused(ctx, targetBytes, dryRun)`): From 7243e9fae4dd4905287ea7105defd308a66a9bf2 Mon Sep 17 00:00:00 2001 From: Ivy Date: Mon, 10 Aug 2026 15:06:02 -0700 Subject: [PATCH 2/3] atelet: classify GC passes by contract; skip honestly on sizing failure 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. --- cmd/atelet/imagegc.go | 117 ++++++++++++++++++--------------- cmd/atelet/imagegc_test.go | 34 +++++++++- internal/imagecache/gc.go | 14 ++-- internal/imagecache/gc_test.go | 7 ++ 4 files changed, 115 insertions(+), 57 deletions(-) diff --git a/cmd/atelet/imagegc.go b/cmd/atelet/imagegc.go index 34652a934..a6ed69d0e 100644 --- a/cmd/atelet/imagegc.go +++ b/cmd/atelet/imagegc.go @@ -26,6 +26,7 @@ package main import ( "context" + "errors" "fmt" "log/slog" "os" @@ -64,13 +65,10 @@ func validateImageCacheGCFlags() error { if *imageCacheLowPct < 0 || *imageCacheLowPct >= *imageCacheHighPct { return fmt.Errorf("--image-cache-low-percent %d must be in [0,%d)", *imageCacheLowPct, *imageCacheHighPct) } - // The watermark is measured on the cache dir's filesystem while the root - // set is read from the actors dir. If an operator points the cache at a - // different volume than BasePath, those are different filesystems: the - // pass would evict based on disk pressure the cache doesn't contribute - // to. Warn rather than fail — a separate cache volume is a legitimate - // (and recommended, for IOPS) configuration, it just wants its own - // watermarks. + // Outside BasePath the watermarks measure a different volume than + // actor state, so the pass would chase pressure the cache doesn't + // contribute to. Warn, don't fail: a separate cache volume is + // legitimate (recommended for IOPS) — it just wants its own numbers. if !strings.HasPrefix(*imageCacheDir, ateompath.BasePath+string(os.PathSeparator)) { slog.Warn("Image cache dir is outside the ateom base path; its volume watermarks are measured separately from actor state", slog.String("image_cache_dir", *imageCacheDir), @@ -79,24 +77,18 @@ func validateImageCacheGCFlags() error { return nil } -// imageCacheGCTarget computes the bytes an eviction pass should free. +// imageCacheGCTarget computes the bytes a pass should free: the larger +// of the watermark shortfall (kubelet's formula — usage at highPct frees +// down to lowPct) and the pool's overage past maxBytes, but never more +// than the cache actually holds. // -// Watermark half (kubelet's formula): when volume usage is at or above the -// high watermark, free down to the low one. Cap half: when the pool's -// recorded size exceeds maxBytes, free the difference. The pass pursues the -// larger — but never more than the cache actually holds. -// -// That cache-size ceiling is the important difference from kubelet, which -// owns its imagefs and can therefore assume the whole shortfall is its to -// free. Our cache is one tenant of a volume it shares with containerd's -// image store, kubelet, logs, actor uppers and local snapshots, so the raw -// watermark target asks the cache to free far more than it holds (measured: -// a 105 GiB volume at 98% yields an 18.9 GiB target against an 11 MiB -// cache). The pass would then evict every unrooted image on every tick — -// a permanent 0% hit rate, turning every actor start back into a full -// re-pull — while barely moving disk usage. Capped at its own size, the -// cache gives back everything it can and no more; the residual shortfall -// is reported (it is someone else's disk), not chased. +// The cache-size ceiling is the difference from kubelet, which owns its +// imagefs. This cache is one tenant of a shared volume, so the raw +// watermark target can dwarf it (measured: a 105 GiB volume at 98% asks +// an 11 MiB cache for 18.9 GiB), and an uncapped pass would evict +// everything every tick for a permanent 0% hit rate. Capped, the cache +// gives back all it can; the residual shortfall is someone else's disk — +// reported, not chased. func imageCacheGCTarget(capacity, available uint64, cacheSize, maxBytes int64, highPct, lowPct int) int64 { var target int64 if capacity > 0 { @@ -125,10 +117,13 @@ func imageCacheGCTarget(capacity, available uint64, cacheSize, maxBytes int64, h // is done. Passes are strictly serialized: a slow pass delays the next // tick rather than overlapping it. func runImageCacheGC(ctx context.Context, store *imagecache.Store, cacheDir string) { + consecutiveShortfalls := 0 + // First pass immediately: a node booting under disk pressure must not + // wait a full period (startup recovery reclaims debris, not pressure). + runImageCacheGCPass(ctx, store, cacheDir, &consecutiveShortfalls) + ticker := time.NewTicker(*imageCacheGCPeriod) defer ticker.Stop() - - consecutiveShortfalls := 0 for { select { case <-ctx.Done(): @@ -160,13 +155,13 @@ func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir capacity := st.Blocks * uint64(st.Bsize) available := st.Bavail * uint64(st.Bsize) - // A sizing error is not fatal to the pass: the watermark half of the - // target needs only statfs. + // Same failure class as the enumeration gates (ReadDir of the layer + // pool): fail toward retention, retry next tick. cacheSize, err := store.CacheSize() if err != nil { - slog.WarnContext(ctx, "Image cache GC: sizing the pool failed; watermark target only this pass", + slog.WarnContext(ctx, "Image cache GC: sizing the pool failed; skipping this pass", slog.Any("err", err)) - cacheSize = 0 + return } target := imageCacheGCTarget(capacity, available, cacheSize, *imageCacheMaxBytes, *imageCacheHighPct, *imageCacheLowPct) @@ -187,27 +182,21 @@ func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir slog.Bool("dry_run", *imageCacheGCDryRun), slog.Duration("took", time.Since(tStart)), } + outcome := classifyGCPass(err, target, stats.FreedBytes) + if outcome == gcPassSkipped { + slog.ErrorContext(ctx, "Image cache GC pass skipped", append(attrs, slog.Any("err", err))...) + return + } if err != nil { - if stats.Candidates == 0 && stats.EvictedImages == 0 { - // Enumeration-gated pass: nothing was attempted, so this is a - // wedged pass, not a shortfall — the shortfall accounting below - // would misread the zero stats as "cache cannot give more". - slog.ErrorContext(ctx, "Image cache GC pass skipped", append(attrs, slog.Any("err", err))...) - return - } - // Per-item failures were already skipped inside the pass; log the - // aggregate and let the next pass retry. + // Per-item failures; each retries next pass. slog.WarnContext(ctx, "Image cache GC pass finished with errors", append(attrs, slog.Any("err", err))...) } - switch { - case target > 0 && stats.FreedBytes < target: - // Everything eligible was evicted and the target still wasn't - // met: the remainder is rooted, pinned, or fresh. Now that the - // target is capped at the cache's own size, a shortfall means - // the cache genuinely cannot give back more — which on a volume - // under foreign pressure is the steady state, so this must not - // log at ERROR every tick. Warn on the first few, then drop to a - // periodic reminder. + switch outcome { + case gcPassShortfall: + // The capped target means a shortfall is "the cache cannot give + // more" — on a volume under foreign pressure, the steady state. + // Warn on the first few, then a periodic reminder, never + // ERROR-per-tick. *consecutiveShortfalls++ switch { case *consecutiveShortfalls <= shortfallWarnLimit: @@ -217,12 +206,36 @@ func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir slog.WarnContext(ctx, "Image cache GC still short of target; the remaining pressure is not the image cache's to free", append(attrs, slog.Int("consecutive", *consecutiveShortfalls))...) } - case target > 0: + case gcPassComplete: *consecutiveShortfalls = 0 slog.InfoContext(ctx, "Image cache GC pass complete", attrs...) - default: - // No disk pressure and no orphans: stay quiet. The gauges are - // the "GC is alive" signal, not a log line per tick. + default: // gcPassQuiet: no target, nothing to say. *consecutiveShortfalls = 0 } } + +// gcPassOutcome classifies one finished pass for logging and backoff. +type gcPassOutcome int + +const ( + gcPassSkipped gcPassOutcome = iota // gated: nothing was attempted + gcPassShortfall // ran; target not met + gcPassComplete // ran; target met + gcPassQuiet // no target +) + +// classifyGCPass keeps the gate-vs-shortfall distinction testable and on +// contract (the engine's sentinel), not inferred from stats: a gated pass +// means "repair the named file", never "the cache cannot give more". +func classifyGCPass(err error, target, freed int64) gcPassOutcome { + switch { + case errors.Is(err, imagecache.ErrIncompleteEnumeration): + return gcPassSkipped + case target > 0 && freed < target: + return gcPassShortfall + case target > 0: + return gcPassComplete + default: + return gcPassQuiet + } +} diff --git a/cmd/atelet/imagegc_test.go b/cmd/atelet/imagegc_test.go index 0772af96e..60375c446 100644 --- a/cmd/atelet/imagegc_test.go +++ b/cmd/atelet/imagegc_test.go @@ -14,7 +14,13 @@ package main -import "testing" +import ( + "errors" + "fmt" + "testing" + + "github.com/agent-substrate/substrate/internal/imagecache" +) func TestImageCacheGCTarget(t *testing.T) { const gib = int64(1 << 30) @@ -132,3 +138,29 @@ func TestValidateImageCacheGCFlags(t *testing.T) { t.Error("negative low accepted") } } + +func TestClassifyGCPass(t *testing.T) { + gated := fmt.Errorf("pass gated: %w", imagecache.ErrIncompleteEnumeration) + perItem := errors.New("while removing retired layer: permission denied") + cases := []struct { + name string + err error + target, freed int64 + want gcPassOutcome + }{ + {"gated pass", gated, 100, 0, gcPassSkipped}, + {"gated wins even with zero target", gated, 0, 0, gcPassSkipped}, + {"per-item errors are not a skip", perItem, 100, 100, gcPassComplete}, + {"per-item errors with shortfall", perItem, 100, 40, gcPassShortfall}, + {"shortfall", nil, 100, 40, gcPassShortfall}, + {"target met", nil, 100, 100, gcPassComplete}, + {"no target", nil, 0, 0, gcPassQuiet}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := classifyGCPass(tc.err, tc.target, tc.freed); got != tc.want { + t.Errorf("classifyGCPass(%v, %d, %d) = %d, want %d", tc.err, tc.target, tc.freed, got, tc.want) + } + }) + } +} diff --git a/internal/imagecache/gc.go b/internal/imagecache/gc.go index 1b4e7633a..19aefddc6 100644 --- a/internal/imagecache/gc.go +++ b/internal/imagecache/gc.go @@ -60,6 +60,12 @@ import ( "golang.org/x/sync/errgroup" ) +// ErrIncompleteEnumeration marks a pass that did nothing because the +// image records or bundle specs could not be fully enumerated. Callers +// use errors.Is to tell "nothing was attempted" (repair the named file; +// not a shortfall) from per-item failures on a pass that ran. +var ErrIncompleteEnumeration = errors.New("image cache enumeration incomplete") + // --- root set --- // RootSet is the set of images and layers that eviction must not touch, @@ -245,7 +251,7 @@ func (s *Store) EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) // partial scan would retire layers a running actor still mounts. slog.ErrorContext(ctx, "Image cache eviction pass skipped: bundle specs could not be fully enumerated", slog.Any("err", rootsErr)) - return stats, rootsErr + return stats, errors.Join(ErrIncompleteEnumeration, rootsErr) } cutoff := time.Now().Add(-s.minAge) @@ -257,7 +263,7 @@ func (s *Store) EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) // delete, and every later pass retries. slog.ErrorContext(ctx, "Image cache eviction pass skipped: image records could not be fully enumerated", slog.Any("err", listErr)) - return stats, listErr + return stats, errors.Join(ErrIncompleteEnumeration, listErr) } stats.Candidates = len(candidates) @@ -487,14 +493,14 @@ func (s *Store) RecoverOrphans(ctx context.Context) (EvictStats, error) { if rootsErr != nil { slog.ErrorContext(ctx, "Image cache startup orphan scan skipped: bundle specs could not be fully enumerated; orphaned layers (if any) will persist until the specs are repaired", slog.Any("err", rootsErr)) - return stats, rootsErr + return stats, errors.Join(ErrIncompleteEnumeration, rootsErr) } cutoff := time.Now().Add(-s.minAge) _, refcount, complete, listErr := s.listEviction(roots, cutoff, &stats) if !complete { slog.ErrorContext(ctx, "Image cache startup orphan scan skipped: image records could not be fully enumerated; orphaned layers (if any) will persist until the records are repaired", slog.Any("err", listErr)) - return stats, listErr + return stats, errors.Join(ErrIncompleteEnumeration, listErr) } retired, errs := s.sweepOrphanLayers(ctx, roots, refcount, cutoff, false, &stats) diff --git a/internal/imagecache/gc_test.go b/internal/imagecache/gc_test.go index 35357c3fd..a4fae1160 100644 --- a/internal/imagecache/gc_test.go +++ b/internal/imagecache/gc_test.go @@ -17,6 +17,7 @@ package imagecache import ( "archive/tar" "context" + "errors" "math" "os" "path/filepath" @@ -332,6 +333,9 @@ func TestEvictUnusedSkipsPassOnUnreadableRoots(t *testing.T) { if err == nil { t.Fatal("EvictUnused returned no error with an unenumerable root set") } + if !errors.Is(err, ErrIncompleteEnumeration) { + t.Errorf("gate error does not wrap ErrIncompleteEnumeration: %v", err) + } if stats.EvictedImages != 0 || stats.EvictedLayers != 0 { t.Errorf("gated pass still evicted: %+v", stats) } @@ -389,6 +393,9 @@ func TestEvictUnusedSkipsPassOnBadRecord(t *testing.T) { if err == nil { t.Fatal("EvictUnused returned no error on a bad record") } + if !errors.Is(err, ErrIncompleteEnumeration) { + t.Errorf("gate error does not wrap ErrIncompleteEnumeration: %v", err) + } if stats.EvictedImages != 0 || stats.EvictedLayers != 0 { t.Errorf("gated pass still evicted: %+v", stats) } From cc425809773de6709bdf5d414a23035fccb6361d Mon Sep 17 00:00:00 2001 From: Ivy Date: Tue, 11 Aug 2026 08:58:58 -0700 Subject: [PATCH 3/3] atelet: address loop review - single gated-pass log, gc state receiver - 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. --- cmd/atelet/imagegc.go | 87 ++++++++++++++++++++++--------- cmd/atelet/imagegc_test.go | 45 +++++++++------- cmd/atelet/main.go | 2 +- internal/imagecache/gc.go | 10 +--- internal/imagecache/imagecache.go | 13 +++-- 5 files changed, 101 insertions(+), 56 deletions(-) diff --git a/cmd/atelet/imagegc.go b/cmd/atelet/imagegc.go index a6ed69d0e..1bc7e1dac 100644 --- a/cmd/atelet/imagegc.go +++ b/cmd/atelet/imagegc.go @@ -30,6 +30,7 @@ import ( "fmt" "log/slog" "os" + "path/filepath" "runtime/debug" "strings" "time" @@ -65,11 +66,16 @@ func validateImageCacheGCFlags() error { if *imageCacheLowPct < 0 || *imageCacheLowPct >= *imageCacheHighPct { return fmt.Errorf("--image-cache-low-percent %d must be in [0,%d)", *imageCacheLowPct, *imageCacheHighPct) } + if *imageCacheMinAge < 0 { + // A negative min-age inverts the veto (the cutoff lands in the + // future), making just-pulled layers evictable. + return fmt.Errorf("--image-cache-min-age %v must be >= 0", *imageCacheMinAge) + } // Outside BasePath the watermarks measure a different volume than // actor state, so the pass would chase pressure the cache doesn't // contribute to. Warn, don't fail: a separate cache volume is // legitimate (recommended for IOPS) — it just wants its own numbers. - if !strings.HasPrefix(*imageCacheDir, ateompath.BasePath+string(os.PathSeparator)) { + if !strings.HasPrefix(filepath.Clean(*imageCacheDir), ateompath.BasePath+string(os.PathSeparator)) { slog.Warn("Image cache dir is outside the ateom base path; its volume watermarks are measured separately from actor state", slog.String("image_cache_dir", *imageCacheDir), slog.String("actors_dir", ateompath.ActorsDir)) @@ -92,6 +98,8 @@ func validateImageCacheGCFlags() error { func imageCacheGCTarget(capacity, available uint64, cacheSize, maxBytes int64, highPct, lowPct int) int64 { var target int64 if capacity > 0 { + // Integer floor of the available fraction: usage reads up to ~1% + // high, so eviction can trigger just before the nominal watermark. usedPct := 100 - int(available*100/capacity) if usedPct >= highPct { // Free enough that available climbs back to (100-lowPct)% of @@ -113,16 +121,42 @@ func imageCacheGCTarget(capacity, available uint64, cacheSize, maxBytes int64, h return target } -// runImageCacheGC runs eviction passes on the configured period until ctx -// is done. Passes are strictly serialized: a slow pass delays the next -// tick rather than overlapping it. -func runImageCacheGC(ctx context.Context, store *imagecache.Store, cacheDir string) { - consecutiveShortfalls := 0 +// imageCacheGC is the loop's state: configuration snapshotted from the +// flags at construction (the pass logic never reads globals, so it is +// testable without flag juggling) plus the shortfall-backoff counter. +type imageCacheGC struct { + store *imagecache.Store + cacheDir string + period time.Duration + highPct int + lowPct int + maxBytes int64 + dryRun bool + + consecutiveShortfalls int +} + +func newImageCacheGC(store *imagecache.Store, cacheDir string) *imageCacheGC { + return &imageCacheGC{ + store: store, + cacheDir: cacheDir, + period: *imageCacheGCPeriod, + highPct: *imageCacheHighPct, + lowPct: *imageCacheLowPct, + maxBytes: *imageCacheMaxBytes, + dryRun: *imageCacheGCDryRun, + } +} + +// Run executes eviction passes on the configured period until ctx is +// done. Passes are strictly serialized: a slow pass delays the next tick +// rather than overlapping it. +func (g *imageCacheGC) Run(ctx context.Context) { // First pass immediately: a node booting under disk pressure must not // wait a full period (startup recovery reclaims debris, not pressure). - runImageCacheGCPass(ctx, store, cacheDir, &consecutiveShortfalls) + g.runPass(ctx) - ticker := time.NewTicker(*imageCacheGCPeriod) + ticker := time.NewTicker(g.period) defer ticker.Stop() for { select { @@ -131,15 +165,15 @@ func runImageCacheGC(ctx context.Context, store *imagecache.Store, cacheDir stri case <-ticker.C: } - runImageCacheGCPass(ctx, store, cacheDir, &consecutiveShortfalls) + g.runPass(ctx) } } -// runImageCacheGCPass performs one pass. It recovers from panics: this is a +// runPass performs one pass. It recovers from panics: this is a // background janitor, and a bug here (or a malformed directory an operator // dropped into the pool) must not take atelet down with it and strand every // actor on the node. -func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir string, consecutiveShortfalls *int) { +func (g *imageCacheGC) runPass(ctx context.Context) { defer func() { if r := recover(); r != nil { slog.ErrorContext(ctx, "Image cache GC pass panicked; skipping this pass", @@ -148,8 +182,8 @@ func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir }() var st unix.Statfs_t - if err := unix.Statfs(cacheDir, &st); err != nil { - slog.WarnContext(ctx, "Image cache GC: statfs failed", slog.String("dir", cacheDir), slog.Any("err", err)) + if err := unix.Statfs(g.cacheDir, &st); err != nil { + slog.WarnContext(ctx, "Image cache GC: statfs failed", slog.String("dir", g.cacheDir), slog.Any("err", err)) return } capacity := st.Blocks * uint64(st.Bsize) @@ -157,17 +191,22 @@ func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir // Same failure class as the enumeration gates (ReadDir of the layer // pool): fail toward retention, retry next tick. - cacheSize, err := store.CacheSize() + cacheSize, err := g.store.CacheSize() if err != nil { slog.WarnContext(ctx, "Image cache GC: sizing the pool failed; skipping this pass", slog.Any("err", err)) return } - target := imageCacheGCTarget(capacity, available, cacheSize, *imageCacheMaxBytes, *imageCacheHighPct, *imageCacheLowPct) + target := imageCacheGCTarget(capacity, available, cacheSize, g.maxBytes, g.highPct, g.lowPct) tStart := time.Now() - stats, err := store.EvictUnused(ctx, target, *imageCacheGCDryRun) + // Runs even at target 0: the enumeration gates should surface a + // corrupt record or spec on the next tick, not first under disk + // pressure. Cost: the full root-set scan (a ReadDir per actor, a + // read per bundle spec) plus a read per image record — hundreds of + // small reads on a busy node. Deliberate, and cheap at this period. + stats, err := g.store.EvictUnused(ctx, target, g.dryRun) attrs := []any{ slog.Int64("target_bytes", target), slog.Int64("freed_bytes", stats.FreedBytes), @@ -179,7 +218,7 @@ func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir slog.Int("skipped_rooted", stats.SkippedRooted), slog.Int("skipped_fresh", stats.SkippedFresh), slog.Int64("cache_size_bytes", cacheSize), - slog.Bool("dry_run", *imageCacheGCDryRun), + slog.Bool("dry_run", g.dryRun), slog.Duration("took", time.Since(tStart)), } outcome := classifyGCPass(err, target, stats.FreedBytes) @@ -197,20 +236,20 @@ func runImageCacheGCPass(ctx context.Context, store *imagecache.Store, cacheDir // more" — on a volume under foreign pressure, the steady state. // Warn on the first few, then a periodic reminder, never // ERROR-per-tick. - *consecutiveShortfalls++ + g.consecutiveShortfalls++ switch { - case *consecutiveShortfalls <= shortfallWarnLimit: + case g.consecutiveShortfalls <= shortfallWarnLimit: slog.WarnContext(ctx, "Image cache GC could not reach target", - append(attrs, slog.Int("consecutive", *consecutiveShortfalls))...) - case *consecutiveShortfalls%shortfallReminderEvery == 0: + append(attrs, slog.Int("consecutive", g.consecutiveShortfalls))...) + case g.consecutiveShortfalls%shortfallReminderEvery == 0: slog.WarnContext(ctx, "Image cache GC still short of target; the remaining pressure is not the image cache's to free", - append(attrs, slog.Int("consecutive", *consecutiveShortfalls))...) + append(attrs, slog.Int("consecutive", g.consecutiveShortfalls))...) } case gcPassComplete: - *consecutiveShortfalls = 0 + g.consecutiveShortfalls = 0 slog.InfoContext(ctx, "Image cache GC pass complete", attrs...) default: // gcPassQuiet: no target, nothing to say. - *consecutiveShortfalls = 0 + g.consecutiveShortfalls = 0 } } diff --git a/cmd/atelet/imagegc_test.go b/cmd/atelet/imagegc_test.go index 60375c446..86e418012 100644 --- a/cmd/atelet/imagegc_test.go +++ b/cmd/atelet/imagegc_test.go @@ -18,6 +18,7 @@ import ( "errors" "fmt" "testing" + "time" "github.com/agent-substrate/substrate/internal/imagecache" ) @@ -111,31 +112,35 @@ func TestImageCacheGCTarget(t *testing.T) { } func TestValidateImageCacheGCFlags(t *testing.T) { - setFlags := func(high, low int) { + setFlags := func(high, low int, minAge time.Duration) { *imageCacheHighPct = high *imageCacheLowPct = low + *imageCacheMinAge = minAge } - defer setFlags(85, 80) + t.Cleanup(func() { setFlags(85, 80, 2*time.Minute) }) - setFlags(85, 80) - if err := validateImageCacheGCFlags(); err != nil { - t.Errorf("defaults rejected: %v", err) - } - setFlags(101, 80) - if err := validateImageCacheGCFlags(); err == nil { - t.Error("high=101 accepted") - } - setFlags(85, 85) - if err := validateImageCacheGCFlags(); err == nil { - t.Error("low == high accepted") - } - setFlags(85, 90) - if err := validateImageCacheGCFlags(); err == nil { - t.Error("low > high accepted") + cases := []struct { + name string + high, low int + minAge time.Duration + wantErr bool + }{ + {"defaults", 85, 80, 2 * time.Minute, false}, + {"boundary high=100 low=0", 100, 0, 0, false}, + {"high over 100", 101, 80, 0, true}, + {"low equals high", 85, 85, 0, true}, + {"low above high", 85, 90, 0, true}, + {"negative low", 85, -1, 0, true}, + {"negative min-age inverts the veto", 85, 80, -time.Second, true}, } - setFlags(85, -1) - if err := validateImageCacheGCFlags(); err == nil { - t.Error("negative low accepted") + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + setFlags(tc.high, tc.low, tc.minAge) + err := validateImageCacheGCFlags() + if (err != nil) != tc.wantErr { + t.Errorf("high=%d low=%d minAge=%v: err=%v, wantErr=%v", tc.high, tc.low, tc.minAge, err, tc.wantErr) + } + }) } } diff --git a/cmd/atelet/main.go b/cmd/atelet/main.go index bb1e5d5dd..adbe3f993 100644 --- a/cmd/atelet/main.go +++ b/cmd/atelet/main.go @@ -177,7 +177,7 @@ func main() { serverboot.Fatal(ctx, "Failed to open image cache", err) } if *imageCacheGCPeriod > 0 { - go runImageCacheGC(ctx, imageCache, *imageCacheDir) + go newImageCacheGC(imageCache, *imageCacheDir).Run(ctx) } anonGCSClient, err := storage.NewClient(ctx, option.WithoutAuthentication()) diff --git a/internal/imagecache/gc.go b/internal/imagecache/gc.go index 19aefddc6..cddd3624a 100644 --- a/internal/imagecache/gc.go +++ b/internal/imagecache/gc.go @@ -249,8 +249,8 @@ func (s *Store) EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) if rootsErr != nil { // Same shape as the record gate below: refcounts and roots from a // partial scan would retire layers a running actor still mounts. - slog.ErrorContext(ctx, "Image cache eviction pass skipped: bundle specs could not be fully enumerated", - slog.Any("err", rootsErr)) + // Not logged here — the caller logs the gated pass once, with its + // own context (see ErrIncompleteEnumeration). return stats, errors.Join(ErrIncompleteEnumeration, rootsErr) } cutoff := time.Now().Add(-s.minAge) @@ -261,8 +261,6 @@ func (s *Store) EvictUnused(ctx context.Context, targetBytes int64, dryRun bool) // be retired while that record still names it. Fail the whole pass // toward retention; the error names the records to repair or // delete, and every later pass retries. - slog.ErrorContext(ctx, "Image cache eviction pass skipped: image records could not be fully enumerated", - slog.Any("err", listErr)) return stats, errors.Join(ErrIncompleteEnumeration, listErr) } stats.Candidates = len(candidates) @@ -491,15 +489,11 @@ func (s *Store) RecoverOrphans(ctx context.Context) (EvictStats, error) { roots, rootsErr := s.InUse() if rootsErr != nil { - slog.ErrorContext(ctx, "Image cache startup orphan scan skipped: bundle specs could not be fully enumerated; orphaned layers (if any) will persist until the specs are repaired", - slog.Any("err", rootsErr)) return stats, errors.Join(ErrIncompleteEnumeration, rootsErr) } cutoff := time.Now().Add(-s.minAge) _, refcount, complete, listErr := s.listEviction(roots, cutoff, &stats) if !complete { - slog.ErrorContext(ctx, "Image cache startup orphan scan skipped: image records could not be fully enumerated; orphaned layers (if any) will persist until the records are repaired", - slog.Any("err", listErr)) return stats, errors.Join(ErrIncompleteEnumeration, listErr) } diff --git a/internal/imagecache/imagecache.go b/internal/imagecache/imagecache.go index b5b6f0eab..da6fb04e4 100644 --- a/internal/imagecache/imagecache.go +++ b/internal/imagecache/imagecache.go @@ -224,10 +224,17 @@ func New(root string, opts ...Option) (*Store, error) { return nil, err } // Startup-only orphan recovery (see RecoverOrphans for why it must not - // run during normal operation). Failure is logged inside, never fatal: - // a corrupt record must not keep atelet from serving actors. + // run during normal operation). Never fatal: a corrupt record must not + // keep atelet from serving actors. Gated means nothing was attempted + // (orphans persist until the named file is repaired and the next + // restart); per-item errors mean the scan ran and reclaimed what it + // could. if _, err := s.RecoverOrphans(context.Background()); err != nil { - slog.Warn("Image cache startup orphan recovery incomplete", slog.Any("err", err)) + if errors.Is(err, ErrIncompleteEnumeration) { + slog.Error("Image cache startup orphan scan skipped", slog.Any("err", err)) + } else { + slog.Warn("Image cache startup orphan recovery incomplete", slog.Any("err", err)) + } } return s, nil }