From 23f3e6019595e6fdd1ad4044dbfc738307a91a60 Mon Sep 17 00:00:00 2001 From: Shreyas Kalyan Date: Thu, 30 Jul 2026 16:03:55 -0400 Subject: [PATCH] disk: bound resident LRU index entries with WithMaxEntries The byte budget does not bound entry count: zero-byte blobs charge nothing and tiny blobs at most one 4 KiB block, while each resident entry costs a measured ~268 B of index metadata (key string, entry struct, list node, map slot). A byte-full 10 GiB cache can therefore hold ~2.6M entries (~670 MiB of metadata). WithMaxEntries adds an opt-in entry-count cap with the same eviction semantics as the byte budget, enforced on Add and during the startup scan, plus a counter separating count-cap evictions from byte-cap evictions. Co-authored-by: Cursor --- cache/disk/load.go | 8 ++ cache/disk/lru.go | 30 ++++ cache/disk/lru_maxentries_test.go | 223 ++++++++++++++++++++++++++++++ cache/disk/lru_mem_test.go | 84 +++++++++++ cache/disk/options.go | 16 +++ 5 files changed, 361 insertions(+) create mode 100644 cache/disk/lru_maxentries_test.go create mode 100644 cache/disk/lru_mem_test.go diff --git a/cache/disk/load.go b/cache/disk/load.go index 30c155b..6e1a61c 100644 --- a/cache/disk/load.go +++ b/cache/disk/load.go @@ -668,6 +668,14 @@ func (c *diskCache) loadExistingFiles(maxSizeBytes int64, cc CacheConfig) error bytesToGigaBytes(c.lru.maxSizeHardLimit)) } + if cc.maxEntries > 0 { + // Set before the Add loop below, so a cache directory holding + // more files than the cap is trimmed (oldest first) during load, + // like a restart with a smaller maxSize. + c.lru.maxEntries = cc.maxEntries + log.Printf("Will evict at max entries: %d", cc.maxEntries) + } + // Start one single goroutine running in background, continuously // waiting for files to be removed and removing them. Benchmarks on // Linux with the XFS file system have surprisingly shown that removal diff --git a/cache/disk/lru.go b/cache/disk/lru.go index 2c0f6dd..792398c 100644 --- a/cache/disk/lru.go +++ b/cache/disk/lru.go @@ -37,6 +37,16 @@ type SizedLRU struct { // cache below maxSize. maxSize int64 + // When positive, SizedLRU additionally evicts items as needed to keep + // the number of resident entries at or below maxEntries. This bounds + // the index metadata (key string, entry struct, list node, map slot - + // a fixed cost per entry regardless of blob size), which the byte + // budget alone does not: zero-byte blobs charge nothing against + // maxSize and tiny blobs charge at most one 4 KiB block, so without a + // count bound the metadata of a byte-full cache is effectively + // unbounded. Zero or negative means no entry-count bound. + maxEntries int64 + // Channel containing evicted entries removed from ll, but not yet // removed from the file system. // @@ -65,6 +75,7 @@ type SizedLRU struct { gaugeCacheLogicalBytes prometheus.Gauge counterEvictedBytes prometheus.Counter counterOverwrittenBytes prometheus.Counter + counterMaxEntriesEvicted prometheus.Counter summaryCacheItemBytes prometheus.Summary @@ -130,6 +141,10 @@ func NewSizedLRU(maxSize int64, onEvict EvictCallback, initialCapacity int) Size Name: "bazel_remote_disk_cache_overwritten_bytes_total", Help: "The total number of bytes removed from disk backend, due to put of already existing key", }), + counterMaxEntriesEvicted: prometheus.NewCounter(prometheus.CounterOpts{ + Name: "bazel_remote_disk_cache_max_entries_evictions_total", + Help: "The total number of entries evicted because the entry-count limit was reached, rather than the byte-size limit", + }), summaryCacheItemBytes: prometheus.NewSummary(prometheus.SummaryOpts{ Name: "bazel_remote_disk_cache_entry_bytes", Help: "Size of cache entries", @@ -150,6 +165,7 @@ func (c *SizedLRU) RegisterMetrics() { prometheus.MustRegister(c.gaugeCacheLogicalBytes) prometheus.MustRegister(c.counterEvictedBytes) prometheus.MustRegister(c.counterOverwrittenBytes) + prometheus.MustRegister(c.counterMaxEntriesEvicted) prometheus.MustRegister(c.summaryCacheItemBytes) // Set gauges to constant configured values to help visualize configured limits @@ -222,6 +238,20 @@ func (c *SizedLRU) Add(key string, value lruItem) (ok bool) { } } + // Entry-count eviction (see maxEntries). Only inserts of new keys can + // grow the count, and the new entry is at the front of the list, so + // the evicted tail is always an older entry. + if c.maxEntries > 0 { + for int64(c.ll.Len()) > c.maxEntries { + ele := c.ll.Back() + if ele == nil { + break + } + c.removeElement(ele) + c.counterMaxEntriesEvicted.Inc() + } + } + c.currentSize += sizeDelta c.uncompressedSize += uncompressedSizeDelta diff --git a/cache/disk/lru_maxentries_test.go b/cache/disk/lru_maxentries_test.go new file mode 100644 index 0000000..e33eb25 --- /dev/null +++ b/cache/disk/lru_maxentries_test.go @@ -0,0 +1,223 @@ +package disk + +import ( + "bytes" + "context" + "fmt" + "io" + "math" + "os" + "testing" + + "github.com/buchgr/bazel-remote/v2/cache" + testutils "github.com/buchgr/bazel-remote/v2/utils" +) + +// Tests for the entry-count bound (SizedLRU.maxEntries / WithMaxEntries), +// which caps resident index metadata independently of the byte budget. + +func TestMaxEntriesEvictsFromTail(t *testing.T) { + var evicted []string + onEvict := func(key string, value lruItem) { + evicted = append(evicted, key) + } + + lru := NewSizedLRU(math.MaxInt64, onEvict, 0) + lru.maxEntries = 2 + + for i := 0; i < 4; i++ { + if !lru.Add(fmt.Sprintf("key-%d", i), lruItem{size: 1, sizeOnDisk: 1}) { + t.Fatalf("Add of key-%d rejected", i) + } + } + + if lru.Len() != 2 { + t.Fatalf("expected 2 entries, got %d", lru.Len()) + } + for _, key := range []string{"key-2", "key-3"} { + if _, ok := lru.Peek(key); !ok { + t.Fatalf("expected %s to be resident", key) + } + } + + // Evicted entries reach onEvict through the queued-eviction channel, + // which has no background consumer in LRU-only tests; drain it once + // (all queued entries merge into a single slice). + lru.performQueuedEvictions() + if len(evicted) != 2 || evicted[0] != "key-0" || evicted[1] != "key-1" { + t.Fatalf("expected [key-0 key-1] evicted in LRU order, got %v", evicted) + } + + // The byte accounting must reflect the evictions: 2 resident single- + // block entries. + if lru.TotalSize() != 2*BlockSize { + t.Fatalf("expected currentSize %d, got %d", 2*BlockSize, lru.TotalSize()) + } +} + +func TestMaxEntriesBoundsZeroByteEntries(t *testing.T) { + // Zero-byte entries charge nothing against the byte budget, so only + // the entry-count bound limits them. + lru := NewSizedLRU(math.MaxInt64, func(string, lruItem) {}, 0) + lru.maxEntries = 10 + + for i := 0; i < 100; i++ { + if !lru.Add(fmt.Sprintf("zero-%d", i), lruItem{}) { + t.Fatalf("Add of zero-%d rejected", i) + } + } + + if lru.Len() != 10 { + t.Fatalf("expected 10 entries, got %d", lru.Len()) + } + if lru.TotalSize() != 0 { + t.Fatalf("expected zero currentSize, got %d", lru.TotalSize()) + } +} + +func TestMaxEntriesOverwriteDoesNotEvict(t *testing.T) { + var evicted []string + onEvict := func(key string, value lruItem) { + evicted = append(evicted, key) + } + + lru := NewSizedLRU(math.MaxInt64, onEvict, 0) + lru.maxEntries = 2 + + if !lru.Add("a", lruItem{size: 1, sizeOnDisk: 1}) { + t.Fatal("Add of a rejected") + } + if !lru.Add("b", lruItem{size: 1, sizeOnDisk: 1}) { + t.Fatal("Add of b rejected") + } + + // Overwriting a resident key at the cap must not change the count or + // evict the other entry. (The overwrite itself queues the replaced + // file for removal, which is not a count eviction but does reach + // onEvict.) + if !lru.Add("a", lruItem{size: 2, sizeOnDisk: 2}) { + t.Fatal("overwrite of a rejected") + } + + if lru.Len() != 2 { + t.Fatalf("expected 2 entries, got %d", lru.Len()) + } + if _, ok := lru.Peek("b"); !ok { + t.Fatal("expected b to remain resident after overwrite of a") + } + + lru.performQueuedEvictions() + if len(evicted) != 1 || evicted[0] != "a" { + t.Fatalf("expected only the replaced version of a in the eviction queue, got %v", evicted) + } +} + +func TestMaxEntriesDisabledByDefault(t *testing.T) { + lru := NewSizedLRU(math.MaxInt64, func(string, lruItem) {}, 0) + + for i := 0; i < 1000; i++ { + if !lru.Add(fmt.Sprintf("zero-%d", i), lruItem{}) { + t.Fatalf("Add of zero-%d rejected", i) + } + } + + if lru.Len() != 1000 { + t.Fatalf("expected 1000 entries with no count bound, got %d", lru.Len()) + } +} + +func TestDiskCacheWithMaxEntries(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cacheDir := tempDir(t) + defer func() { _ = os.RemoveAll(cacheDir) }() + + const maxEntries = 4 + const numBlobs = 10 + const itemSize = int64(64) + + testCacheI, err := New(cacheDir, math.MaxInt64, + WithAccessLogger(testutils.NewSilentLogger()), + WithMaxEntries(maxEntries)) + if err != nil { + t.Fatal(err) + } + testCache := testCacheI.(*diskCache) + + var hashes []string + for i := 0; i < numBlobs; i++ { + data, hash := testutils.RandomDataAndHash(itemSize) + hashes = append(hashes, hash) + err = testCache.Put(ctx, cache.CAS, hash, itemSize, + io.NopCloser(bytes.NewReader(data))) + if err != nil { + t.Fatal(err) + } + } + + if testCache.lru.Len() != maxEntries { + t.Fatalf("expected %d resident entries, got %d", + maxEntries, testCache.lru.Len()) + } + + // The most recently put blobs are resident, the oldest are not. + for _, hash := range hashes[numBlobs-maxEntries:] { + if ok, _ := testCache.Contains(ctx, cache.CAS, hash, itemSize); !ok { + t.Fatalf("expected recent blob %s to be resident", hash) + } + } + for _, hash := range hashes[:numBlobs-maxEntries] { + if ok, _ := testCache.Contains(ctx, cache.CAS, hash, itemSize); ok { + t.Fatalf("expected old blob %s to have been evicted", hash) + } + } +} + +func TestDiskCacheMaxEntriesTrimsOnLoad(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + cacheDir := tempDir(t) + defer func() { _ = os.RemoveAll(cacheDir) }() + + const maxEntries = 3 + const numBlobs = 8 + const itemSize = int64(64) + + // Fill a cache directory with no entry-count bound. + unboundedI, err := New(cacheDir, math.MaxInt64, + WithAccessLogger(testutils.NewSilentLogger())) + if err != nil { + t.Fatal(err) + } + unbounded := unboundedI.(*diskCache) + for i := 0; i < numBlobs; i++ { + data, hash := testutils.RandomDataAndHash(itemSize) + err = unbounded.Put(ctx, cache.CAS, hash, itemSize, + io.NopCloser(bytes.NewReader(data))) + if err != nil { + t.Fatal(err) + } + } + if unbounded.lru.Len() != numBlobs { + t.Fatalf("expected %d entries before reload, got %d", + numBlobs, unbounded.lru.Len()) + } + + // Reloading the same directory with a bound trims it during load, + // like a restart with a smaller maxSize. New only returns after the + // eviction backlog has been removed from disk. + boundedI, err := New(cacheDir, math.MaxInt64, + WithAccessLogger(testutils.NewSilentLogger()), + WithMaxEntries(maxEntries)) + if err != nil { + t.Fatal(err) + } + bounded := boundedI.(*diskCache) + + if bounded.lru.Len() != maxEntries { + t.Fatalf("expected %d entries after bounded reload, got %d", + maxEntries, bounded.lru.Len()) + } +} diff --git a/cache/disk/lru_mem_test.go b/cache/disk/lru_mem_test.go new file mode 100644 index 0000000..9fb0629 --- /dev/null +++ b/cache/disk/lru_mem_test.go @@ -0,0 +1,84 @@ +package disk + +// Measurement of resident LRU metadata cost per entry: the heap bytes +// retained by one cache entry independent of blob size (key string, entry +// struct, list.Element, map slot). This is the constant behind the +// entry-count cap in the embedder's memory envelope; run with: +// +// go test ./cache/disk/ -run TestLRUEntryMetadataCost -v +// +// It asserts only a loose sanity bound so drift in Go runtime internals +// does not break the build, and logs the measured value for use in sizing. + +import ( + "crypto/rand" + "encoding/hex" + "fmt" + "runtime" + "testing" +) + +func heapInUse() uint64 { + runtime.GC() + runtime.GC() + var ms runtime.MemStats + runtime.ReadMemStats(&ms) + return ms.HeapAlloc +} + +func TestLRUEntryMetadataCost(t *testing.T) { + const numEntries = 1_000_000 + + // Realistic lookup keys: "cas/<64 hex>" plus an lruItem with the + // random filename component populated, matching what the startup scan + // and Put paths store. + keys := make([]string, numEntries) + var buf [32]byte + for i := range keys { + if _, err := rand.Read(buf[:]); err != nil { + t.Fatal(err) + } + keys[i] = "cas/" + hex.EncodeToString(buf[:]) + } + + lru := NewSizedLRU(int64(numEntries)*BlockSize, nil, 0) + + // Baseline after key generation so only LRU-internal allocations are + // attributed to the per-entry cost... except the keys themselves are + // retained by the LRU, so charge them too by measuring before keys + // are considered reachable only from the LRU. Simplest correct + // accounting: measure baseline before insertion, keep the keys slice + // alive, and add the average key size explicitly afterwards. + baseline := heapInUse() + + for i, k := range keys { + if !lru.Add(k, lruItem{size: 1, sizeOnDisk: 1, random: fmt.Sprintf("%08d", i)}) { + t.Fatalf("Add rejected entry %d", i) + } + } + + after := heapInUse() + + // Keep both the LRU and the keys reachable across the measurement, or + // the GC inside heapInUse collects them first. + if lru.Len() != numEntries { + t.Fatalf("expected %d entries, got %d", numEntries, lru.Len()) + } + runtime.KeepAlive(keys) + + perEntry := float64(after-baseline) / numEntries + + // The key bytes (~68 B) and its string header were allocated during + // key generation and are retained by the LRU afterwards; count them + // in the reported total. + const keyBytes = 4 + 64 + total := perEntry + keyBytes + + t.Logf("LRU metadata per entry: %.0f B internal + %d B key = %.0f B total (%d entries, heap %d -> %d)", + perEntry, keyBytes, total, numEntries, baseline, after) + + // Sanity bounds only; the log line is the deliverable. + if total < 100 || total > 1000 { + t.Fatalf("implausible per-entry metadata cost: %.0f B", total) + } +} diff --git a/cache/disk/options.go b/cache/disk/options.go index fe12244..0dfef63 100644 --- a/cache/disk/options.go +++ b/cache/disk/options.go @@ -17,6 +17,7 @@ type CacheConfig struct { diskCache *diskCache // Assumed to be non-nil. metrics *metricsDecorator // May be nil. maxSizeHardLimit int64 + maxEntries int64 } func WithStorageMode(mode string) Option { @@ -59,6 +60,21 @@ func WithZstdLimits(limits zstdimpl.ZstdLimits) Option { } } +// WithMaxEntries bounds the number of entries resident in the cache index, +// evicting least-recently-used entries when the bound is exceeded - the same +// semantics as the byte budget, but counting entries. Each resident entry +// costs a fixed ~270 bytes of process memory (key string, entry struct, list +// node, map slot) regardless of blob size, while charging at most one 4 KiB +// block against the byte budget (zero-byte blobs charge nothing), so without +// a count bound the index metadata of a byte-full cache is effectively +// unbounded. n <= 0 (the default) disables the bound. +func WithMaxEntries(n int64) Option { + return func(c *CacheConfig) error { + c.maxEntries = n + return nil + } +} + func WithMaxBlobSize(size int64) Option { return func(c *CacheConfig) error { if size <= 0 {