From d8fff69570df90001b7d90516029ca8839a09385 Mon Sep 17 00:00:00 2001 From: sileshidev-lab Date: Thu, 6 Aug 2026 12:25:29 +0300 Subject: [PATCH] lib/store: force cleanup now removes in-memory cache entries DeleteCacheFile had no CAStore-level override, unlike GetCacheFileStat and ListCacheFiles which both already check the memory cache. Calls fell straight through to the disk-only cacheStore.DeleteCacheFile, so entries that only lived in the memory cache were never actually removed. This meant the force-cleanup endpoint would list an in-memory entry as a deletion candidate (ListCacheFiles already includes memory entries) but silently fail to remove it, since the delete step only touched disk. Add a DeleteCacheFile override that removes the entry from the memory cache first, then falls through to the disk delete. A not-exist error from the disk delete is only swallowed when we know the entry was memory-only (never had a disk file to begin with), so the not-exist signal is preserved for entries that genuinely don't exist anywhere. Fixes #490 --- lib/store/ca_store.go | 21 ++++++++ lib/store/ca_store_test.go | 108 +++++++++++++++++++++++++++++++++++++ 2 files changed, 129 insertions(+) diff --git a/lib/store/ca_store.go b/lib/store/ca_store.go index f2c2f7653..bec50f8fa 100644 --- a/lib/store/ca_store.go +++ b/lib/store/ca_store.go @@ -529,6 +529,27 @@ func (s *CAStore) ListCacheFiles() ([]string, error) { return result, nil } +// DeleteCacheFile overrides cacheStore.DeleteCacheFile to also remove the +// entry from the memory cache, if present. Without this override, entries +// that only exist in the memory cache (never persisted to disk) are never +// actually removed by callers like the force-cleanup endpoint, which lists +// candidates via ListCacheFiles (which does include memory entries) but +// deletes them via this method. +func (s *CAStore) DeleteCacheFile(name string) error { + var inMemCache bool + if s.memCache != nil { + inMemCache = s.memCache.Get(name) != nil + s.memCache.Remove(name) + } + + err := s.cacheStore.DeleteCacheFile(name) + if err != nil && os.IsNotExist(err) && inMemCache { + // Entry only existed in memory; there was never a disk file to delete. + return nil + } + return err +} + var _ os.FileInfo = &memoryFileInfo{} // memoryFileInfo implements os.FileInfo for memory cache entries. diff --git a/lib/store/ca_store_test.go b/lib/store/ca_store_test.go index ed6e83e0a..8d943f4ea 100644 --- a/lib/store/ca_store_test.go +++ b/lib/store/ca_store_test.go @@ -1209,6 +1209,114 @@ func TestCAStore_ListCacheFiles(t *testing.T) { } } +func TestCAStore_DeleteCacheFile(t *testing.T) { + t.Run("removes memory-only entry and does not error", func(t *testing.T) { + config, cleanup := CAStoreConfigFixture() + defer cleanup() + config.MemoryCache = MemoryCacheConfig{ + Enabled: true, + MaxSize: 1024 * 1024, + TTL: time.Hour, + } + + s, err := NewCAStore(config, tally.NoopScope) + require.NoError(t, err) + defer s.Close() + + data := []byte("in memory only") + digest, err := core.NewDigester().FromBytes(data) + require.NoError(t, err) + name := digest.Hex() + + added := s.memCache.Add(&cache.MemoryEntry{ + Name: name, + Data: data, + CreatedAt: time.Now(), + }) + require.True(t, added) + require.NotNil(t, s.memCache.Get(name), "entry should be in memory cache before delete") + + err = s.DeleteCacheFile(name) + require.NoError(t, err) + require.Nil(t, s.memCache.Get(name), "entry should be removed from memory cache after delete") + + // Confirm it's really gone from the combined listing too, not just + // from a direct memCache.Get check. + files, err := s.ListCacheFiles() + require.NoError(t, err) + for _, f := range files { + require.NotEqual(t, name, f, "deleted entry should not appear in ListCacheFiles") + } + }) + + t.Run("still deletes a disk-only entry as before", func(t *testing.T) { + config, cleanup := CAStoreConfigFixture() + defer cleanup() + config.MemoryCache = MemoryCacheConfig{ + Enabled: true, + MaxSize: 1024 * 1024, + TTL: time.Hour, + } + + s, err := NewCAStore(config, tally.NoopScope) + require.NoError(t, err) + defer s.Close() + + data := []byte("on disk only") + digest, err := core.NewDigester().FromBytes(data) + require.NoError(t, err) + name := digest.Hex() + + err = s.CreateCacheFile(name, bytes.NewReader(data)) + require.NoError(t, err) + + err = s.DeleteCacheFile(name) + require.NoError(t, err) + + _, err = s.GetCacheFileStat(name) + require.True(t, os.IsNotExist(err), "disk file should be gone after delete") + }) + + t.Run("falls back to disk-only behavior when memory cache is disabled", func(t *testing.T) { + config, cleanup := CAStoreConfigFixture() + defer cleanup() + config.MemoryCache = MemoryCacheConfig{Enabled: false} + + s, err := NewCAStore(config, tally.NoopScope) + require.NoError(t, err) + defer s.Close() + require.Nil(t, s.memCache) + + data := []byte("disk entry, no memory cache") + digest, err := core.NewDigester().FromBytes(data) + require.NoError(t, err) + name := digest.Hex() + + err = s.CreateCacheFile(name, bytes.NewReader(data)) + require.NoError(t, err) + + err = s.DeleteCacheFile(name) + require.NoError(t, err) + }) + + t.Run("returns not-exist error for an entry that is nowhere", func(t *testing.T) { + config, cleanup := CAStoreConfigFixture() + defer cleanup() + config.MemoryCache = MemoryCacheConfig{ + Enabled: true, + MaxSize: 1024 * 1024, + TTL: time.Hour, + } + + s, err := NewCAStore(config, tally.NoopScope) + require.NoError(t, err) + defer s.Close() + + err = s.DeleteCacheFile("does-not-exist-anywhere") + require.True(t, os.IsNotExist(err), "expected a not-exist error, got: %v", err) + }) +} + func TestGetCacheFileMetadata_MemoryCache_NoCopy(t *testing.T) { require := require.New(t)