serve: prune cached hashes to bound cache growth#417
Merged
Conversation
The serve query service wrote one cache entry per queried commit SHA to --cacheDir and never removed any, so a long-running server's cache grew without bound. Add optional, off-by-default pruning: - PrunableHashCacheStorage capability interface + LocalDiskHashCacheStorage impl: LRU by last-use (get() touches mtime on a cache hit), evicting oldest-first by any mix of max age / max entry count / max total size. get() is hardened against a file pruned out from under a concurrent read. - CachePruner: a daemon scheduler that sweeps once at startup then every --cachePruneInterval (default 1h); a no-op when no limit is set; never throws, so a transient FS error cannot kill the sweeper thread. - serve flags --cacheMaxAge (e.g. 7d), --cacheMaxEntries, --cacheMaxSize (e.g. 10GB), --cachePruneInterval, plus two reusable picocli converters (Duration, byte size). Limits are deliberately NOT folded into the config fingerprint (retention does not affect hash content). A limit set against a non-prunable backend (a future S3 store) is reported and ignored rather than silently pretended-to. With no --cacheMax* flag set the cache is never pruned (prior behavior). Tests: prune logic on a real filesystem, touch-on-hit LRU, both converters, CLI wiring, and an end-to-end test that drives the real scheduler against real on-disk storage and asserts files are deleted down to the limit. Docs updated in readme_template.md and the regenerated README. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
tinder-maxwellelliott
added a commit
that referenced
this pull request
Jul 9, 2026
Resolves conflicts from #417 (serve cache pruning) landing on master: - HashCacheStorage.kt: LocalDiskHashCacheStorage now implements BOTH MeasurableHashCacheStorage (stats(), this PR) and PrunableHashCacheStorage (prune(), #417), keeping both method bodies and the shared touch-on-get LRU. - ServeCommand.buildAndStartServer wires the MetricsService alongside #417's CachePruner startup; imports for both features merged. - cli/BUILD and LocalDiskHashCacheStorageTest keep both features' test targets and cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
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.
What
The
servequery service caches one hash entry per queried commit SHA under--cacheDirand never removed any, so a long-running server's cache grew without bound. This adds optional, off-by-default pruning that bounds the on-disk footprint.New
serveflags (all optional; unset = unlimited):--cacheMaxAge7d,36h,90m.--cacheMaxEntries--cacheMaxSize10GB,500MB, or a bare byte count.--cachePruneInterval1h.With no
--cacheMax*flag set, the cache is never pruned (prior behavior).How
PrunableHashCacheStoragecapability interface (extends the byte-orientedHashCacheStorage).LocalDiskHashCacheStorageimplements it: LRU by last-use —get()bumps the file's mtime on a cache hit, so a base revision under active query is not expired out from under live traffic — andprune()evicts oldest-first by any combination of age / count / size.get()is also hardened against a file pruned out from under a concurrent read.CachePruner— a single daemonScheduledExecutorthat sweeps once at startup, then every--cachePruneInterval(fixed delay, so a slow sweep can't pile up). No-op when no limit is set; never throws, so a transient FS error can't kill the sweeper thread.DurationConverter/ByteSizeConverter— small reusable picocli converters for the human-friendly flag values.ServeCommand's startup/shutdown lifecycle. A backend that can't self-prune (a future S3 store, which would use a bucket lifecycle policy) is detected and the--cacheMax*flags are reported-and-ignored rather than silently pretended-to.Notes for reviewers
HashCacheStorageinterface stays byte-oriented so a remote backend still drops in unchanged (RFC RFC: Bazel Query Service #29 S3 direction).Testing
.jsonfiles left alone, both converters, and CLI wiring/parsing (incl. the1hdefault parsed through the converter).E2ETest#testServeEndToEnd(exercises the modified cache-hit path) passes.ktfmt/buildifierclean.tools/readme_template.md+ regeneratedREADME.md.