From 33c958ad1cdeb2fba7ab8b8b533da49983a1ef8b Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 05:54:42 +0000 Subject: [PATCH 1/8] Test that a cache hit takes the content type from memory (closes #70) Tests for keeping each variant's content type in memory. A second hit must still get the stored content type after the variant's .meta file is deleted, whether the first came from storing the variant or from reading it after a restart; this fails now. A variant removed by EvictToLimit, or whose file was deleted from disk, must be a miss and must not be served, and concurrent stores, reads and evictions run under the race detector; these pass now and guard the change. Model: opus-5-5 --- internal/imgcache/metacache_internal_test.go | 222 +++++++++++++++++++ internal/imgcache/testutil_internal_test.go | 1 + 2 files changed, 223 insertions(+) create mode 100644 internal/imgcache/metacache_internal_test.go diff --git a/internal/imgcache/metacache_internal_test.go b/internal/imgcache/metacache_internal_test.go new file mode 100644 index 0000000..0a2d368 --- /dev/null +++ b/internal/imgcache/metacache_internal_test.go @@ -0,0 +1,222 @@ +package imgcache + +import ( + "bytes" + "errors" + "fmt" + "io" + "os" + "sync" + "testing" + "time" +) + +// webpRequest returns a request for a 100x100 WebP variant of path. +func webpRequest(path string) *ImageRequest { + return &ImageRequest{ + SourceHost: testHostCDN, + SourcePath: path, + Size: Size{Width: 100, Height: 100}, + Format: FormatWebP, + Quality: 85, + FitMode: FitCover, + } +} + +// assertVariantServed checks that GetVariant serves key with the given +// content and the image/webp content type storeEvictionTestVariant stores. +func assertVariantServed(t *testing.T, cache *Cache, key VariantKey, content []byte) { + t.Helper() + + reader, size, contentType, err := cache.GetVariant(key) + if err != nil { + t.Fatalf("GetVariant(%s) error = %v", key, err) + } + + defer func() { _ = reader.Close() }() + + got, err := io.ReadAll(reader) + if err != nil { + t.Fatalf("reading variant %s: %v", key, err) + } + + if !bytes.Equal(got, content) { + t.Errorf("GetVariant(%s) content = %q, want %q", key, got, content) + } + + if size != int64(len(content)) { + t.Errorf("GetVariant(%s) size = %d, want %d", key, size, len(content)) + } + + if contentType != testContentTypeWebP { + t.Errorf("GetVariant(%s) content type = %q, want %q", + key, contentType, testContentTypeWebP) + } +} + +// assertVariantNotFound checks that GetVariant refuses key with +// ErrNotFound. +func assertVariantNotFound(t *testing.T, cache *Cache, key VariantKey) { + t.Helper() + + reader, _, _, err := cache.GetVariant(key) + if err == nil { + _ = reader.Close() + } + + if !errors.Is(err, ErrNotFound) { + t.Errorf("GetVariant(%s) error = %v, want ErrNotFound", key, err) + } +} + +// assertLookupMisses checks that Lookup reports request as a miss. +func assertLookupMisses(t *testing.T, cache *Cache, request *ImageRequest) { + t.Helper() + + lookup, err := cache.Lookup(t.Context(), request) + if err != nil { + t.Fatalf("Lookup(%s) error = %v", request.SourcePath, err) + } + + if lookup.Hit { + t.Errorf("Lookup(%s) is a hit, want a miss", request.SourcePath) + } +} + +// TestSecondHitDoesNotReadMetaFile checks that once a variant has been +// stored or read, a hit takes its content type from memory: with the +// .meta file deleted, GetVariant must still return the stored content +// type rather than the application/octet-stream it uses without one. +func TestSecondHitDoesNotReadMetaFile(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<20) + content := []byte("webp variant bytes") + + storeEvictionTestVariant(t, cache, testVariantKeyOne, content) + + // A second Cache on the same state directory starts with nothing in + // memory, as pixad does after a restart, so its first read uses the + // .meta file. + restarted, err := NewCache(cache.db, cache.config) + if err != nil { + t.Fatalf("NewCache() error = %v", err) + } + + assertVariantServed(t, restarted, testVariantKeyOne, content) + + err = os.Remove(cache.variants.keyToPath(testVariantKeyOne) + ".meta") + if err != nil { + t.Fatalf("removing .meta file: %v", err) + } + + assertVariantServed(t, cache, testVariantKeyOne, content) + assertVariantServed(t, restarted, testVariantKeyOne, content) +} + +// TestEvictedVariantIsNotServed checks that a variant the evictor +// removed is a miss and cannot be read, although it had been stored +// and served before. +func TestEvictedVariantIsNotServed(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1500) + + oldRequest := webpRequest("/old.jpg") + newRequest := webpRequest("/new.jpg") + oldKey := CacheKey(oldRequest) + newKey := CacheKey(newRequest) + oldContent := bytes.Repeat([]byte{0x01}, 1000) + newContent := bytes.Repeat([]byte{0x02}, 1000) + + storeEvictionTestVariant(t, cache, oldKey, oldContent) + storeEvictionTestVariant(t, cache, newKey, newContent) + assertVariantServed(t, cache, oldKey, oldContent) + setVariantLastAccessed(t, cache, oldKey, time.Now().Add(-time.Hour)) + + err := cache.EvictToLimit(t.Context()) + if err != nil { + t.Fatalf("EvictToLimit() error = %v", err) + } + + assertLookupMisses(t, cache, oldRequest) + assertVariantNotFound(t, cache, oldKey) + assertVariantServed(t, cache, newKey, newContent) +} + +// TestVariantDeletedFromDiskIsNotServed checks that a variant whose +// file was deleted by something other than the evictor cannot be read, +// and is a miss afterwards. +func TestVariantDeletedFromDiskIsNotServed(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<20) + + request := webpRequest("/deleted.jpg") + key := CacheKey(request) + content := []byte("webp variant bytes") + + storeEvictionTestVariant(t, cache, key, content) + assertVariantServed(t, cache, key, content) + + err := os.Remove(cache.variants.keyToPath(key)) + if err != nil { + t.Fatalf("removing variant file: %v", err) + } + + assertVariantNotFound(t, cache, key) + assertLookupMisses(t, cache, request) +} + +// TestConcurrentVariantStoreReadAndEvict stores, reads and evicts +// variants from several goroutines at once, for the race detector. +func TestConcurrentVariantStoreReadAndEvict(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<20) + ctx := t.Context() + + var wg sync.WaitGroup + + for goroutine := range 8 { + wg.Go(func() { + key := VariantKey(fmt.Sprintf("aabbccdd01%02d", goroutine)) + content := []byte(key) + + for range 20 { + err := cache.StoreVariant( + ctx, key, bytes.NewReader(content), testContentTypeWebP) + if err != nil { + t.Errorf("StoreVariant(%s) error = %v", key, err) + + return + } + + reader, _, contentType, err := cache.GetVariant(key) + if err != nil { + t.Errorf("GetVariant(%s) error = %v", key, err) + + return + } + + _ = reader.Close() + + if contentType != testContentTypeWebP { + t.Errorf("GetVariant(%s) content type = %q, want %q", + key, contentType, testContentTypeWebP) + } + + err = cache.evictVariant(ctx, key) + if err != nil { + t.Errorf("evictVariant(%s) error = %v", key, err) + + return + } + + assertVariantNotFound(t, cache, key) + } + }) + } + + wg.Wait() +} diff --git a/internal/imgcache/testutil_internal_test.go b/internal/imgcache/testutil_internal_test.go index 1ca7c11..b29f750 100644 --- a/internal/imgcache/testutil_internal_test.go +++ b/internal/imgcache/testutil_internal_test.go @@ -24,6 +24,7 @@ const ( testHostExample = "example.com" testPathCat = "/photos/cat.jpg" testContentTypeJPEG = "image/jpeg" + testContentTypeWebP = "image/webp" testHeaderContentType = "Content-Type" ) -- 2.54.0 From ffe64d5fe05cf75d14a073aa3e4f081d52127c25 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 06:01:27 +0000 Subject: [PATCH 2/8] Keep variant content types in memory for cache hits (closes #70) Cache.metaCache was declared and never used, so every hit read and parsed the variant's .meta file. It is now an LRU of up to 10,000 content types (hashicorp/golang-lru/v2), filled by StoreVariant and by GetVariant after it reads a .meta file. For a variant it holds, Lookup skips the disk check and GetVariant skips the .meta read; the variant file is still opened and its size taken from it. Eviction removes the entry before deleting the files, and GetVariant removes it when the file will not open, so a missing variant is never served. The cap is a constant, not a setting. README.md describes it. Model: opus-5-5 --- README.md | 5 ++- TODO.md | 9 +++++ go.mod | 1 + go.sum | 2 ++ internal/imgcache/cache.go | 63 ++++++++++++++++++++++++++--------- internal/imgcache/eviction.go | 6 +++- internal/imgcache/storage.go | 31 +++++++++++------ 7 files changed, 89 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 808d192..9321291 100644 --- a/README.md +++ b/README.md @@ -86,7 +86,10 @@ prevent abuse, and allowlisted source hosts for open access. Multiple source paths may reference the same content blob; the database tracks references rather than using filesystem refcounting. -In-process caching of request-to-output mappings targets 1-5k r/s. +Toward a target of 1-5k r/s, pixa keeps in memory the content types of +the 10,000 transformed images most recently cached or served, so a +cache hit on one of them reads only the image file from disk and not +the metadata file stored beside it. ### Routes diff --git a/TODO.md b/TODO.md index 4674052..1173c47 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,15 @@ P2: security: referer blacklist # Completed Steps +- 2026-09-29 variant content types kept in memory (closes #70): + `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU + (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by + `GetVariant` after it reads a `.meta` file; for a variant it holds, `Lookup` + skips the check of the disk and `GetVariant` skips the `.meta` read, still + opening the variant file and taking the size from it; eviction removes the + entry before deleting the files, and `GetVariant` removes it when the file + will not open; the cap is a constant, not a setting; the unused `variantMeta` + type is gone; `README.md` describes it. - 2026-09-29 maintenance mode refuses image requests (closes #71): while `maintenance_mode` is on, `/v1/image/` and `/v1/e/` answer 503 with a `Retry-After` header and the JSON error body, from one middleware in diff --git a/go.mod b/go.mod index f5c4c98..4a13f35 100644 --- a/go.mod +++ b/go.mod @@ -14,6 +14,7 @@ require ( github.com/go-chi/httprate v0.16.0 github.com/gorilla/csrf v1.7.3 github.com/gorilla/securecookie v1.1.2 + github.com/hashicorp/golang-lru/v2 v2.0.7 github.com/prometheus/client_golang v1.23.2 github.com/slok/go-http-metrics v0.13.0 github.com/spf13/cobra v1.10.2 diff --git a/go.sum b/go.sum index f9a4f3d..c4542f6 100644 --- a/go.sum +++ b/go.sum @@ -228,6 +228,8 @@ github.com/hashicorp/go-version v1.2.1/go.mod h1:fltr4n8CU8Ke44wwGCBoEymUuxUHl09 github.com/hashicorp/golang-lru v0.5.0/go.mod h1:/m3WP610KZHVQ1SGc6re/UDhFvYD7pJ4Ao+sR/qLZy8= github.com/hashicorp/golang-lru v0.5.4 h1:YDjusn29QI/Das2iO9M0BHnIbxPeyuCHsjMW+lJfyTc= github.com/hashicorp/golang-lru v0.5.4/go.mod h1:iADmTwqILo4mZ8BN3D2Q6+9jd8WM5uGBxy+E8yxSoD4= +github.com/hashicorp/golang-lru/v2 v2.0.7 h1:a+bsQ5rvGLjzHuww6tVxozPZFVghXaHOwFs4luLUK2k= +github.com/hashicorp/golang-lru/v2 v2.0.7/go.mod h1:QeFd9opnmA6QUJc5vARoKUSoFhyfM2/ZepoAG6RGpeM= github.com/hashicorp/hcl v1.0.1-vault-7 h1:ag5OxFVy3QYTFTJODRzTKVZ6xvdfLLCA1cy/Y6xGI0I= github.com/hashicorp/hcl v1.0.1-vault-7/go.mod h1:XYhtn6ijBSAj6n4YqAaf7RBPS4I06AItNorpy+MoQNM= github.com/hashicorp/logutils v1.0.0/go.mod h1:QIAnNjmIWmVIIkWDTG1z5v++HQmx9WQRO+LraFDTW64= diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index ddac1b6..7a3726c 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -14,6 +14,7 @@ import ( "sync" "time" + lru "github.com/hashicorp/golang-lru/v2" "sneak.berlin/go/pixa/internal/httpfetcher" ) @@ -26,6 +27,10 @@ var ( // HTTP status code for successful fetch. const httpStatusOK = 200 +// metaCacheSize is how many variants' content types metaCache holds. A +// variant not among them is served as before, reading its .meta file. +const metaCacheSize = 10000 + // CacheConfig holds cache configuration. type CacheConfig struct { StateDir string @@ -49,12 +54,6 @@ type CacheConfig struct { Logger *slog.Logger } -// variantMeta stores content type for fast cache hits without reading .meta file. -type variantMeta struct { - ContentType string - Size int64 -} - // Cache implements the caching layer for the image proxy. type Cache struct { db *sql.DB @@ -76,9 +75,10 @@ type Cache struct { evictionStarted bool evictionStopOnce sync.Once - // In-memory cache of variant metadata (content type, size) to avoid - // reading .meta files - metaCache map[VariantKey]variantMeta + // metaCache holds the content types of the variants most recently + // stored or served, so a hit does not read the variant's .meta file. + // It never stands in for the variant file, which is always opened. + metaCache *lru.Cache[VariantKey, string] // contentLocks serializes StoreSource and evictSourceBlob per // content hash, closing the race window between an eviction's row @@ -101,6 +101,11 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) { log = slog.Default() } + metaCache, err := lru.New[VariantKey, string](metaCacheSize) + if err != nil { + return nil, fmt.Errorf("failed to create variant content type cache: %w", err) + } + c := &Cache{ db: db, config: config, @@ -109,7 +114,7 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) { evictionPressure: make(chan struct{}, 1), evictionStop: make(chan struct{}), evictionDone: make(chan struct{}), - metaCache: make(map[VariantKey]variantMeta), + metaCache: metaCache, contentLocks: newContentLock(), } @@ -154,13 +159,15 @@ type LookupResult struct { CacheStatus CacheStatus } -// Lookup checks if a processed variant exists on disk. Hits touch the -// variant's LRU timestamp; a disabled cache always misses. +// Lookup checks if a processed variant exists on disk: a variant held +// in metaCache counts as present without a check of the disk. Hits +// touch the variant's LRU timestamp; a disabled cache always misses. func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) { cacheKey := CacheKey(req) - // Check variant storage directly - no DB needed for cache hits - if !c.disabled && c.variants.Exists(cacheKey) { + // Check memory, then variant storage - no DB needed for cache hits + if !c.disabled && + (c.metaCache.Contains(cacheKey) || c.variants.Exists(cacheKey)) { c.touchVariant(ctx, cacheKey) return &LookupResult{ @@ -177,13 +184,35 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e }, nil } -// GetVariant returns a reader, size, and content type for a cached variant. +// GetVariant returns a reader, size, and content type for a cached +// variant. The content type comes from metaCache, or else from the +// variant's .meta file and is then kept in metaCache. func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) { if c.disabled { return nil, 0, "", ErrNotFound } - return c.variants.LoadWithMeta(cacheKey) + contentType, known := c.metaCache.Get(cacheKey) + if known { + reader, size, err := c.variants.LoadWithSize(cacheKey) + if err != nil { + // The file is gone, e.g. deleted outside pixa + c.metaCache.Remove(cacheKey) + + return nil, 0, "", err + } + + return reader, size, contentType, nil + } + + reader, size, contentType, err := c.variants.LoadWithMeta(cacheKey) + if err != nil { + return nil, 0, "", err + } + + c.metaCache.Add(cacheKey, contentType) + + return reader, size, contentType, nil } // StoreSource stores fetched source content and metadata. On a @@ -286,6 +315,8 @@ func (c *Cache) StoreVariant( return err } + c.metaCache.Add(cacheKey, contentType) + _, err = c.db.ExecContext(ctx, ` INSERT INTO variant_content (cache_key, size_bytes, content_type) VALUES (?, ?, ?) diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go index 8bf6edb..9366334 100644 --- a/internal/imgcache/eviction.go +++ b/internal/imgcache/eviction.go @@ -271,7 +271,9 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro // evictVariant removes one variant: accounting row first, then the // content and .meta files, so the database never references a deleted -// file. +// file. The metaCache entry goes before the files; a GetVariant that +// read them just before may put it back, and the next GetVariant then +// fails to open the file and removes it again. func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error { _, err := c.db.ExecContext(ctx, `DELETE FROM variant_content WHERE cache_key = ?`, string(cacheKey)) @@ -279,6 +281,8 @@ func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error { return fmt.Errorf("failed to delete variant accounting row: %w", err) } + c.metaCache.Remove(cacheKey) + err = c.variants.DeleteWithMeta(cacheKey) if err != nil { return err diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go index b9487ce..2ecabca 100644 --- a/internal/imgcache/storage.go +++ b/internal/imgcache/storage.go @@ -506,32 +506,43 @@ func (s *VariantStorage) Load(key VariantKey) (io.ReadCloser, error) { return f, nil } -// LoadWithMeta returns a reader, size, and content type for the content at -// the given key. -func (s *VariantStorage) LoadWithMeta( - key VariantKey, -) (io.ReadCloser, int64, string, error) { +// LoadWithSize returns a reader and file size for the content at the +// given key. +func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, error) { path := s.keyToPath(key) - metaPath := path + ".meta" f, err := os.Open(path) //nolint:gosec // path derived from cache key if err != nil { if os.IsNotExist(err) { - return nil, 0, "", ErrNotFound + return nil, 0, ErrNotFound } - return nil, 0, "", fmt.Errorf("failed to open content: %w", err) + return nil, 0, fmt.Errorf("failed to open content: %w", err) } stat, err := f.Stat() if err != nil { _ = f.Close() - return nil, 0, "", fmt.Errorf("failed to stat content: %w", err) + return nil, 0, fmt.Errorf("failed to stat content: %w", err) + } + + return f, stat.Size(), nil +} + +// LoadWithMeta returns a reader, size, and content type for the content at +// the given key. +func (s *VariantStorage) LoadWithMeta( + key VariantKey, +) (io.ReadCloser, int64, string, error) { + f, size, err := s.LoadWithSize(key) + if err != nil { + return nil, 0, "", err } // Load metadata for content type contentType := "application/octet-stream" // fallback + metaPath := s.keyToPath(key) + ".meta" metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key if err == nil { @@ -541,7 +552,7 @@ func (s *VariantStorage) LoadWithMeta( } } - return f, stat.Size(), contentType, nil + return f, size, contentType, nil } // Exists checks if content exists at the given key. -- 2.54.0 From 220fb3ee4e74c8fd8f5b7bbb0bd5dc375483adb7 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 07:22:21 +0000 Subject: [PATCH 3/8] Test that a read during a store keeps the stored content type (closes #70) A GetVariant that begins before StoreVariant finishes can find the variant file but not yet its .meta file, and so reads application/octet-stream. When it adds that to memory after the store added the real type, the wrong type is served to every later hit. The part of GetVariant that runs after its check of memory moves, unchanged, into loadVariantWithMeta, so the test can run it after a store. The test fails now. Model: opus-5-5 --- internal/imgcache/cache.go | 35 ++++++++++++-------- internal/imgcache/metacache_internal_test.go | 31 +++++++++++++++++ 2 files changed, 53 insertions(+), 13 deletions(-) diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index 7a3726c..ad06eb2 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -193,25 +193,18 @@ func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, e } contentType, known := c.metaCache.Get(cacheKey) - if known { - reader, size, err := c.variants.LoadWithSize(cacheKey) - if err != nil { - // The file is gone, e.g. deleted outside pixa - c.metaCache.Remove(cacheKey) - - return nil, 0, "", err - } - - return reader, size, contentType, nil + if !known { + return c.loadVariantWithMeta(cacheKey) } - reader, size, contentType, err := c.variants.LoadWithMeta(cacheKey) + reader, size, err := c.variants.LoadWithSize(cacheKey) if err != nil { + // The file is gone, e.g. deleted outside pixa + c.metaCache.Remove(cacheKey) + return nil, 0, "", err } - c.metaCache.Add(cacheKey, contentType) - return reader, size, contentType, nil } @@ -530,6 +523,22 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) { } } +// loadVariantWithMeta is GetVariant for a variant metaCache does not +// hold: it reads the content type from the variant's .meta file and +// keeps it in metaCache. +func (c *Cache) loadVariantWithMeta( + cacheKey VariantKey, +) (io.ReadCloser, int64, string, error) { + reader, size, contentType, err := c.variants.LoadWithMeta(cacheKey) + if err != nil { + return nil, 0, "", err + } + + c.metaCache.Add(cacheKey, contentType) + + return reader, size, contentType, nil +} + // writeMetadataSidecar writes the JSON metadata sidecar of a stored source. // A failure is logged and is otherwise non-fatal; the metadata is in the // database. diff --git a/internal/imgcache/metacache_internal_test.go b/internal/imgcache/metacache_internal_test.go index 0a2d368..c327249 100644 --- a/internal/imgcache/metacache_internal_test.go +++ b/internal/imgcache/metacache_internal_test.go @@ -114,6 +114,37 @@ func TestSecondHitDoesNotReadMetaFile(t *testing.T) { assertVariantServed(t, restarted, testVariantKeyOne, content) } +// TestReadDuringStoreKeepsStoredContentType checks that a GetVariant +// which began before StoreVariant finished cannot replace the content +// type the store kept in memory. Such a read can find the variant file +// but not yet its .meta file, and so gets application/octet-stream. The +// test deletes the .meta file after the store, then runs the part of +// GetVariant that comes after its check of memory. +func TestReadDuringStoreKeepsStoredContentType(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<20) + content := []byte("webp variant bytes") + + storeEvictionTestVariant(t, cache, testVariantKeyOne, content) + + err := os.Remove(cache.variants.keyToPath(testVariantKeyOne) + ".meta") + if err != nil { + t.Fatalf("removing .meta file: %v", err) + } + + reader, _, contentType, err := cache.loadVariantWithMeta(testVariantKeyOne) + if err != nil { + t.Fatalf("loadVariantWithMeta(%s) error = %v", testVariantKeyOne, err) + } + + _ = reader.Close() + + t.Logf("the read without a .meta file got content type %q", contentType) + + assertVariantServed(t, cache, testVariantKeyOne, content) +} + // TestEvictedVariantIsNotServed checks that a variant the evictor // removed is a miss and cannot be read, although it had been stored // and served before. -- 2.54.0 From 77e9c9cd7f432f7428134fd4ed2b232d79d0e263 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 07:29:52 +0000 Subject: [PATCH 4/8] Keep the content type a store put in memory over one read from disk (closes #70) GetVariant now adds the content type it read from a .meta file only when memory holds none for the variant, so a type StoreVariant added meanwhile is not replaced. A read that found no .meta file yet still gets application/octet-stream for that one request, as before this change, but no longer leaves it in memory for later hits. Model: opus-5-5 --- TODO.md | 3 ++- internal/imgcache/cache.go | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/TODO.md b/TODO.md index 1173c47..5f22565 100644 --- a/TODO.md +++ b/TODO.md @@ -32,7 +32,8 @@ P2: security: referer blacklist - 2026-09-29 variant content types kept in memory (closes #70): `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by - `GetVariant` after it reads a `.meta` file; for a variant it holds, `Lookup` + `GetVariant` after it reads a `.meta` file, where a type `StoreVariant` added + meanwhile is kept over the one read; for a variant it holds, `Lookup` skips the check of the disk and `GetVariant` skips the `.meta` read, still opening the variant file and taking the size from it; eviction removes the entry before deleting the files, and `GetVariant` removes it when the file diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index ad06eb2..43933db 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -525,7 +525,10 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) { // loadVariantWithMeta is GetVariant for a variant metaCache does not // hold: it reads the content type from the variant's .meta file and -// keeps it in metaCache. +// keeps it in metaCache, unless a StoreVariant has put one there +// meanwhile. The stored one wins, since this read may have found the +// variant file before the store wrote the .meta file, and so got +// application/octet-stream. func (c *Cache) loadVariantWithMeta( cacheKey VariantKey, ) (io.ReadCloser, int64, string, error) { @@ -534,7 +537,7 @@ func (c *Cache) loadVariantWithMeta( return nil, 0, "", err } - c.metaCache.Add(cacheKey, contentType) + c.metaCache.ContainsOrAdd(cacheKey, contentType) return reader, size, contentType, nil } -- 2.54.0 From c7e4eacd87c701242ae35741bcd20d2c253d059c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 07:30:24 +0000 Subject: [PATCH 5/8] Have Lookup check the disk for variants held in memory too (closes #70) Lookup again counts a variant as present only when its file exists. Treating a variant held in memory as present saved one check of the disk, but no test covered it, and GetVariant opens the file anyway. Model: opus-5-5 --- TODO.md | 12 ++++++------ internal/imgcache/cache.go | 10 ++++------ 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/TODO.md b/TODO.md index 5f22565..4cbc167 100644 --- a/TODO.md +++ b/TODO.md @@ -33,12 +33,12 @@ P2: security: referer blacklist `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by `GetVariant` after it reads a `.meta` file, where a type `StoreVariant` added - meanwhile is kept over the one read; for a variant it holds, `Lookup` - skips the check of the disk and `GetVariant` skips the `.meta` read, still - opening the variant file and taking the size from it; eviction removes the - entry before deleting the files, and `GetVariant` removes it when the file - will not open; the cap is a constant, not a setting; the unused `variantMeta` - type is gone; `README.md` describes it. + meanwhile is kept over the one read; for a variant it holds, `GetVariant` + skips the `.meta` read, still opening the variant file and taking the size + from it; eviction removes the entry before deleting the files, and + `GetVariant` removes it when the file will not open; the cap is a constant, + not a setting; the unused `variantMeta` type is gone; `README.md` describes + it. - 2026-09-29 maintenance mode refuses image requests (closes #71): while `maintenance_mode` is on, `/v1/image/` and `/v1/e/` answer 503 with a `Retry-After` header and the JSON error body, from one middleware in diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index 43933db..2a87900 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -159,15 +159,13 @@ type LookupResult struct { CacheStatus CacheStatus } -// Lookup checks if a processed variant exists on disk: a variant held -// in metaCache counts as present without a check of the disk. Hits -// touch the variant's LRU timestamp; a disabled cache always misses. +// Lookup checks if a processed variant exists on disk. Hits touch the +// variant's LRU timestamp; a disabled cache always misses. func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, error) { cacheKey := CacheKey(req) - // Check memory, then variant storage - no DB needed for cache hits - if !c.disabled && - (c.metaCache.Contains(cacheKey) || c.variants.Exists(cacheKey)) { + // Check variant storage directly - no DB needed for cache hits + if !c.disabled && c.variants.Exists(cacheKey) { c.touchVariant(ctx, cacheKey) return &LookupResult{ -- 2.54.0 From 8be21c005f2a1ef86eecf560bc49f735af5cafdd Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 08:26:48 +0000 Subject: [PATCH 6/8] Test that a failed read during a store keeps the stored content type (closes #70) A read that finds a variant in memory but cannot open its file removes it from memory. When that happens after StoreVariant has added the variant's content type, a read that opened the variant file before the store wrote its .meta file finds memory empty and keeps application/octet-stream there, so every later hit serves the image with it. The test runs the store, the failed read and the read without a .meta file in that order, then checks a later hit. It fails now. Model: opus-5-5 --- internal/imgcache/metacache_internal_test.go | 47 ++++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/internal/imgcache/metacache_internal_test.go b/internal/imgcache/metacache_internal_test.go index c327249..14be0f4 100644 --- a/internal/imgcache/metacache_internal_test.go +++ b/internal/imgcache/metacache_internal_test.go @@ -83,6 +83,16 @@ func assertLookupMisses(t *testing.T, cache *Cache, request *ImageRequest) { } } +// renameFile renames the file at from to to. +func renameFile(t *testing.T, from, to string) { + t.Helper() + + err := os.Rename(from, to) + if err != nil { + t.Fatalf("renaming %s: %v", from, err) + } +} + // TestSecondHitDoesNotReadMetaFile checks that once a variant has been // stored or read, a hit takes its content type from memory: with the // .meta file deleted, GetVariant must still return the stored content @@ -145,6 +155,43 @@ func TestReadDuringStoreKeepsStoredContentType(t *testing.T) { assertVariantServed(t, cache, testVariantKeyOne, content) } +// TestFailedReadDuringStoreKeepsStoredContentType checks that a read +// which found no .meta file cannot leave application/octet-stream in +// memory, even when another read has removed the content type +// StoreVariant kept there. In this order: the store; a read that finds +// the variant in memory but cannot open its file, and so removes it +// from memory; a read that opened the variant file before the store +// wrote its .meta file. Later hits must get the stored content type. +func TestFailedReadDuringStoreKeepsStoredContentType(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<20) + content := []byte("webp variant bytes") + variantPath := cache.variants.keyToPath(testVariantKeyOne) + metaPath := variantPath + ".meta" + + storeEvictionTestVariant(t, cache, testVariantKeyOne, content) + + renameFile(t, variantPath, variantPath+".hidden") + assertVariantNotFound(t, cache, testVariantKeyOne) + renameFile(t, variantPath+".hidden", variantPath) + + renameFile(t, metaPath, metaPath+".hidden") + + reader, _, contentType, err := cache.GetVariant(testVariantKeyOne) + if err != nil { + t.Fatalf("GetVariant(%s) error = %v", testVariantKeyOne, err) + } + + _ = reader.Close() + + t.Logf("the read without a .meta file got content type %q", contentType) + + renameFile(t, metaPath+".hidden", metaPath) + + assertVariantServed(t, cache, testVariantKeyOne, content) +} + // TestEvictedVariantIsNotServed checks that a variant the evictor // removed is a miss and cannot be read, although it had been stored // and served before. -- 2.54.0 From 314fbd186d02c32e3db81c3db9f43354448667c4 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 08:28:26 +0000 Subject: [PATCH 7/8] Keep application/octet-stream out of memory for variants (closes #70) VariantStorage.LoadWithMeta now returns an empty content type when the .meta file is missing or unreadable, and GetVariant serves application/octet-stream in that case without keeping it in memory. Memory now only ever holds a content type read from a .meta file or passed to StoreVariant, so a read that ran before a store wrote the .meta file can no longer leave the wrong type in memory, however it interleaves with the store, other reads and eviction. Model: opus-5-5 --- TODO.md | 13 +++++++------ internal/imgcache/cache.go | 16 ++++++++++++---- internal/imgcache/eviction.go | 4 ++-- internal/imgcache/storage.go | 7 ++++--- 4 files changed, 25 insertions(+), 15 deletions(-) diff --git a/TODO.md b/TODO.md index 4cbc167..bb548f5 100644 --- a/TODO.md +++ b/TODO.md @@ -33,12 +33,13 @@ P2: security: referer blacklist `Cache.metaCache` holds the content types of up to 10,000 variants in an LRU (`github.com/hashicorp/golang-lru/v2`), filled by `StoreVariant` and by `GetVariant` after it reads a `.meta` file, where a type `StoreVariant` added - meanwhile is kept over the one read; for a variant it holds, `GetVariant` - skips the `.meta` read, still opening the variant file and taking the size - from it; eviction removes the entry before deleting the files, and - `GetVariant` removes it when the file will not open; the cap is a constant, - not a setting; the unused `variantMeta` type is gone; `README.md` describes - it. + meanwhile is kept over the one read, and never with the + `application/octet-stream` served for a variant without one; for a variant it + holds, `GetVariant` skips the `.meta` read, still opening the variant file and + taking the size from it; eviction removes the entry before deleting the files, + and `GetVariant` removes it when the file will not open; the cap is a + constant, not a setting; the unused `variantMeta` type is gone; `README.md` + describes it. - 2026-09-29 maintenance mode refuses image requests (closes #71): while `maintenance_mode` is on, `/v1/image/` and `/v1/e/` answer 503 with a `Retry-After` header and the JSON error body, from one middleware in diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index 2a87900..a4faab4 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -184,7 +184,9 @@ func (c *Cache) Lookup(ctx context.Context, req *ImageRequest) (*LookupResult, e // GetVariant returns a reader, size, and content type for a cached // variant. The content type comes from metaCache, or else from the -// variant's .meta file and is then kept in metaCache. +// variant's .meta file and is then kept in metaCache. A variant with +// no .meta file is served as application/octet-stream, which is not +// kept. func (c *Cache) GetVariant(cacheKey VariantKey) (io.ReadCloser, int64, string, error) { if c.disabled { return nil, 0, "", ErrNotFound @@ -524,9 +526,11 @@ func (c *Cache) IncrementTransformCount(ctx context.Context) { // loadVariantWithMeta is GetVariant for a variant metaCache does not // hold: it reads the content type from the variant's .meta file and // keeps it in metaCache, unless a StoreVariant has put one there -// meanwhile. The stored one wins, since this read may have found the -// variant file before the store wrote the .meta file, and so got -// application/octet-stream. +// meanwhile, as the store's is newer. A read that finds no .meta file, +// as one can between a store's writing of the variant file and of its +// .meta file, serves application/octet-stream and keeps nothing, so +// metaCache only ever holds a type read from a .meta file or passed to +// StoreVariant. func (c *Cache) loadVariantWithMeta( cacheKey VariantKey, ) (io.ReadCloser, int64, string, error) { @@ -535,6 +539,10 @@ func (c *Cache) loadVariantWithMeta( return nil, 0, "", err } + if contentType == "" { + return reader, size, fallbackContentType, nil + } + c.metaCache.ContainsOrAdd(cacheKey, contentType) return reader, size, contentType, nil diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go index 9366334..5d185ea 100644 --- a/internal/imgcache/eviction.go +++ b/internal/imgcache/eviction.go @@ -39,8 +39,8 @@ const tempFilePrefix = ".tmp-" // to each variant file. const variantMetaSuffix = ".meta" -// fallbackContentType is recorded when a reconciled variant file has -// no readable .meta sidecar. +// fallbackContentType is the content type given to a variant file that +// has no readable .meta sidecar, when it is served or reconciled. const fallbackContentType = "application/octet-stream" // UsageBytes returns the total number of bytes of cache content diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go index 2ecabca..7e1cb38 100644 --- a/internal/imgcache/storage.go +++ b/internal/imgcache/storage.go @@ -531,7 +531,8 @@ func (s *VariantStorage) LoadWithSize(key VariantKey) (io.ReadCloser, int64, err } // LoadWithMeta returns a reader, size, and content type for the content at -// the given key. +// the given key. The content type is read from the .meta file, and is +// empty when that file is missing or unreadable. func (s *VariantStorage) LoadWithMeta( key VariantKey, ) (io.ReadCloser, int64, string, error) { @@ -540,8 +541,8 @@ func (s *VariantStorage) LoadWithMeta( return nil, 0, "", err } - // Load metadata for content type - contentType := "application/octet-stream" // fallback + var contentType string + metaPath := s.keyToPath(key) + ".meta" metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key -- 2.54.0 From c6e66bf45aa619b56275118ea904b84fe49be041 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 29 Sep 2026 09:31:47 +0000 Subject: [PATCH 8/8] Test that a read of an older .meta file keeps the stored content type (closes #70) A read that finds no .meta file keeps nothing in memory, so the existing test no longer reaches the step that keeps a type StoreVariant put there. This one writes a .meta file with a different content type after the store, as one not yet rewritten by it would hold, and checks that the read leaves the stored type in memory. Model: opus-5-5 --- internal/imgcache/metacache_internal_test.go | 49 ++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/internal/imgcache/metacache_internal_test.go b/internal/imgcache/metacache_internal_test.go index 14be0f4..bade726 100644 --- a/internal/imgcache/metacache_internal_test.go +++ b/internal/imgcache/metacache_internal_test.go @@ -2,6 +2,7 @@ package imgcache import ( "bytes" + "encoding/json" "errors" "fmt" "io" @@ -155,6 +156,54 @@ func TestReadDuringStoreKeepsStoredContentType(t *testing.T) { assertVariantServed(t, cache, testVariantKeyOne, content) } +// TestReadOfOlderMetaFileKeepsStoredContentType checks that a read +// which got its content type from a .meta file that StoreVariant had +// not yet rewritten cannot replace the type the store kept in memory. +// The test writes such a .meta file, with a different content type, +// after the store, then runs the part of GetVariant that comes after +// its check of memory. +func TestReadOfOlderMetaFileKeepsStoredContentType(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<20) + content := []byte("webp variant bytes") + + storeEvictionTestVariant(t, cache, testVariantKeyOne, content) + + olderMeta, err := json.Marshal(VariantMeta{ + ContentType: testContentTypeJPEG, + Size: int64(len(content)), + }) + if err != nil { + t.Fatalf("encoding .meta file: %v", err) + } + + metaPath := cache.variants.keyToPath(testVariantKeyOne) + ".meta" + + err = os.WriteFile(metaPath, olderMeta, StorageFilePerm) + if err != nil { + t.Fatalf("writing .meta file: %v", err) + } + + reader, _, contentType, err := cache.loadVariantWithMeta(testVariantKeyOne) + if err != nil { + t.Fatalf("loadVariantWithMeta(%s) error = %v", testVariantKeyOne, err) + } + + _ = reader.Close() + + if contentType != testContentTypeJPEG { + t.Fatalf("loadVariantWithMeta(%s) content type = %q, want %q from the .meta file", + testVariantKeyOne, contentType, testContentTypeJPEG) + } + + kept, _ := cache.metaCache.Get(testVariantKeyOne) + if kept != testContentTypeWebP { + t.Errorf("content type in memory = %q, want the stored %q", + kept, testContentTypeWebP) + } +} + // TestFailedReadDuringStoreKeepsStoredContentType checks that a read // which found no .meta file cannot leave application/octet-stream in // memory, even when another read has removed the content type -- 2.54.0