Serve a cache hit with no database read or write (closes #309)

A hit opens the variant file and nothing else: its content type comes
from the file's first bytes, and a file in no output format is a miss.
Its access time and count, and the access time of a cached source a
miss uses, go without waiting to one background goroutine that writes
them once a second; each kind of access time waits in a channel of
10,000 that drops the rest. Expired negative cache rows are left to
CleanExpired. The .meta files, metaCache, Lookup and golang-lru go,
with their tests; the two reconciliation walks, identical without the
.meta skip, share one. make bench runs a parallel cache hit benchmark
on the host, outside make check.

Model: opus-5-5

Makefile

@@ -1,4 +1,4 @@
-.PHONY: bootstrap setup check lint test fmt fmt-check build clean docker docker-smoke docker-versioned docker-test devserver devserver-stop hooks loadtest
+.PHONY: bootstrap setup check lint test fmt fmt-check build clean docker docker-smoke docker-versioned docker-test devserver devserver-stop hooks loadtest bench
 
 VERSION := $(shell git describe --tags --always --dirty 2>/dev/null || echo "dev")
 LDFLAGS := -X main.Version=$(VERSION)
@@ -64,6 +64,12 @@ docker-smoke:
 loadtest:
 	@script/loadtest
 
+# Serve cache hits in parallel through the image handler and report hits per
+# second (needs Go on the host after `script/bootstrap --cgo`, not Docker; a
+# benchmark, not part of check)
+bench:
+	@script/bench
+
 # Build Docker image as `make docker` does, and also tag it pixa:$(VERSION)
 docker-versioned:
 	@script/docker

README.md

@@ -76,8 +76,7 @@ database and the disk cache:
   pixa runs as (uid 65532) and sets its mode to `750`; outside it, that user
   must be able to write the directory.
 - `cache_max_bytes` limits the source and transformed images together. The
-  database, the metadata files, the `.meta` file beside each transformed image
+  database and files still being written come on top, and eviction runs in the
-  and files still being written come on top, and eviction runs in the
   background, so the cache can pass the limit for a while: leave room on the
   volume beyond it.
 - Set `cache_max_bytes` for a lasting deployment. Its default, worked out each
@@ -91,11 +90,12 @@ which answers 200 whenever pixa is running, in maintenance mode too (see
 
 On SIGTERM or SIGINT pixa stops accepting connections, gives the requests in
 progress and the images being processed 5 seconds to finish, then writes to the
-database the counts of cache hits, misses, fetches and conversions that requests
+database the counts of cache hits, misses, fetches and conversions and the times
-could not write by their deadline, and exits: with 0, or with 1 when images were
+cached images were last used that are not written yet (see Storage), and exits:
-still being processed after those 5 seconds, some of those counts could not be
+with 0, or with 1 when images were still being processed after those 5 seconds,
-written, or another part of pixa failed to stop. A request not finished by then
+some of those counts or times could not be written, or another part of pixa
-is cut off. `docker stop` waits 10 seconds before it kills the container.
+failed to stop. A request not finished by then is cut off. `docker stop` waits
+10 seconds before it kills the container.
 
 Outside Docker, pixa needs libvips (the image has 8.16) and libheif to run, as
 it uses libvips through CGO. pixad does not start unless libvips has its JPEG XL
@@ -177,38 +177,46 @@ allowlisted source hosts for open access.
 
 ### Storage
 
-- **Source content**:
+- **Source images**:
-  `<state_dir>/cache/sources/<ab>/<cd>/<sha256 of source content>`
+  `<state_dir>/cache/sources/<ab>/<cd>/<sha256 of host, path and query>`, one
-- **Source metadata**:
+  file for each source URL
-  `<state_dir>/cache/metadata/<hostname>/<sha256 of path and query>.json` (host,
-  path and query, content hash, upstream status and headers, fetch time)
 - **Database**: `<state_dir>/state.sqlite3` (SQLite)
 - **Transformed images**:
-  `<state_dir>/cache/variants/<ab>/<cd>/<sha256 of host, path, query, size, format, quality and fit>`,
+  `<state_dir>/cache/variants/<ab>/<cd>/<sha256 of host, path, query, size, format, quality and fit>`
-  each with a `.meta` file beside it holding its content type
 
 `<ab>` and `<cd>` are the first and second pairs of characters of the file's
 name.
 
-Multiple source paths may reference the same content blob; the database tracks
+Each source image and each transformed image has one row in the database,
-references rather than using filesystem refcounting.
+holding its size and when it was last used. Two source URLs that serve the same
+bytes are stored twice, once under each URL. A source image is stored by writing
+its file and then its row; when the row cannot be written, the file is removed
+again.
 
-Source and transformed image files, and the `.meta` file beside each transformed
+Source and transformed image files are written under a temporary name, synced to
-image, are written under a temporary name, synced to disk, and only then renamed
+disk, and only then renamed into place, so a crash leaves no file under its own
-into place, so a crash leaves no file under its own name with part of its data.
+name with part of its data. When pixa starts, and every five minutes after, it
-When pixa starts, and every five minutes after, it removes each cached source or
+removes each cached source or transformed image whose file is empty, or not the
-transformed image whose file is empty, or not the size the database records for
+size the database records for it, with a warning naming the file; the next
-it, with a warning naming the file; the next request for it fetches the source
+request for it fetches the source or converts the image again. Looking for those
-or converts the image again. Looking for those files reads the database once for
+files reads the database once for every cached file, so pixa skips each
-every cached file, so pixa skips each five-minute look that would start less
+five-minute look that would start less than ten times as long after the last
-than ten times as long after the last look ended as that look took. After a look
+look ended as that look took. After a look that took one minute, the next one
-that took one minute, the next one starts 10 to 15 minutes after that look
+starts 10 to 15 minutes after that look ended.
-ended.
 
 pixa's target is 1-5k r/s, which has not been measured at that rate (see Load
-Test). Toward it, pixa keeps in memory the content types of the 10,000
+Test). Toward it, a cache hit opens the transformed image's file and nothing
-transformed images most recently cached or served, so a cache hit on one of them
+else, and makes no database call. The content type comes from the file's first
-reads only the image file from disk and not the metadata file stored beside it.
+bytes, which are those of the format pixa wrote it in; a file whose first bytes
+are not those of a format pixa writes is not served, and the image is converted
+again. The time the image was last served, which eviction orders by, and the hit
+count are handed to a background writer without waiting for it, as are the
+counts of misses, fetches and conversions, and the time a miss last used a
+cached source image. Once a second that writer writes the waiting times in one
+transaction and the counts in one statement. While 10,000 times of transformed
+images, or 10,000 of source images, wait to be written, a further one is
+dropped, and that image only looks less recently used to eviction.
+`script/bench` measures how many hits pixa serves per second (see Load Test).
 
 ### Routes
 
@@ -846,6 +854,9 @@ provide:
 - `script/docker-smoke` — build the image, start it, wait for it to be healthy
 - `script/loadtest` — measure pixad's throughput, latency and peak memory; a
   benchmark, not part of `script/check` (see Load Test)
+- `script/bench` — run the Go benchmark that serves cache hits in parallel
+  through the image handler, with Go on the host after `script/bootstrap --cgo`;
+  a benchmark, not part of `script/check` (see Load Test)
 - `script/cibuild` — CI entrypoint: run `script/bootstrap` (without `--cgo`),
   then `script/check`, then build the image as `script/docker` does
 - `script/precommit` — pre-commit checks (`go mod tidy` guard, then
@@ -926,6 +937,17 @@ The numbers depend on the machine and on whatever else runs on it. The first
 measurement, made on a shared machine with few clients, is in `TODO.md`; it says
 nothing about the target.
 
+`script/bench` (or `make bench`) measures cache hits alone, without the network
+and with no pixad process: a Go benchmark in `internal/handlers` serves one
+cached image through the `/v1/image/` handler from as many goroutines at once as
+the machine has CPUs, and reports the hits served per second as `hits/s`. Each
+run starts with an empty cache and an empty database, a SQLite file of its own,
+and puts the image in the cache before it measures. The benchmark fails if
+serving the hits ran any statement on the database. Like the load test it is not
+part of `script/check`. It runs Go on the host, as the load test does, and
+compiles pixa there, so it needs the C compiler and libraries that
+`script/bootstrap --cgo` installs, but not Docker.
+
 ## TODO
 
 See [TODO.md](TODO.md) for the full prioritized task list.

TODO.md

@@ -30,6 +30,31 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
+- 2026-10-21 a cache hit makes no database call (closes #309): `Service.Get`
+  opens the variant file with `Cache.GetVariant` and nothing else, with no
+  existence check first. The content type comes from the file's first bytes; a
+  file whose first bytes are not those of an output format, SVG included, is a
+  miss, and the variant is made again. A hit hands its access time to a channel
+  that holds up to 10,000 and drops the rest, and its hit count to the pending
+  counts, without waiting; every count is now a pending count. A miss that finds
+  a cached source hands that source's access time to a second such channel. The
+  goroutine `StartPendingCountWrites` starts writes, once a second, the access
+  times in one transaction and the counts in one `UPDATE`;
+  `StopPendingCountWrites` writes what is left once at shutdown and logs
+  separately the access times and the counts it could not write.
+  `checkNegativeCache` no longer deletes an expired row; `CleanExpired` in the
+  eviction goroutine does. The `.meta` files, `VariantMeta`, `LoadWithMeta`,
+  `variantContentTypeFromSidecar`, `metaCache`, `Lookup` and `golang-lru` are
+  gone. `make bench` (`script/bench`) runs `BenchmarkHandleImage_CacheHits` with
+  Go on the host after `script/bootstrap --cgo`, outside `make check`.
+- 2026-10-21 a source is stored the way a variant is (closes #308): one
+  `source_content` row and one file per source URL, the file named by the
+  source's `SourceKey`, the SHA-256 of its host, path and query made as the
+  variant key is, under `cache/sources/`. A store writes the file, then the row,
+  and removes the file when the row cannot be written; an eviction deletes the
+  row, then the file, with no lock and no transaction. `source_metadata`, the
+  JSON files under `cache/metadata/`, `MetadataStorage` and the content lock are
+  gone. Two source URLs that serve the same bytes now store them twice.
 - 2026-10-21 one image request type instead of five (closes #307): the new
   package `internal/imagerequest`, which imports no other package of pixa, holds
   `imagerequest.Request` with its two parts, `Source` (host, path and query) and

go.mod

@@ -14,7 +14,6 @@ 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

go.sum

@@ -228,8 +228,6 @@ 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=

internal/database/concurrent_writes_internal_test.go

@@ -131,7 +131,8 @@ func writeLikeOneRequest(ctx context.Context, db *sql.DB, key string) error {
 	}
 
 	_, err = db.ExecContext(ctx, `INSERT INTO source_content
-		(content_hash, content_type, size_bytes) VALUES (?, 'image/png', 1)`, key)
+		(source_key, source_host, source_path, content_type, size_bytes)
+		VALUES (?, 'example.com', '/image.png', 'image/png', 1)`, key)
 	if err != nil {
 		return fmt.Errorf("storing a source: %w", err)
 	}

internal/database/database_internal_test.go

@@ -160,7 +160,7 @@ func TestApplyMigrations_CreatesSchemaAndTables(t *testing.T) {
 
 	// Verify that the application tables created by 001.sql exist.
 	tables := []string{
-		"source_content", "source_metadata", "output_content",
+		"source_content", "output_content",
 		"request_cache", "negative_cache", "cache_stats",
 	}
 	for _, table := range tables {

internal/db/migrations/001_schema.sql

@@ -1,10 +1,14 @@
 -- Migration 001: Initial schema
 -- Creates all tables for the pixa caching image proxy
 
--- Source content blobs
+-- Source images, one row per source URL
--- Files stored at: cache/sources/<ab>/<cd>/<sha256>
+-- Files stored at: cache/sources/<ab>/<cd>/<source_key>, where source_key
+-- is the SHA-256 of the source's host, path and query
 CREATE TABLE IF NOT EXISTS source_content (
-    content_hash TEXT PRIMARY KEY,
+    source_key TEXT PRIMARY KEY,
+    source_host TEXT NOT NULL,
+    source_path TEXT NOT NULL,
+    source_query TEXT NOT NULL DEFAULT '',
     content_type TEXT NOT NULL,
     size_bytes INTEGER NOT NULL,
     fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
@@ -13,35 +17,13 @@ CREATE TABLE IF NOT EXISTS source_content (
 CREATE INDEX IF NOT EXISTS idx_source_content_last_accessed
     ON source_content(last_accessed_at);
 
--- Source URL metadata - maps URLs to content hashes
--- JSON stored at: cache/metadata/<hostname>/<path_hash>.json
-CREATE TABLE IF NOT EXISTS source_metadata (
-    id INTEGER PRIMARY KEY AUTOINCREMENT,
-    source_host TEXT NOT NULL,
-    source_path TEXT NOT NULL,
-    source_query TEXT NOT NULL DEFAULT '',
-    path_hash TEXT NOT NULL,
-    content_hash TEXT,
-    status_code INTEGER NOT NULL,
-    content_type TEXT,
-    response_headers TEXT,
-    fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
-    expires_at DATETIME,
-    etag TEXT,
-    last_modified TEXT,
-    UNIQUE(source_host, source_path, source_query),
-    FOREIGN KEY (content_hash) REFERENCES source_content(content_hash)
-);
-CREATE INDEX IF NOT EXISTS idx_source_meta_host ON source_metadata(source_host);
-CREATE INDEX IF NOT EXISTS idx_source_meta_path_hash ON source_metadata(path_hash);
-CREATE INDEX IF NOT EXISTS idx_source_meta_expires ON source_metadata(expires_at);
-CREATE INDEX IF NOT EXISTS idx_source_meta_content_hash ON source_metadata(content_hash);
-
 -- Processed variant blobs
--- Files stored at: cache/variants/<ab>/<cd>/<cache_key> (plus a .meta
+-- Files stored at: cache/variants/<ab>/<cd>/<cache_key>. Tracked here
--- sidecar with the content type). Tracked here (like source content
+-- (like source content blobs above) so total cache usage is known
--- blobs above) so total cache usage is known without a directory scan,
+-- without a directory scan, and so LRU eviction has a timestamp to order
--- and so LRU eviction has a timestamp to order on.
+-- on. A cache hit does not read this table: it takes the content type
+-- from the file's first bytes, and its last_accessed_at is written
+-- later, in a batch.
 CREATE TABLE IF NOT EXISTS variant_content (
     cache_key TEXT PRIMARY KEY,
     size_bytes INTEGER NOT NULL,

internal/handlers/cache_hit_internal_test.go

@@ -0,0 +1,265 @@
+package handlers
+
+import (
+	"bytes"
+	"context"
+	"database/sql"
+	"database/sql/driver"
+	"image"
+	"image/jpeg"
+	"log/slog"
+	"net/http"
+	"net/http/httptest"
+	"path/filepath"
+	"sync/atomic"
+	"testing"
+	"testing/fstest"
+	"time"
+
+	"github.com/go-chi/chi/v5"
+
+	"sneak.berlin/go/pixa/internal/database"
+	"sneak.berlin/go/pixa/internal/encurl"
+	"sneak.berlin/go/pixa/internal/imagerequest"
+	"sneak.berlin/go/pixa/internal/imgcache"
+)
+
+// countingHost is the allowlisted host whose photoPath the server of
+// newCountingDatabaseServer serves.
+const countingHost = "counting.example.com"
+
+// countingConnector opens connections to the SQLite database file name with
+// sqliteDriver, each one counting in statements the statements run on it.
+type countingConnector struct {
+	sqliteDriver driver.Driver
+	name         string
+	statements   *atomic.Int64
+}
+
+func (c countingConnector) Connect(context.Context) (driver.Conn, error) {
+	conn, err := c.sqliteDriver.Open(c.name)
+	if err != nil {
+		return nil, err
+	}
+
+	return countingConn{Conn: conn, statements: c.statements}, nil
+}
+
+func (c countingConnector) Driver() driver.Driver {
+	return c.sqliteDriver
+}
+
+// countingConn counts the statements run on its connection. It has only the
+// methods of driver.Conn, so database/sql prepares every query and every
+// other statement with Prepare, which counts it.
+type countingConn struct {
+	driver.Conn
+
+	statements *atomic.Int64
+}
+
+func (c countingConn) Prepare(query string) (driver.Stmt, error) {
+	c.statements.Add(1)
+
+	return c.Conn.Prepare(query)
+}
+
+// newCountingDatabase returns a database with pixa's schema, in a file of
+// its own and with one connection, and the count of the statements run on
+// it once the schema is in place.
+func newCountingDatabase(tb testing.TB) (*sql.DB, *atomic.Int64) {
+	tb.Helper()
+
+	path := filepath.Join(tb.TempDir(), "state.sqlite3")
+
+	schemaDB, err := sql.Open("sqlite", path)
+	if err != nil {
+		tb.Fatalf("opening the database: %v", err)
+	}
+
+	defer func() { _ = schemaDB.Close() }()
+
+	err = database.ApplyMigrations(tb.Context(), schemaDB, nil)
+	if err != nil {
+		tb.Fatalf("applying the migrations: %v", err)
+	}
+
+	statements := new(atomic.Int64)
+
+	db := sql.OpenDB(countingConnector{
+		sqliteDriver: schemaDB.Driver(),
+		name:         path,
+		statements:   statements,
+	})
+	db.SetMaxOpenConns(1)
+
+	tb.Cleanup(func() { _ = db.Close() })
+
+	return db, statements
+}
+
+// newCountingDatabaseServer returns a router for both image routes whose
+// image cache uses newCountingDatabase, the count of the statements run on
+// that database, and a URL on each route for a different image of
+// photoPath on countingHost. Nothing writes the cache's pending counts and
+// access times in the background, so every statement counted comes from a
+// request.
+func newCountingDatabaseServer(
+	tb testing.TB,
+) (http.Handler, *atomic.Int64, []string) {
+	tb.Helper()
+
+	db, statements := newCountingDatabase(tb)
+
+	var photo bytes.Buffer
+
+	err := jpeg.Encode(&photo, image.NewRGBA(image.Rect(0, 0, 100, 100)), nil)
+	if err != nil {
+		tb.Fatalf("encoding the photo: %v", err)
+	}
+
+	cache, err := imgcache.NewCache(db, imgcache.CacheConfig{
+		StateDir:    tb.TempDir(),
+		NegativeTTL: 5 * time.Minute,
+	})
+	if err != nil {
+		tb.Fatalf("imgcache.NewCache() error = %v", err)
+	}
+
+	svc, err := imgcache.NewService(&imgcache.ServiceConfig{
+		Cache: cache,
+		Fetcher: newMockFetcher(fstest.MapFS{
+			countingHost + photoPath: &fstest.MapFile{Data: photo.Bytes()},
+		}),
+		SigningKey: testSigningKey,
+		Allowlist:  []string{countingHost},
+		Logger:     slog.New(slog.DiscardHandler),
+	})
+	if err != nil {
+		tb.Fatalf("imgcache.NewService() error = %v", err)
+	}
+
+	encGen, err := encurl.NewGenerator(testSigningKey)
+	if err != nil {
+		tb.Fatalf("encurl.NewGenerator() error = %v", err)
+	}
+
+	token, err := encGen.Generate(&imagerequest.Request{
+		Source: imagerequest.Source{Host: countingHost, Path: photoPath},
+		Transform: imagerequest.Transform{
+			Size:    imagerequest.Size{Width: 40, Height: 40},
+			Format:  imagerequest.FormatJPEG,
+			Quality: 85,
+			FitMode: imagerequest.FitCover,
+		},
+	})
+	if err != nil {
+		tb.Fatalf("encGen.Generate() error = %v", err)
+	}
+
+	h := &Handlers{
+		log:    slog.New(slog.DiscardHandler),
+		imgSvc: svc,
+		encGen: encGen,
+	}
+
+	r := chi.NewRouter()
+	r.Get("/v1/image/*", h.HandleImage())
+	r.Get("/v1/e/{token}/*", h.HandleImageEnc())
+
+	return r, statements, []string{
+		"/v1/image/" + countingHost + photoPath + "/50x50.jpeg",
+		"/v1/e/" + token + "/img.jpg",
+	}
+}
+
+// serveImage sends srv a GET for target and returns the response, failing
+// the test unless it is a 200 with an image.
+func serveImage(
+	tb testing.TB, srv http.Handler, target string,
+) *httptest.ResponseRecorder {
+	tb.Helper()
+
+	rec := httptest.NewRecorder()
+	srv.ServeHTTP(rec,
+		httptest.NewRequestWithContext(tb.Context(), http.MethodGet, target, nil))
+
+	if rec.Code != http.StatusOK || rec.Body.Len() == 0 {
+		tb.Errorf("GET %s: %d with %d bytes, want 200 with an image",
+			target, rec.Code, rec.Body.Len())
+	}
+
+	return rec
+}
+
+// TestImageRoutes_CacheHitsMakeNoDatabaseCall serves an image on each image
+// route once, which makes and caches it, then serves it again from the cache.
+// Serving those cache hits must run no statement on the database.
+func TestImageRoutes_CacheHitsMakeNoDatabaseCall(t *testing.T) {
+	t.Parallel()
+
+	srv, statements, targets := newCountingDatabaseServer(t)
+
+	for _, target := range targets {
+		before := statements.Load()
+
+		serveImage(t, srv, target)
+
+		afterMiss := statements.Load()
+		t.Logf("GET %s, a miss: %d statements", target, afterMiss-before)
+
+		if afterMiss == before {
+			t.Fatalf("GET %s, a miss: no statement counted, want some", target)
+		}
+
+		for range 3 {
+			rec := serveImage(t, srv, target)
+
+			cacheStatus := rec.Header().Get("X-Pixa-Cache")
+			if cacheStatus != string(imgcache.CacheHit) {
+				t.Errorf("GET %s: X-Pixa-Cache = %q, want %s",
+					target, cacheStatus, imgcache.CacheHit)
+			}
+		}
+
+		if hitStatements := statements.Load() - afterMiss; hitStatements != 0 {
+			t.Errorf("GET %s: 3 cache hits ran %d statements, want 0",
+				target, hitStatements)
+		}
+	}
+}
+
+// BenchmarkHandleImage_CacheHits serves one cached image through the
+// /v1/image/ route from b.RunParallel's goroutines, and reports the cache
+// hits served per second. It fails if serving them ran any statement on the
+// database. script/bench runs it.
+func BenchmarkHandleImage_CacheHits(b *testing.B) {
+	srv, statements, targets := newCountingDatabaseServer(b)
+	target := targets[0]
+
+	serveImage(b, srv, target)
+
+	before := statements.Load()
+
+	b.ResetTimer()
+
+	b.RunParallel(func(pb *testing.PB) {
+		for pb.Next() {
+			rec := serveImage(b, srv, target)
+
+			cacheStatus := rec.Header().Get("X-Pixa-Cache")
+			if cacheStatus != string(imgcache.CacheHit) {
+				b.Errorf("GET %s: X-Pixa-Cache = %q, want %s",
+					target, cacheStatus, imgcache.CacheHit)
+
+				return
+			}
+		}
+	})
+
+	b.ReportMetric(float64(b.N)/b.Elapsed().Seconds(), "hits/s")
+
+	if hitStatements := statements.Load() - before; hitStatements != 0 {
+		b.Fatalf("%d cache hits ran %d statements, want 0", b.N, hitStatements)
+	}
+}

internal/handlers/handlers.go

@@ -76,8 +76,8 @@ func New(lc fx.Lifecycle, params Params) (*Handlers, error) {
 		OnStart: func(_ context.Context) error {
 			return s.initImageService()
 		},
-		// The pending counts are written here, before the database's stop
+		// The pending counts and access times are written here, before the
-		// hook closes it.
+		// database's stop hook closes it.
 		OnStop: func(ctx context.Context) error {
 			if s.imgCache == nil {
 				return nil
@@ -128,7 +128,8 @@ func (s *Handlers) initImageService() error {
 	// rows, which is all it does when the disk cache is disabled.
 	cache.StartEviction(imgcache.DefaultEvictionInterval)
 
-	// Writes the counts requests could not write by their deadline
+	// Writes the counts and the access times of cache hits that requests
+	// hand over, so that no request waits for the database to record them
 	cache.StartPendingCountWrites()
 
 	// Create the fetcher config

internal/imagerequest/imagerequest.go

@@ -1,7 +1,7 @@
 // Package imagerequest holds the image request type, its parts, its checks,
-// the parsing of the /v1/image/ path and the variant key. It imports no other
+// the parsing of the /v1/image/ path, the variant key and the source key. It
-// package of this repo, so every package that handles an image request can
+// imports no other package of this repo, so every package that handles an
-// import it.
+// image request can import it.
 package imagerequest
 
 import (

internal/imagerequest/sourcekey.go

@@ -0,0 +1,26 @@
+package imagerequest
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"fmt"
+)
+
+// SourceKey is a SHA256 hash identifying a cached source image (hex-encoded).
+type SourceKey string
+
+// Key returns the key the source is cached under, made as CacheKey makes a
+// variant key.
+// Format: sha256(host:path:query)
+// The path and query go through escapePercentAndColon, so that a ":" in
+// either cannot be taken for the one between them.
+func (s Source) Key() SourceKey {
+	data := fmt.Sprintf("%s:%s:%s",
+		s.Host,
+		escapePercentAndColon(s.Path),
+		escapePercentAndColon(s.Query),
+	)
+	h := sha256.Sum256([]byte(data))
+
+	return SourceKey(hex.EncodeToString(h[:]))
+}

internal/imagerequest/sourcekey_internal_test.go

@@ -0,0 +1,40 @@
+package imagerequest
+
+import "testing"
+
+// TestSourceKey_DiffersForEachSource checks that each source gets a key of
+// 64 hex characters, the same each time, and that no two of these sources
+// share one, also when they differ only by a ":" or "%" in the path or query.
+func TestSourceKey_DiffersForEachSource(t *testing.T) {
+	t.Parallel()
+
+	sources := []Source{
+		{Host: testHostCDN, Path: testPathCat},
+		{Host: testHostCDN, Path: testPathCat, Query: "v=2"},
+		{Host: "other.example.com", Path: testPathCat},
+		{Host: testHostCDN, Path: "/c:d"},
+		{Host: testHostCDN, Path: "/c", Query: "d:"},
+		{Host: testHostCDN, Path: "/c%3Ad"},
+	}
+
+	seen := make(map[SourceKey]Source)
+
+	for _, source := range sources {
+		key := source.Key()
+		t.Logf("Key() of %+v = %s", source, key)
+
+		if len(key) != 64 {
+			t.Errorf("Key() of %+v length = %d, want 64", source, len(key))
+		}
+
+		if again := source.Key(); again != key {
+			t.Errorf("Key() of %+v not deterministic: %s vs %s", source, key, again)
+		}
+
+		if earlier, found := seen[key]; found {
+			t.Errorf("Key() gave %+v and %+v the same key", earlier, source)
+		}
+
+		seen[key] = source
+	}
+}

internal/imgcache/cache.go

@@ -2,10 +2,7 @@ package imgcache
 
 import (
 	"context"
-	"crypto/sha256"
 	"database/sql"
-	"encoding/hex"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
@@ -15,7 +12,6 @@ import (
 	"sync/atomic"
 	"time"
 
-	lru "github.com/hashicorp/golang-lru/v2"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imagerequest"
 )
@@ -26,12 +22,11 @@ var (
 	ErrNegativeCache = errors.New("negative cache hit")
 )
 
-// HTTP status code for successful fetch.
+// maxPendingAccessTimes is how many access times of variants served from
-const httpStatusOK = 200
+// the cache, and how many of cached sources a miss used, may each wait to be
-
+// written, at about 80 bytes each. An access time handed over while that
-// metaCacheSize is how many variants' content types metaCache holds. A
+// many of its kind wait is dropped.
-// variant not among them is served as before, reading its .meta file.
+const maxPendingAccessTimes = 10000
-const metaCacheSize = 10000
 
 // cleanExpiredBatchSize is the most expired negative cache rows one
 // statement of CleanExpired deletes. Each batch is a statement of its
@@ -69,12 +64,11 @@ type CacheConfig struct {
 
 // Cache implements the caching layer for the image proxy.
 type Cache struct {
-	db          *sql.DB
+	db         *sql.DB
-	srcContent  *ContentStorage  // source images by content hash
+	srcContent *ContentStorage // source images by source key
-	variants    *VariantStorage  // processed variants by cache key
+	variants   *VariantStorage // processed variants by cache key
-	srcMetadata *MetadataStorage // source metadata by host/path
+	config     CacheConfig
-	config      CacheConfig
+	log        *slog.Logger
-	log         *slog.Logger
 
 	// disabled means the disk cache is turned off entirely: lookups
 	// always miss, stores are no-ops, and no eviction runs.
@@ -89,19 +83,23 @@ type Cache struct {
 	evictionCancel   context.CancelFunc
 
 	// The pending counts: hits, misses, upstream fetches and transforms
-	// whose write to the database missed its request's deadline, kept
+	// that requests counted, kept here until they are written by the
-	// here until they are written by the goroutine that
+	// goroutine that StartPendingCountWrites starts, so that no request
-	// StartPendingCountWrites starts. As for eviction, the channels are
+	// waits for the database to count it. pendingVariantAccessTimes holds
-	// created in NewCache: pendingCountsAdded wakes that goroutine, and
+	// the variants served from the cache, and pendingSourceAccessTimes the
-	// pendingCountsDone is closed when it returns. pendingCountsCancel,
+	// cached sources a miss used, whose access time is still to be written
-	// set by StartPendingCountWrites, cancels its context, which tells it
+	// by that goroutine. As for eviction, the channels are created in
-	// to finish.
+	// NewCache, so that requests can hand over access times before that
+	// goroutine starts; pendingCountsDone is closed when it returns.
+	// pendingCountsCancel, set by StartPendingCountWrites, cancels its
+	// context, which tells it to finish.
 	pendingHits               atomic.Int64
 	pendingMisses             atomic.Int64
 	pendingUpstreamFetches    atomic.Int64
 	pendingUpstreamFetchBytes atomic.Int64
 	pendingTransforms         atomic.Int64
-	pendingCountsAdded        chan struct{}
+	pendingVariantAccessTimes chan imagerequest.VariantKey
+	pendingSourceAccessTimes  chan imagerequest.SourceKey
 	pendingCountsDone         chan struct{}
 	pendingCountsCancel       context.CancelFunc
 
@@ -109,24 +107,6 @@ type Cache struct {
 	// readHitAndMissCounts to read them
 	pendingCountsWriteMutex sync.Mutex
 
-	// 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[imagerequest.VariantKey, string]
-
-	// contentLocks serializes StoreSource and evictSourceBlob per
-	// content hash, closing the race window between an eviction's row
-	// deletion and its file unlink against a concurrent store of
-	// identical content.
-	contentLocks *contentLock
-
-	// evictSourceBlobTestHook, when set, is invoked by evictSourceBlob
-	// after its row-deletion transaction commits and before the
-	// content file is unlinked. It exists solely so tests can
-	// deterministically pause inside that window to exercise
-	// concurrent stores against it; production code leaves it nil.
-	evictSourceBlobTestHook func(ContentHash)
-
 	// reconciliationPageSize is the most rows one read of a content table
 	// returns in the reconciliation pass and in Stats. newCache sets it to
 	// defaultReconciliationPageSize; tests set it smaller.
@@ -154,11 +134,6 @@ func newCache(
 		log = slog.Default()
 	}
 
-	metaCache, err := lru.New[imagerequest.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,
@@ -166,11 +141,10 @@ func newCache(
 		disabled:         config.DisableDiskCache,
 		evictionPressure: make(chan struct{}, 1),
 		evictionDone:     make(chan struct{}),
-		metaCache:        metaCache,
-		contentLocks:     newContentLock(),
 
-		pendingCountsAdded: make(chan struct{}, 1),
+		pendingVariantAccessTimes: make(chan imagerequest.VariantKey, maxPendingAccessTimes),
-		pendingCountsDone:  make(chan struct{}),
+		pendingSourceAccessTimes:  make(chan imagerequest.SourceKey, maxPendingAccessTimes),
+		pendingCountsDone:         make(chan struct{}),
 
 		reconciliationPageSize: defaultReconciliationPageSize,
 	}
@@ -187,22 +161,14 @@ func newCache(
 	}
 
 	variants, err := NewVariantStorage(
-		filepath.Join(config.StateDir, "cache", "variants"), log,
+		filepath.Join(config.StateDir, "cache", "variants"),
 	)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create variant storage: %w", err)
 	}
 
-	srcMetadata, err := NewMetadataStorage(
-		filepath.Join(config.StateDir, "cache", "metadata"),
-	)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create source metadata storage: %w", err)
-	}
-
 	c.srcContent = srcContent
 	c.variants = variants
-	c.srcMetadata = srcMetadata
 
 	if config.UseDefaultMaxBytes {
 		limit, err := c.computeDefaultMaxBytes(context.Background(), probe)
@@ -221,45 +187,11 @@ func newCache(
 	return c, nil
 }
 
-// LookupResult contains the result of a cache lookup.
+// GetVariant opens a cached variant and returns a reader, its size and its
-type LookupResult struct {
+// content type, read from the variant file's first bytes. It opens that one
-	Hit         bool
+// file and makes no database call. A variant not in the cache, or any
-	CacheKey    imagerequest.VariantKey
+// variant of a disabled cache, is ErrNotFound; a variant file whose first
-	ContentType string
+// bytes are not those of an output format is another error.
-	SizeBytes   int64
-	CacheStatus CacheStatus
-}
-
-// 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.Request,
-) (*LookupResult, error) {
-	cacheKey := imagerequest.CacheKey(req)
-
-	// Check variant storage directly - no DB needed for cache hits
-	if !c.disabled && c.variants.Exists(cacheKey) {
-		c.touchVariant(ctx, cacheKey)
-
-		return &LookupResult{
-			Hit:         true,
-			CacheKey:    cacheKey,
-			CacheStatus: CacheHit,
-		}, nil
-	}
-
-	return &LookupResult{
-		Hit:         false,
-		CacheKey:    cacheKey,
-		CacheStatus: CacheMiss,
-	}, nil
-}
-
-// 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. A variant with
-// no .meta file is served as application/octet-stream, which is not
-// kept.
 func (c *Cache) GetVariant(
 	cacheKey imagerequest.VariantKey,
 ) (io.ReadCloser, int64, string, error) {
@@ -267,104 +199,62 @@ func (c *Cache) GetVariant(
 		return nil, 0, "", ErrNotFound
 	}
 
-	contentType, known := c.metaCache.Get(cacheKey)
+	return c.variants.LoadWithContentType(cacheKey)
-	if !known {
-		return c.loadVariantWithMeta(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
-	}
-
-	return reader, size, contentType, nil
 }
 
-// StoreSource stores fetched source content and metadata. On a
+// StoreSource stores fetched source content: it writes the file named by
-// disabled cache it is a no-op returning an empty hash.
+// the source's key, then the source's row. When the row cannot be written
+// the file is removed and the store fails, as a file with no row is never
+// served or evicted; if that removal fails too, the next reconciliation
+// pass removes the file. On a disabled cache it is a no-op returning an
+// empty key.
 func (c *Cache) StoreSource(
 	ctx context.Context,
 	req *imagerequest.Request,
 	content io.Reader,
 	result *httpfetcher.FetchResult,
-) (ContentHash, error) {
+) (imagerequest.SourceKey, error) {
 	if c.disabled {
 		return "", nil
 	}
 
-	// Hash the content ourselves (rather than via srcContent.Store,
-	// which would hash internally) so the content hash is known before
-	// any file or database work happens: that lets the entire store be
-	// serialized, per hash, against a concurrent eviction of the same
-	// content below.
 	data, err := io.ReadAll(content)
 	if err != nil {
 		return "", fmt.Errorf("failed to read source content: %w", err)
 	}
 
-	sum := sha256.Sum256(data)
+	key := req.Source.Key()
-	contentHash := ContentHash(hex.EncodeToString(sum[:]))
 
-	// Hold the content hash's lock for the whole store operation. A
+	size, err := c.srcContent.Store(key, data)
-	// concurrent eviction of this exact hash (the real SHA-256 dedup
-	// case: a different source path whose bytes hash identically)
-	// deletes the accounting rows and unlinks the file inside the same
-	// lock, so the two can never interleave: either this store
-	// completes first (and a subsequent eviction removes it together
-	// with its rows and file, correctly), or eviction completes first
-	// (and this store finds the file already gone and recreates it
-	// fresh) — never a fresh row left pointing at a file eviction is
-	// mid-unlink on.
-	unlock := c.contentLocks.Lock(string(contentHash))
-	defer unlock()
-
-	size, err := c.srcContent.StoreHashed(contentHash, data)
 	if err != nil {
 		return "", fmt.Errorf("failed to store source content: %w", err)
 	}
 
-	// Store in database
-	pathHash := HashPath(req.Source.Path + "?" + req.Source.Query)
-
-	headersJSON, err := json.Marshal(result.Headers)
-	if err != nil {
-		return "", fmt.Errorf("failed to marshal response headers: %w", err)
-	}
-
 	_, err = c.db.ExecContext(ctx, `
-		INSERT INTO source_content (content_hash, content_type, size_bytes)
+		INSERT INTO source_content
-		VALUES (?, ?, ?)
+			(source_key, source_host, source_path, source_query,
-		ON CONFLICT(content_hash) DO NOTHING
+			 content_type, size_bytes)
-	`, contentHash, result.ContentType, size)
+		VALUES (?, ?, ?, ?, ?, ?)
+		ON CONFLICT(source_key) DO UPDATE SET
+			content_type = excluded.content_type,
+			size_bytes = excluded.size_bytes,
+			fetched_at = CURRENT_TIMESTAMP,
+			last_accessed_at = CURRENT_TIMESTAMP
+	`, string(key), req.Source.Host, req.Source.Path, req.Source.Query,
+		result.ContentType, size)
 	if err != nil {
+		deleteErr := c.srcContent.Delete(key)
+		if deleteErr != nil {
+			c.log.Warn("failed to remove source file whose row was not written",
+				"source_key", key, "error", deleteErr)
+		}
+
 		return "", fmt.Errorf("failed to insert source content: %w", err)
 	}
 
-	_, err = c.db.ExecContext(ctx, `
-		INSERT INTO source_metadata
-			(source_host, source_path, source_query, path_hash,
-			 content_hash, status_code, content_type, response_headers)
-		VALUES (?, ?, ?, ?, ?, ?, ?, ?)
-		ON CONFLICT(source_host, source_path, source_query) DO UPDATE SET
-			content_hash = excluded.content_hash,
-			status_code = excluded.status_code,
-			content_type = excluded.content_type,
-			response_headers = excluded.response_headers,
-			fetched_at = CURRENT_TIMESTAMP
-	`, req.Source.Host, req.Source.Path, req.Source.Query, pathHash,
-		contentHash, httpStatusOK, result.ContentType, string(headersJSON))
-	if err != nil {
-		return "", fmt.Errorf("failed to insert source metadata: %w", err)
-	}
-
-	c.writeMetadataSidecar(req, pathHash, contentHash, result)
-
 	c.notifyWritePressure()
 
-	return contentHash, nil
+	return key, nil
 }
 
 // StoreVariant stores a processed variant by its cache key and records
@@ -379,13 +269,11 @@ func (c *Cache) StoreVariant(
 		return nil
 	}
 
-	size, err := c.variants.Store(cacheKey, content, contentType)
+	size, err := c.variants.Store(cacheKey, content)
 	if err != nil {
 		return err
 	}
 
-	c.metaCache.Add(cacheKey, contentType)
-
 	_, err = c.db.ExecContext(ctx, `
 		INSERT INTO variant_content (cache_key, size_bytes, content_type)
 		VALUES (?, ?, ?)
@@ -405,22 +293,24 @@ func (c *Cache) StoreVariant(
 }
 
 // LookupSource checks if we have cached source content for a request.
-// Returns the content hash and content type if found, or empty values
+// Returns the source key and content type if found, or empty values
-// if not. Hits touch the blob's LRU timestamp; a disabled cache always
+// if not; a row whose file is missing is not found. A source found has
-// reports no cached source.
+// its access time handed to the background writer without waiting (see
+// touchSourceContent); a disabled cache always reports no cached source.
 func (c *Cache) LookupSource(
 	ctx context.Context, req *imagerequest.Request,
-) (ContentHash, string, error) {
+) (imagerequest.SourceKey, string, error) {
 	if c.disabled {
 		return "", "", nil
 	}
 
-	var hashStr, contentType string
+	key := req.Source.Key()
+
+	var contentType string
 
 	err := c.db.QueryRowContext(ctx, `
-		SELECT content_hash, content_type FROM source_metadata
+		SELECT content_type FROM source_content WHERE source_key = ?
-		WHERE source_host = ? AND source_path = ? AND source_query = ?
+	`, string(key)).Scan(&contentType)
-	`, req.Source.Host, req.Source.Path, req.Source.Query).Scan(&hashStr, &contentType)
 
 	if errors.Is(err, sql.ErrNoRows) {
 		return "", "", nil
@@ -430,16 +320,14 @@ func (c *Cache) LookupSource(
 		return "", "", fmt.Errorf("failed to lookup source: %w", err)
 	}
 
-	contentHash := ContentHash(hashStr)
-
 	// Verify the content file exists
-	if !c.srcContent.Exists(contentHash) {
+	if !c.srcContent.Exists(key) {
 		return "", "", nil
 	}
 
-	c.touchSourceContent(ctx, contentHash)
+	c.touchSourceContent(key)
 
-	return contentHash, contentType, nil
+	return key, contentType, nil
 }
 
 // StoreNegative stores a negative cache entry for a failed fetch.
@@ -466,33 +354,16 @@ func (c *Cache) StoreNegative(
 	return nil
 }
 
-// GetSourceMetadataID returns the source metadata ID for a request.
+// GetSourceContent returns a reader for cached source content by its key,
-func (c *Cache) GetSourceMetadataID(
-	ctx context.Context, req *imagerequest.Request,
-) (int64, error) {
-	var id int64
-
-	err := c.db.QueryRowContext(ctx, `
-		SELECT id FROM source_metadata
-		WHERE source_host = ? AND source_path = ? AND source_query = ?
-	`, req.Source.Host, req.Source.Path, req.Source.Query).Scan(&id)
-	if err != nil {
-		return 0, fmt.Errorf("failed to get source metadata ID: %w", err)
-	}
-
-	return id, nil
-}
-
-// GetSourceContent returns a reader for cached source content by its hash,
 // and the content's size in bytes.
 func (c *Cache) GetSourceContent(
-	contentHash ContentHash,
+	key imagerequest.SourceKey,
 ) (io.ReadCloser, int64, error) {
 	if c.disabled {
 		return nil, 0, ErrNotFound
 	}
 
-	return c.srcContent.LoadWithSize(contentHash)
+	return c.srcContent.LoadWithSize(key)
 }
 
 // CleanExpired removes the negative cache rows past their expiry, one
@@ -555,92 +426,33 @@ func (c *Cache) Stats(ctx context.Context) (*CacheStats, error) {
 
 // IncrementStats counts a cache hit or miss, and an upstream fetch that read
 // fetchBytes bytes, as IncrementUpstreamFetch does. Like the other Increment
-// methods, it writes to the database with ctx's deadline but not its
+// methods, it only adds to the pending counts, which the goroutine
-// cancellation, so an ended request is still counted but does not wait for
+// StartPendingCountWrites starts writes to the database, so it never waits
-// the database past its deadline; a count not written by then becomes a
+// for the database or fails.
-// pending count (see StartPendingCountWrites).
+func (c *Cache) IncrementStats(hit bool, fetchBytes int64) {
-func (c *Cache) IncrementStats(ctx context.Context, hit bool, fetchBytes int64) {
-	countCtx, cancel := withoutCancelKeepingDeadline(ctx)
-	defer cancel()
-
-	var err error
-
-	pendingCount := &c.pendingMisses
-
 	if hit {
-		pendingCount = &c.pendingHits
+		c.pendingHits.Add(1)
-
-		_, err = c.db.ExecContext(countCtx, `
-			UPDATE cache_stats
-			SET hit_count = hit_count + 1,
-			    last_updated_at = CURRENT_TIMESTAMP
-			WHERE id = 1
-		`)
 	} else {
-		_, err = c.db.ExecContext(countCtx, `
+		c.pendingMisses.Add(1)
-			UPDATE cache_stats
-			SET miss_count = miss_count + 1,
-			    last_updated_at = CURRENT_TIMESTAMP
-			WHERE id = 1
-		`)
 	}
 
-	switch {
+	c.IncrementUpstreamFetch(fetchBytes)
-	case errors.Is(err, context.DeadlineExceeded):
-		c.addPendingCount(pendingCount, 1)
-	case err != nil:
-		c.log.Warn("failed to count cache hit or miss", "hit", hit, "error", err)
-	}
-
-	c.IncrementUpstreamFetch(ctx, fetchBytes)
 }
 
 // IncrementUpstreamFetch counts one upstream fetch that read fetchBytes bytes.
 // A fetch that read no bytes is not counted.
-func (c *Cache) IncrementUpstreamFetch(ctx context.Context, fetchBytes int64) {
+func (c *Cache) IncrementUpstreamFetch(fetchBytes int64) {
 	if fetchBytes <= 0 {
 		return
 	}
 
-	countCtx, cancel := withoutCancelKeepingDeadline(ctx)
+	c.pendingUpstreamFetches.Add(1)
-	defer cancel()
+	c.pendingUpstreamFetchBytes.Add(fetchBytes)
-
-	_, err := c.db.ExecContext(countCtx, `
-		UPDATE cache_stats
-		SET upstream_fetch_count = upstream_fetch_count + 1,
-		    upstream_fetch_bytes = upstream_fetch_bytes + ?,
-		    last_updated_at = CURRENT_TIMESTAMP
-		WHERE id = 1
-	`, fetchBytes)
-
-	switch {
-	case errors.Is(err, context.DeadlineExceeded):
-		c.addPendingCount(&c.pendingUpstreamFetches, 1)
-		c.addPendingCount(&c.pendingUpstreamFetchBytes, fetchBytes)
-	case err != nil:
-		c.log.Warn("failed to count upstream fetch",
-			"fetch_bytes", fetchBytes, "error", err)
-	}
 }
 
 // IncrementTransformCount counts one image transcoded by the image processor.
-func (c *Cache) IncrementTransformCount(ctx context.Context) {
+func (c *Cache) IncrementTransformCount() {
-	countCtx, cancel := withoutCancelKeepingDeadline(ctx)
+	c.pendingTransforms.Add(1)
-	defer cancel()
-
-	_, err := c.db.ExecContext(countCtx, `
-		UPDATE cache_stats
-		SET transform_count = transform_count + 1,
-		    last_updated_at = CURRENT_TIMESTAMP
-		WHERE id = 1
-	`)
-
-	switch {
-	case errors.Is(err, context.DeadlineExceeded):
-		c.addPendingCount(&c.pendingTransforms, 1)
-	case err != nil:
-		c.log.Warn("failed to count transform", "error", err)
-	}
 }
 
 // readHitAndMissCounts sets the hit and miss counts of stats to those of the
@@ -662,88 +474,33 @@ func (c *Cache) readHitAndMissCounts(ctx context.Context, stats *CacheStats) err
 	return err
 }
 
-// loadVariantWithMeta is GetVariant for a variant metaCache does not
+// touchVariant hands the access time of a variant served from the cache to
-// hold: it reads the content type from the variant's .meta file and
+// the goroutine StartPendingCountWrites starts, which writes it in its next
-// keeps it in metaCache, unless a StoreVariant has put one there
+// batch. It never waits: while maxPendingAccessTimes access times of
-// meanwhile, as the store's is newer. A read that finds no .meta file,
+// variants wait to be written, this one is dropped, and the variant only
-// as one can when the store's .meta write failed or an eviction removed
+// looks colder to eviction.
-// the file after the read opened the variant, serves
+func (c *Cache) touchVariant(cacheKey imagerequest.VariantKey) {
-// application/octet-stream and keeps nothing, so metaCache only ever
+	select {
-// holds a type read from a .meta file or passed to StoreVariant.
+	case c.pendingVariantAccessTimes <- cacheKey:
-func (c *Cache) loadVariantWithMeta(
+	default:
-	cacheKey imagerequest.VariantKey,
-) (io.ReadCloser, int64, string, error) {
-	reader, size, contentType, err := c.variants.LoadWithMeta(cacheKey)
-	if err != nil {
-		return nil, 0, "", err
-	}
-
-	if contentType == "" {
-		return reader, size, fallbackContentType, nil
-	}
-
-	c.metaCache.ContainsOrAdd(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.
-func (c *Cache) writeMetadataSidecar(
-	req *imagerequest.Request,
-	pathHash PathHash,
-	contentHash ContentHash,
-	result *httpfetcher.FetchResult,
-) {
-	meta := &SourceMetadata{
-		Host:            req.Source.Host,
-		Path:            req.Source.Path,
-		Query:           req.Source.Query,
-		ContentHash:     string(contentHash),
-		StatusCode:      result.StatusCode,
-		ContentType:     result.ContentType,
-		ContentLength:   result.ContentLength,
-		ResponseHeaders: result.Headers,
-		FetchedAt:       time.Now().UTC().Unix(),
-		FetchDurationMs: result.FetchDurationMs,
-		RemoteAddr:      result.RemoteAddr,
-	}
-
-	err := c.srcMetadata.Store(req.Source.Host, pathHash, meta)
-	if err != nil {
-		c.log.Warn("failed to write metadata sidecar",
-			"host", req.Source.Host, "path_hash", pathHash, "error", err)
 	}
 }
 
-// touchVariant updates the LRU timestamp of a variant, best-effort:
+// touchSourceContent hands the access time of a cached source a miss used to
-// a failed touch only makes the entry look colder to eviction.
+// the goroutine StartPendingCountWrites starts, which writes it in its next
-func (c *Cache) touchVariant(ctx context.Context, cacheKey imagerequest.VariantKey) {
+// batch. It never waits: while maxPendingAccessTimes access times of sources
-	_, err := c.db.ExecContext(ctx, `
+// wait to be written, this one is dropped, and the source only looks colder
-		UPDATE variant_content SET last_accessed_at = CURRENT_TIMESTAMP
+// to eviction.
-		WHERE cache_key = ?
+func (c *Cache) touchSourceContent(key imagerequest.SourceKey) {
-	`, string(cacheKey))
+	select {
-	if err != nil {
+	case c.pendingSourceAccessTimes <- key:
-		c.log.Debug("failed to touch variant LRU timestamp",
+	default:
-			"cache_key", cacheKey, "error", err)
 	}
 }
 
-// touchSourceContent updates the LRU timestamp of a source content
+// checkNegativeCache checks if a request is in the negative cache. It only
-// blob, best-effort: a failed touch only makes the blob look colder.
+// reads: a row past its expiry is not a hit, and the eviction goroutine
-func (c *Cache) touchSourceContent(ctx context.Context, contentHash ContentHash) {
+// deletes it with the other expired rows (see CleanExpired).
-	_, err := c.db.ExecContext(ctx, `
-		UPDATE source_content SET last_accessed_at = CURRENT_TIMESTAMP
-		WHERE content_hash = ?
-	`, string(contentHash))
-	if err != nil {
-		c.log.Debug("failed to touch source content LRU timestamp",
-			"content_hash", contentHash, "error", err)
-	}
-}
-
-// checkNegativeCache checks if a request is in the negative cache.
 func (c *Cache) checkNegativeCache(
 	ctx context.Context, req *imagerequest.Request,
 ) (bool, error) {
@@ -762,22 +519,9 @@ func (c *Cache) checkNegativeCache(
 		return false, fmt.Errorf("failed to check negative cache: %w", err)
 	}
 
-	// Check if expired
+	expired := time.Now().After(expiresAt)
-	if time.Now().After(expiresAt) {
-		// Clean up expired entry
-		_, err = c.db.ExecContext(ctx, `
-			DELETE FROM negative_cache
-			WHERE source_host = ? AND source_path = ? AND source_query = ?
-		`, req.Source.Host, req.Source.Path, req.Source.Query)
-		if err != nil {
-			c.log.Warn("failed to delete expired negative cache entry",
-				"host", req.Source.Host, "path", req.Source.Path, "error", err)
-		}
 
-		return false, nil
+	return !expired, nil
-	}
-
-	return true, nil
 }
 
 // cleanExpiredBatch deletes up to cleanExpiredBatchSize negative cache

internal/imgcache/cache_hit_internal_test.go

@@ -0,0 +1,299 @@
+package imgcache
+
+import (
+	"bytes"
+	"errors"
+	"fmt"
+	"image/color"
+	"io"
+	"os"
+	"testing"
+	"time"
+
+	"sneak.berlin/go/pixa/internal/imageprocessor"
+	"sneak.berlin/go/pixa/internal/imagerequest"
+	"sneak.berlin/go/pixa/internal/magic"
+)
+
+// TestGetVariant_ContentTypeFromFirstBytes stores an image in each output
+// format as a variant, recorded with the content type text/plain, and checks
+// that GetVariant serves it whole with the content type of its format, read
+// from the file.
+func TestGetVariant_ContentTypeFromFirstBytes(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	processor := imageprocessor.New(imageprocessor.Params{})
+	source := generateTestJPEG(t, 40, 30, color.RGBA{200, 40, 40, 255})
+
+	for _, format := range []imagerequest.Format{
+		imagerequest.FormatJPEG, imagerequest.FormatPNG, imagerequest.FormatGIF,
+		imagerequest.FormatWebP, imagerequest.FormatAVIF, imagerequest.FormatJXL,
+	} {
+		result, err := processor.Process(t.Context(), bytes.NewReader(source),
+			&imagerequest.Transform{
+				Size:    imagerequest.Size{Width: 20, Height: 15},
+				Format:  format,
+				Quality: 85,
+				FitMode: imagerequest.FitCover,
+			})
+		if err != nil {
+			t.Fatalf("making a %s image: %v", format, err)
+		}
+
+		image, err := io.ReadAll(result.Content)
+		if err != nil {
+			t.Fatalf("reading the %s image: %v", format, err)
+		}
+
+		key := imagerequest.VariantKey("aabbccdd10" + string(format))
+
+		err = cache.StoreVariant(t.Context(), key, bytes.NewReader(image), "text/plain")
+		if err != nil {
+			t.Fatalf("StoreVariant(%s) error = %v", format, err)
+		}
+
+		reader, size, contentType, err := cache.GetVariant(key)
+		if err != nil {
+			t.Fatalf("GetVariant(%s) error = %v", format, err)
+		}
+
+		served, err := io.ReadAll(reader)
+		_ = reader.Close()
+
+		if err != nil {
+			t.Fatalf("reading the %s variant: %v", format, err)
+		}
+
+		t.Logf("%s: %d bytes served as %s", format, size, contentType)
+
+		if want := imageprocessor.FormatToMIME(format); contentType != want {
+			t.Errorf("%s variant content type = %q, want %q", format, contentType, want)
+		}
+
+		if size != int64(len(image)) || !bytes.Equal(served, image) {
+			t.Errorf("%s variant served %d bytes, size %d, want the %d bytes stored",
+				format, len(served), size, len(image))
+		}
+	}
+}
+
+// TestGetVariant_RefusesFileOfNoOutputFormat checks that GetVariant refuses
+// a variant file whose first bytes are not those of an output format, with
+// an error other than ErrNotFound, and that a variant with no file, or any
+// variant of a disabled cache, is ErrNotFound.
+func TestGetVariant_RefusesFileOfNoOutputFormat(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	jpegImage := generateTestJPEG(t, 10, 10, color.RGBA{0, 0, 255, 255})
+
+	tests := []struct {
+		name    string
+		content []byte
+		wantErr error
+	}{
+		{"text", []byte("this is text, not an image"), magic.ErrUnknownFormat},
+		{"SVG", []byte(`<svg xmlns="http://www.w3.org/2000/svg"></svg>`),
+			errNotAnOutputFormat},
+		{"JPEG cut to 10 bytes", jpegImage[:10], io.EOF},
+		{"empty", nil, io.EOF},
+	}
+
+	for i, tt := range tests {
+		key := imagerequest.VariantKey(fmt.Sprintf("aabbccdd20%02d", i))
+
+		_, err := cache.variants.Store(key, bytes.NewReader(tt.content))
+		if err != nil {
+			t.Fatalf("%s: storing the variant file: %v", tt.name, err)
+		}
+
+		reader, _, contentType, err := cache.GetVariant(key)
+		t.Logf("%s: GetVariant() error = %v", tt.name, err)
+
+		if reader != nil || contentType != "" || !errors.Is(err, tt.wantErr) {
+			t.Errorf("%s: GetVariant() = reader %v, content type %q, error %v; "+
+				"want no reader and %v", tt.name, reader, contentType, err, tt.wantErr)
+		}
+	}
+
+	reader, _, _, err := cache.GetVariant(testVariantKeyFour)
+	if reader != nil || !errors.Is(err, ErrNotFound) {
+		t.Errorf("GetVariant() of a variant with no file = reader %v, error %v; "+
+			"want no reader and %v", reader, err, ErrNotFound)
+	}
+
+	disabled, _ := newEvictionTestCache(t, 0)
+
+	reader, _, _, err = disabled.GetVariant(testVariantKeyOne)
+	if reader != nil || !errors.Is(err, ErrNotFound) {
+		t.Errorf("GetVariant() on a disabled cache = reader %v, error %v; "+
+			"want no reader and %v", reader, err, ErrNotFound)
+	}
+}
+
+// TestService_Get_MakesAgainVariantOfNoOutputFormat replaces a cached
+// variant's file with text, and checks that the next request is a miss
+// answered with the variant made again, and the one after a hit.
+func TestService_Get_MakesAgainVariantOfNoOutputFormat(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+	req := photoVariant(fixtures, 85, imagerequest.FitCover)
+
+	want, _ := getImage(t, svc, req)
+
+	path := svc.cache.variants.keyToPath(imagerequest.CacheKey(req))
+
+	err := os.WriteFile(path, []byte("this is text, not an image"), StorageFilePerm)
+	if err != nil {
+		t.Fatalf("replacing the variant file: %v", err)
+	}
+
+	got, status := getImage(t, svc, req)
+	t.Logf("after the replacement: %d bytes, %s", len(got), status)
+
+	if status != CacheMiss || !bytes.Equal(got, want) {
+		t.Errorf("after the replacement: Get() = %d bytes, %s; want the %d bytes "+
+			"made again, %s", len(got), status, len(want), CacheMiss)
+	}
+
+	got, status = getImage(t, svc, req)
+
+	if status != CacheHit || !bytes.Equal(got, want) {
+		t.Errorf("next request: Get() = %d bytes, %s; want the %d bytes, %s",
+			len(got), status, len(want), CacheHit)
+	}
+}
+
+// TestService_Get_HitWaitsForNoDatabaseWrite holds the database's one
+// connection while a cached variant is requested. The hit must be answered
+// while the connection is still held, and its access time and hit count
+// must reach the database once it is free.
+func TestService_Get_HitWaitsForNoDatabaseWrite(t *testing.T) {
+	t.Parallel()
+
+	svc, fixtures := SetupTestService(t)
+	req := photoVariant(fixtures, 85, imagerequest.FitCover)
+	key := imagerequest.CacheKey(req)
+
+	getImage(t, svc, req)
+	waitForPendingCountsWritten(t, svc.cache)
+
+	anHourAgo := time.Now().Add(-time.Hour)
+	setVariantLastAccessed(t, svc.cache, key, anHourAgo)
+
+	want := readCacheStatsCounters(t, svc.cache)
+	want.hitCount++
+
+	releaseDatabase := holdDatabase(t, svc.cache)
+	defer releaseDatabase()
+
+	results := startGet(t.Context(), svc, req)
+
+	select {
+	case got := <-results:
+		if got.err != nil {
+			t.Fatalf("Get() of the cached variant error = %v", got.err)
+		}
+	case <-time.After(pendingCountsWait):
+		t.Fatal("the cache hit waited for the database")
+	}
+
+	releaseDatabase()
+
+	waitForCounters(t, svc.cache, want)
+
+	deadline := time.Now().Add(pendingCountsWait)
+
+	for countRows(t, svc.cache, `
+		SELECT COUNT(*) FROM variant_content
+		WHERE cache_key = ? AND last_accessed_at > ?
+	`, string(key), anHourAgo.UTC().Format(sqliteTimestampFormat)) == 0 {
+		if time.Now().After(deadline) {
+			t.Fatal("the hit's access time was not written")
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
+}
+
+// TestTouchVariantAndTouchSourceContent_DropAccessTimesPastTheLimit hands one
+// more access time of a variant, and one more of a source, than may wait to
+// be written. touchVariant and touchSourceContent must return each time
+// without waiting, and the last of each must be dropped.
+func TestTouchVariantAndTouchSourceContent_DropAccessTimesPastTheLimit(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	done := make(chan struct{})
+
+	go func() {
+		defer close(done)
+
+		for range maxPendingAccessTimes + 1 {
+			cache.touchVariant(testVariantKeyOne)
+			cache.touchSourceContent("aabbccdd3001")
+		}
+	}()
+
+	select {
+	case <-done:
+	case <-time.After(pendingCountsWait):
+		t.Fatal("touchVariant or touchSourceContent waited once the access " +
+			"times reached the limit")
+	}
+
+	if waiting := len(cache.pendingVariantAccessTimes); waiting != maxPendingAccessTimes {
+		t.Errorf("%d access times of variants wait to be written, want %d",
+			waiting, maxPendingAccessTimes)
+	}
+
+	if waiting := len(cache.pendingSourceAccessTimes); waiting != maxPendingAccessTimes {
+		t.Errorf("%d access times of sources wait to be written, want %d",
+			waiting, maxPendingAccessTimes)
+	}
+}
+
+// TestStopPendingCountWrites_WritesPendingAccessTimes hands the access time
+// of one of two variants, three times, then stops the goroutine that writes
+// them, which never started. The stop must write that access time and
+// leave the other variant's.
+func TestStopPendingCountWrites_WritesPendingAccessTimes(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	anHourAgo := time.Now().Add(-time.Hour)
+
+	for _, key := range []imagerequest.VariantKey{testVariantKeyOne, testVariantKeyTwo} {
+		storeEvictionTestVariant(t, cache, key, []byte("variant bytes"))
+		setVariantLastAccessed(t, cache, key, anHourAgo)
+	}
+
+	for range 3 {
+		cache.touchVariant(testVariantKeyOne)
+	}
+
+	err := cache.StopPendingCountWrites(t.Context())
+	if err != nil {
+		t.Fatalf("StopPendingCountWrites() error = %v", err)
+	}
+
+	for key, wantTouched := range map[imagerequest.VariantKey]int{
+		testVariantKeyOne: 1, testVariantKeyTwo: 0,
+	} {
+		touched := countRows(t, cache, `
+			SELECT COUNT(*) FROM variant_content
+			WHERE cache_key = ? AND last_accessed_at > ?
+		`, string(key), anHourAgo.UTC().Format(sqliteTimestampFormat))
+		if touched != wantTouched {
+			t.Errorf("variant %s: %d rows with a new access time, want %d",
+				key, touched, wantTouched)
+		}
+	}
+
+	if waiting := len(cache.pendingVariantAccessTimes); waiting != 0 {
+		t.Errorf("%d access times still wait to be written, want 0", waiting)
+	}
+}

internal/imgcache/cache_internal_test.go

@@ -1,7 +1,6 @@
 package imgcache
 
 import (
-	"bytes"
 	"context"
 	"database/sql"
 	"os"
@@ -9,8 +8,6 @@ import (
 	"time"
 
 	_ "modernc.org/sqlite"
-	"sneak.berlin/go/pixa/internal/httpfetcher"
-	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
 func setupTestDB(t *testing.T) *sql.DB {
@@ -24,26 +21,14 @@ func setupTestDB(t *testing.T) *sql.DB {
 	// Create schema
 	schema := `
 		CREATE TABLE source_content (
-			content_hash TEXT PRIMARY KEY,
+			source_key TEXT PRIMARY KEY,
-			content_type TEXT NOT NULL,
-			size_bytes INTEGER NOT NULL,
-			fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP
-		);
-		CREATE TABLE source_metadata (
-			id INTEGER PRIMARY KEY AUTOINCREMENT,
 			source_host TEXT NOT NULL,
 			source_path TEXT NOT NULL,
 			source_query TEXT NOT NULL DEFAULT '',
-			path_hash TEXT NOT NULL,
+			content_type TEXT NOT NULL,
-			content_hash TEXT,
+			size_bytes INTEGER NOT NULL,
-			status_code INTEGER NOT NULL,
-			content_type TEXT,
-			response_headers TEXT,
 			fetched_at DATETIME DEFAULT CURRENT_TIMESTAMP,
-			expires_at DATETIME,
+			last_accessed_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP
-			etag TEXT,
-			last_modified TEXT,
-			UNIQUE(source_host, source_path, source_query)
 		);
 		CREATE TABLE output_content (
 			content_hash TEXT PRIMARY KEY,
@@ -112,347 +97,6 @@ func setupTestCache(t *testing.T) *Cache {
 	return cache
 }
 
-func TestCache_LookupMiss(t *testing.T) {
-	t.Parallel()
-
-	cache := setupTestCache(t)
-	ctx := context.Background()
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: testPathCat,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 800, Height: 600},
-			Format:  imagerequest.FormatWebP,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	result, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() error = %v", err)
-	}
-
-	if result.Hit {
-		t.Error("Lookup() hit = true, want false for cache miss")
-	}
-
-	if result.CacheStatus != CacheMiss {
-		t.Errorf("Lookup() status = %v, want %v", result.CacheStatus, CacheMiss)
-	}
-}
-
-func TestCache_StoreAndLookup(t *testing.T) {
-	t.Parallel()
-
-	cache := setupTestCache(t)
-	ctx := context.Background()
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: testPathCat,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 800, Height: 600},
-			Format:  imagerequest.FormatWebP,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	// Store source content
-	sourceContent := []byte("fake jpeg data")
-	fetchResult := &httpfetcher.FetchResult{
-		ContentType: testContentTypeJPEG,
-		Headers:     map[string][]string{testHeaderContentType: {testContentTypeJPEG}},
-	}
-
-	contentHash, err := cache.StoreSource(
-		ctx, req, bytes.NewReader(sourceContent), fetchResult)
-	if err != nil {
-		t.Fatalf("StoreSource() error = %v", err)
-	}
-
-	if contentHash == "" {
-		t.Error("StoreSource() returned empty hash")
-	}
-
-	// Store variant
-	cacheKey := imagerequest.CacheKey(req)
-	outputContent := []byte("fake webp data")
-
-	err = cache.StoreVariant(
-		t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
-	if err != nil {
-		t.Fatalf("StoreVariant() error = %v", err)
-	}
-
-	// Now lookup should hit
-	result, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() error = %v", err)
-	}
-
-	if !result.Hit {
-		t.Error("Lookup() hit = false, want true after store")
-	}
-
-	if result.CacheStatus != CacheHit {
-		t.Errorf("Lookup() status = %v, want %v", result.CacheStatus, CacheHit)
-	}
-
-	if result.CacheKey == "" {
-		t.Error("Lookup() returned empty cache key")
-	}
-}
-
-func TestCache_NegativeCache(t *testing.T) {
-	t.Parallel()
-
-	cache := setupTestCache(t)
-	ctx := context.Background()
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: "/photos/notfound.jpg",
-		},
-		Transform: imagerequest.Transform{
-			Size:   imagerequest.Size{Width: 800, Height: 600},
-			Format: imagerequest.FormatWebP,
-		},
-	}
-
-	// Store negative cache entry
-	err := cache.StoreNegative(ctx, req, 404, "not found")
-	if err != nil {
-		t.Fatalf("StoreNegative() error = %v", err)
-	}
-
-	// Lookup should return miss (negative cache is checked at service level)
-	result, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() error = %v", err)
-	}
-
-	if result.Hit {
-		t.Error("Lookup() hit = true, want false for negative cache entry")
-	}
-}
-
-func TestCache_NegativeCacheExpiry(t *testing.T) {
-	t.Parallel()
-
-	tmpDir := t.TempDir()
-	db := setupTestDB(t)
-
-	// Very short negative TTL for testing
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    tmpDir,
-		NegativeTTL: 1 * time.Millisecond,
-	})
-	if err != nil {
-		t.Fatalf("failed to create cache: %v", err)
-	}
-
-	ctx := context.Background()
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: "/photos/expired.jpg",
-		},
-		Transform: imagerequest.Transform{
-			Size:   imagerequest.Size{Width: 800, Height: 600},
-			Format: imagerequest.FormatWebP,
-		},
-	}
-
-	// Store negative cache entry
-	err = cache.StoreNegative(ctx, req, 404, "not found")
-	if err != nil {
-		t.Fatalf("StoreNegative() error = %v", err)
-	}
-
-	// Wait for expiry
-	time.Sleep(10 * time.Millisecond)
-
-	// Lookup should return miss after expiry
-	result, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() error = %v, want nil after expiry", err)
-	}
-
-	if result.Hit {
-		t.Error("Lookup() hit = true, want false after negative cache expiry")
-	}
-}
-
-func TestCache_VariantLookup(t *testing.T) {
-	t.Parallel()
-
-	cache := setupTestCache(t)
-	ctx := context.Background()
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: "/photos/variant.jpg",
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 800, Height: 600},
-			Format:  imagerequest.FormatWebP,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	// Store variant
-	cacheKey := imagerequest.CacheKey(req)
-	outputContent := []byte("output data")
-
-	err := cache.StoreVariant(
-		t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
-	if err != nil {
-		t.Fatalf("StoreVariant() error = %v", err)
-	}
-
-	// First lookup
-	result1, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() first error = %v", err)
-	}
-
-	if !result1.Hit {
-		t.Error("Lookup() first hit = false")
-	}
-
-	// Second lookup should also hit (from disk)
-	result2, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() second error = %v", err)
-	}
-
-	if !result2.Hit {
-		t.Error("Lookup() second hit = false")
-	}
-
-	if result1.CacheKey != result2.CacheKey {
-		t.Error("Lookup() returned different cache keys")
-	}
-}
-
-func TestCache_GetVariant_ReturnsContentType(t *testing.T) {
-	t.Parallel()
-
-	cache := setupTestCache(t)
-	ctx := context.Background()
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: "/photos/variantct.jpg",
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 800, Height: 600},
-			Format:  imagerequest.FormatWebP,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	// Store variant
-	cacheKey := imagerequest.CacheKey(req)
-	outputContent := []byte("output webp data")
-
-	err := cache.StoreVariant(
-		t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
-	if err != nil {
-		t.Fatalf("StoreVariant() error = %v", err)
-	}
-
-	// Lookup
-	result, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() error = %v", err)
-	}
-
-	if !result.Hit {
-		t.Fatal("Lookup() hit = false, want true")
-	}
-
-	// GetVariant should return content type
-	reader, size, contentType, err := cache.GetVariant(result.CacheKey)
-	if err != nil {
-		t.Fatalf("GetVariant() error = %v", err)
-	}
-
-	defer func() { _ = reader.Close() }()
-
-	if contentType != "image/webp" {
-		t.Errorf("GetVariant() ContentType = %q, want %q", contentType, "image/webp")
-	}
-
-	if size != int64(len(outputContent)) {
-		t.Errorf("GetVariant() size = %d, want %d", size, len(outputContent))
-	}
-}
-
-func TestCache_GetVariant(t *testing.T) {
-	t.Parallel()
-
-	cache := setupTestCache(t)
-	ctx := context.Background()
-
-	req := &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: "/photos/output.jpg",
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 800, Height: 600},
-			Format:  imagerequest.FormatWebP,
-			Quality: 85,
-			FitMode: imagerequest.FitCover,
-		},
-	}
-
-	// Store variant
-	cacheKey := imagerequest.CacheKey(req)
-	outputContent := []byte("the actual output content")
-
-	err := cache.StoreVariant(
-		t.Context(), cacheKey, bytes.NewReader(outputContent), "image/webp")
-	if err != nil {
-		t.Fatalf("StoreVariant() error = %v", err)
-	}
-
-	// Lookup to get cache key
-	result, err := cache.Lookup(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup() error = %v", err)
-	}
-
-	// Get variant content
-	reader, _, _, err := cache.GetVariant(result.CacheKey)
-	if err != nil {
-		t.Fatalf("GetVariant() error = %v", err)
-	}
-
-	defer func() { _ = reader.Close() }()
-
-	buf := make([]byte, 100)
-	n, _ := reader.Read(buf)
-
-	if !bytes.Equal(buf[:n], outputContent) {
-		t.Errorf("GetVariant() content = %q, want %q", buf[:n], outputContent)
-	}
-}
-
 func TestCache_Stats(t *testing.T) {
 	t.Parallel()
 
@@ -460,9 +104,9 @@ func TestCache_Stats(t *testing.T) {
 	ctx := context.Background()
 
 	// Increment some stats
-	cache.IncrementStats(ctx, true, 0)
+	cache.IncrementStats(true, 0)
-	cache.IncrementStats(ctx, true, 0)
+	cache.IncrementStats(true, 0)
-	cache.IncrementStats(ctx, false, 1024)
+	cache.IncrementStats(false, 1024)
 
 	stats, err := cache.Stats(ctx)
 	if err != nil {
@@ -547,7 +191,6 @@ func TestCache_StorageDirectoriesCreated(t *testing.T) {
 	dirs := []string{
 		"cache/sources",
 		"cache/variants",
-		"cache/metadata",
 	}
 
 	for _, dir := range dirs {

internal/imgcache/cache_usage_internal_test.go

@@ -31,10 +31,10 @@ func TestEvictionReadsTheUsageTotal(t *testing.T) {
 	storeEvictionTestVariant(t, cache, testVariantKeyTwo,
 		bytes.Repeat([]byte{0x75}, 200))
 
-	// pixa never changes a source's size, but a statement that does must
+	// Storing a source URL again changes its size, so the total must
-	// change the total too.
+	// follow a statement that changes it.
 	_, err := cache.db.ExecContext(ctx,
-		`UPDATE source_content SET size_bytes = 900 WHERE content_hash = ?`,
+		`UPDATE source_content SET size_bytes = 900 WHERE source_key = ?`,
 		string(kept))
 	if err != nil {
 		t.Fatalf("failed to change the source's size: %v", err)
@@ -155,8 +155,9 @@ func TestUsageTotalNotCorrectedFromAnOlderSum(t *testing.T) {
 // pass. No read the pass makes of a content table, to check its rows or
 // to sum them, may cover more than two rows, so a request's query waits
 // for one page at most. Between them the reads must still cover every
-// row of both tables twice, once to check it and once to sum it, and the
+// variant row twice, once to check it and once to sum it, and every
-// sum must put a wrong total right.
+// source row once, as source rows are read only to sum them, and the sum
+// must put a wrong total right.
 func TestReconciliationReadsAPageAtATime(t *testing.T) {
 	t.Parallel()
 
@@ -207,9 +208,10 @@ func TestReconciliationReadsAPageAtATime(t *testing.T) {
 		coveredRows += rows
 	}
 
-	// Ten rows, each read once to check it and once to sum it.
+	// Five variant rows, each read once to check it and once to sum it,
-	if coveredRows != 20 {
+	// and five source rows, read only to sum them.
-		t.Errorf("the reads covered %d rows in all, want 20", coveredRows)
+	if coveredRows != 15 {
+		t.Errorf("the reads covered %d rows in all, want 15", coveredRows)
 	}
 
 	usage, err := cache.UsageBytes(ctx)

internal/imgcache/concurrent_misses_internal_test.go

@@ -193,6 +193,8 @@ func TestService_Get_ConcurrentMissesShareOneFetch(t *testing.T) {
 
 	want := cacheStatsCounters{0, requests, 1, int64(len(photo)), 1}
 
+	waitForPendingCountsWritten(t, svc.cache)
+
 	if got := readCacheStatsCounters(t, svc.cache); got != want {
 		t.Errorf("counters = %+v, want %+v", got, want)
 	}

internal/imgcache/contentlock.go

@@ -1,64 +0,0 @@
-package imgcache
-
-import "sync"
-
-// contentLock provides per-key mutual exclusion for content-hash keyed
-// operations. StoreSource and evictSourceBlob each hold a content
-// hash's lock for the full duration of their file-plus-accounting-row
-// work, so a store and an eviction racing on identical content bytes
-// (the real SHA-256 content-addressed dedup case, not a contrived one)
-// can never interleave: the unlink of an evicted blob's file can never
-// race the creation of a fresh database row for a concurrently
-// re-stored copy of the same content. Entries are removed once no
-// goroutine holds or is waiting for them, so a long-running process
-// does not accumulate memory proportional to the number of distinct
-// content hashes it has ever seen.
-type contentLock struct {
-	mu      sync.Mutex
-	entries map[string]*contentLockEntry
-}
-
-// contentLockEntry is one key's exclusion lock plus a count of
-// goroutines currently holding or waiting to acquire it, used to know
-// when it is safe to remove the entry from the map.
-type contentLockEntry struct {
-	mu    sync.Mutex
-	count int
-}
-
-// newContentLock creates an empty contentLock.
-func newContentLock() *contentLock {
-	return &contentLock{entries: make(map[string]*contentLockEntry)}
-}
-
-// Lock acquires exclusive access for key, blocking until it is
-// available, and returns a function that releases it. The caller must
-// invoke the returned function exactly once to release the lock.
-func (c *contentLock) Lock(key string) func() {
-	c.mu.Lock()
-
-	entry, ok := c.entries[key]
-	if !ok {
-		entry = &contentLockEntry{}
-		c.entries[key] = entry
-	}
-
-	entry.count++
-
-	c.mu.Unlock()
-
-	entry.mu.Lock()
-
-	return func() {
-		entry.mu.Unlock()
-
-		c.mu.Lock()
-
-		entry.count--
-		if entry.count == 0 {
-			delete(c.entries, key)
-		}
-
-		c.mu.Unlock()
-	}
-}

internal/imgcache/contentlock_internal_test.go

@@ -1,142 +0,0 @@
-package imgcache
-
-import (
-	"sync"
-	"sync/atomic"
-	"testing"
-	"time"
-)
-
-// TestContentLockExcludesSameKey verifies that two goroutines locking
-// the same key never run their critical sections concurrently.
-func TestContentLockExcludesSameKey(t *testing.T) {
-	t.Parallel()
-
-	lock := newContentLock()
-
-	var (
-		active  atomic.Int32
-		maxSeen int32
-		wg      sync.WaitGroup
-	)
-
-	const goroutines = 20
-
-	wg.Add(goroutines)
-
-	for range goroutines {
-		go func() {
-			defer wg.Done()
-
-			unlock := lock.Lock("same-key")
-			defer unlock()
-
-			n := active.Add(1)
-
-			for {
-				seen := atomic.LoadInt32(&maxSeen)
-				if n <= seen || atomic.CompareAndSwapInt32(&maxSeen, seen, n) {
-					break
-				}
-			}
-
-			time.Sleep(time.Millisecond)
-
-			active.Add(-1)
-		}()
-	}
-
-	wg.Wait()
-
-	if maxSeen != 1 {
-		t.Errorf("max concurrent holders of the same key = %d, want 1", maxSeen)
-	}
-}
-
-// TestContentLockAllowsDifferentKeys verifies that locking distinct
-// keys does not serialize unrelated work: all goroutines must be able
-// to enter their critical sections at once, proven by every one of
-// them reaching the rendezvous point before any is allowed to
-// proceed.
-func TestContentLockAllowsDifferentKeys(t *testing.T) {
-	t.Parallel()
-
-	lock := newContentLock()
-
-	const goroutines = 20
-
-	var (
-		wg      sync.WaitGroup
-		inside  atomic.Int32
-		reached = make(chan struct{}, goroutines)
-	)
-
-	wg.Add(goroutines)
-
-	release := make(chan struct{})
-
-	for i := range goroutines {
-		key := string(rune('a' + i))
-
-		go func() {
-			defer wg.Done()
-
-			unlock := lock.Lock(key)
-			defer unlock()
-
-			inside.Add(1)
-
-			reached <- struct{}{}
-
-			<-release
-		}()
-	}
-
-	// Every goroutine must reach the rendezvous point (i.e. acquire its
-	// own key's lock) without needing any other to release first. If
-	// keys were incorrectly serialized onto one underlying lock, only
-	// one would get here and this would time out.
-	for i := range goroutines {
-		select {
-		case <-reached:
-		case <-time.After(2 * time.Second):
-			t.Fatalf("only %d/%d goroutines locking distinct keys made progress; "+
-				"keys may be incorrectly serialized", i, goroutines)
-		}
-	}
-
-	if n := inside.Load(); n != goroutines {
-		t.Errorf("goroutines inside their critical section = %d, want %d",
-			n, goroutines)
-	}
-
-	close(release)
-	wg.Wait()
-}
-
-// TestContentLockRemovesEntryAfterUnlock verifies that the internal
-// entries map does not grow without bound: once no goroutine holds or
-// awaits a key, its entry is removed.
-func TestContentLockRemovesEntryAfterUnlock(t *testing.T) {
-	t.Parallel()
-
-	lock := newContentLock()
-
-	unlock := lock.Lock("k")
-
-	lock.mu.Lock()
-	if _, ok := lock.entries["k"]; !ok {
-		lock.mu.Unlock()
-		t.Fatal("entry missing while lock is held")
-	}
-	lock.mu.Unlock()
-
-	unlock()
-
-	lock.mu.Lock()
-	defer lock.mu.Unlock()
-
-	if _, ok := lock.entries["k"]; ok {
-		t.Error("entry for key still present after the last holder unlocked")
-	}
-}

internal/imgcache/eviction.go

@@ -3,7 +3,6 @@ package imgcache
 import (
 	"context"
 	"database/sql"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io/fs"
@@ -52,12 +51,9 @@ const sqliteTimestampLayout = "2006-01-02 15:04:05"
 // writes (".tmp-*" patterns in the storage layer).
 const tempFilePrefix = ".tmp-"
 
-// variantMetaSuffix is the sidecar suffix VariantStorage writes next
+// fallbackContentType is the content type reconciliation records for a
-// to each variant file.
+// variant file it adopts whose first bytes are not those of an output
-const variantMetaSuffix = ".meta"
+// format. Such a file is never served.
-
-// 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
@@ -84,11 +80,11 @@ func (c *Cache) UsageBytes(ctx context.Context) (int64, error) {
 
 // evictionCandidate is one LRU eviction victim candidate: either a
 // processed variant (isVariant true, identified by cacheKey) or a
-// source content blob (identified by contentHash).
+// source content blob (identified by sourceKey).
 type evictionCandidate struct {
 	isVariant      bool
 	cacheKey       imagerequest.VariantKey
-	contentHash    ContentHash
+	sourceKey      imagerequest.SourceKey
 	sizeBytes      int64
 	lastAccessedAt string
 }
@@ -160,7 +156,7 @@ func (c *Cache) evictBatch(ctx context.Context, excessBytes int64) (int64, error
 
 			c.log.Warn("failed to evict cache entry",
 				"cache_key", candidate.cacheKey,
-				"content_hash", candidate.contentHash,
+				"source_key", candidate.sourceKey,
 				"error", err,
 			)
 
@@ -179,7 +175,7 @@ func (c *Cache) evictCandidate(ctx context.Context, candidate evictionCandidate)
 		return c.evictVariant(ctx, candidate.cacheKey)
 	}
 
-	return c.evictSourceBlob(ctx, candidate.contentHash)
+	return c.evictSourceBlob(ctx, candidate.sourceKey)
 }
 
 // evictionCandidates returns up to evictionBatchSize variants and
@@ -259,9 +255,9 @@ func (c *Cache) variantCandidates(ctx context.Context) ([]evictionCandidate, err
 // SQLite reads the rows in order from that column's index instead of
 // sorting the whole table.
 const sourceCandidatesQuery = `
-	SELECT content_hash, size_bytes, last_accessed_at
+	SELECT source_key, size_bytes, last_accessed_at
 	FROM source_content
-	ORDER BY last_accessed_at ASC, content_hash ASC
+	ORDER BY last_accessed_at ASC, source_key ASC
 	LIMIT ?`
 
 // sourceCandidates returns the least recently used source blobs.
@@ -278,14 +274,14 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro
 	for rows.Next() {
 		var candidate evictionCandidate
 
-		var hash string
+		var key string
 
-		err := rows.Scan(&hash, &candidate.sizeBytes, &candidate.lastAccessedAt)
+		err := rows.Scan(&key, &candidate.sizeBytes, &candidate.lastAccessedAt)
 		if err != nil {
 			return nil, fmt.Errorf("failed to scan source candidate: %w", err)
 		}
 
-		candidate.contentHash = ContentHash(hash)
+		candidate.sourceKey = imagerequest.SourceKey(key)
 		candidates = append(candidates, candidate)
 	}
 
@@ -298,10 +294,7 @@ 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, 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 imagerequest.VariantKey,
 ) error {
@@ -311,8 +304,6 @@ func (c *Cache) evictVariant(
 		return fmt.Errorf("failed to delete variant accounting row: %w", err)
 	}
 
-	c.metaCache.Remove(cacheKey)
-
 	err = c.variants.Delete(cacheKey)
 	if err != nil {
 		return err
@@ -321,117 +312,18 @@ func (c *Cache) evictVariant(
 	return nil
 }
 
-// sourceReference identifies one source_metadata row's JSON sidecar.
+// evictSourceBlob removes one source: its row first, then its file, as
-type sourceReference struct {
+// evictVariant does for a variant. It holds no lock: a store of the same
-	host     string
+// source in between can leave the source's row with no file, which
-	pathHash PathHash
+// LookupSource treats as a miss.
-}
+func (c *Cache) evictSourceBlob(ctx context.Context, key imagerequest.SourceKey) error {
-
+	_, err := c.db.ExecContext(ctx,
-// evictSourceBlob removes one source content blob. All source_metadata
+		`DELETE FROM source_content WHERE source_key = ?`, string(key))
-// rows referencing the blob are deleted together with its
-// source_content row in a single transaction BEFORE the file is
-// unlinked: a blob referenced by multiple source paths is only ever
-// removed together with all of its references, and database rows never
-// point at deleted files. The JSON metadata sidecars for the removed
-// rows are deleted afterwards.
-//
-// The whole operation holds the content hash's lock (the same one
-// StoreSource holds for its full store), so a concurrent store of
-// identical content bytes can never observe the file gone but a row
-// still present, or insert a fresh row between this transaction's
-// commit and the file unlink below: it either runs entirely before
-// this eviction starts, or is blocked until this eviction (row
-// deletion and unlink together) has fully completed.
-func (c *Cache) evictSourceBlob(ctx context.Context, contentHash ContentHash) error {
-	unlock := c.contentLocks.Lock(string(contentHash))
-	defer unlock()
-
-	references, err := c.sourceReferences(ctx, contentHash)
-	if err != nil {
-		return err
-	}
-
-	tx, err := c.db.BeginTx(ctx, nil)
-	if err != nil {
-		return fmt.Errorf("failed to begin eviction transaction: %w", err)
-	}
-
-	defer func() { _ = tx.Rollback() }()
-
-	_, err = tx.ExecContext(ctx,
-		`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash))
-	if err != nil {
-		return fmt.Errorf("failed to delete source metadata rows: %w", err)
-	}
-
-	_, err = tx.ExecContext(ctx,
-		`DELETE FROM source_content WHERE content_hash = ?`, string(contentHash))
 	if err != nil {
 		return fmt.Errorf("failed to delete source content row: %w", err)
 	}
 
-	err = tx.Commit()
+	return c.srcContent.Delete(key)
-	if err != nil {
-		return fmt.Errorf("failed to commit eviction transaction: %w", err)
-	}
-
-	if c.evictSourceBlobTestHook != nil {
-		c.evictSourceBlobTestHook(contentHash)
-	}
-
-	// Only after the rows are gone may the files be removed.
-	for _, reference := range references {
-		err := c.srcMetadata.Delete(reference.host, reference.pathHash)
-		if err != nil {
-			c.log.Warn("failed to delete metadata sidecar",
-				"host", reference.host, "path_hash", reference.pathHash, "error", err)
-		}
-	}
-
-	err = c.srcContent.Delete(contentHash)
-	if err != nil {
-		return err
-	}
-
-	return nil
-}
-
-// sourceReferences lists the metadata sidecar locations of every
-// source_metadata row referencing the given blob.
-func (c *Cache) sourceReferences(
-	ctx context.Context, contentHash ContentHash,
-) ([]sourceReference, error) {
-	rows, err := c.db.QueryContext(ctx, `
-		SELECT source_host, path_hash FROM source_metadata WHERE content_hash = ?
-	`, string(contentHash))
-	if err != nil {
-		return nil, fmt.Errorf("failed to query source references: %w", err)
-	}
-
-	defer func() { _ = rows.Close() }()
-
-	var references []sourceReference
-
-	for rows.Next() {
-		var reference sourceReference
-
-		var pathHash string
-
-		err := rows.Scan(&reference.host, &pathHash)
-		if err != nil {
-			return nil, fmt.Errorf("failed to scan source reference: %w", err)
-		}
-
-		reference.pathHash = PathHash(pathHash)
-		references = append(references, reference)
-	}
-
-	err = rows.Err()
-	if err != nil {
-		return nil, fmt.Errorf("source reference iteration failed: %w", err)
-	}
-
-	return references, nil
 }
 
 // notifyWritePressure wakes the background evictor after a store, so
@@ -586,12 +478,12 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
 // as long after the last pass ended as that pass took, off the request
 // hot path: it adopts variant files that predate the accounting table
 // (or whose accounting insert failed, e.g. StoreVariant's best-effort
-// insert under transient DB contention), drops accounting rows whose
+// insert under transient DB contention), drops variant accounting rows
-// files are missing, removes source blob files the database does not
+// whose files are missing, removes source blob files the database does
-// know (and rows whose files are gone), removes source and variant files
+// not know, removes source and variant files that are empty or not the
-// that are empty or not the size their rows record, sweeps stale temp
+// size their rows record, sweeps stale temp files left behind by crashed
-// files left behind by crashed writes, and last checks the total cache
+// writes, and last checks the total cache usage against the tables. It
-// usage against the tables. It reads the tables a page at a time.
+// reads the tables a page at a time.
 // Running it periodically, not just once, bounds how long such drift can
 // accumulate unaccounted for on a long-running process to about one
 // eviction interval plus ten times the last pass's time.
@@ -617,20 +509,19 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error {
 		return err
 	}
 
-	err = c.reconcileSourceRows(ctx)
-	if err != nil {
-		return err
-	}
-
 	return c.reconcileUsageTotal(ctx)
 }
 
-// reconcileVariantFiles walks the variant storage directory, checking
+// reconcileFiles walks baseDir, one of the cache's storage directories,
-// each variant file against its accounting row (reconcileVariantFile)
+// sweeping stale temp files and passing each other file, with its name, to
-// and sweeping stale temp files.
+// reconcileFile. Once ctx is cancelled it stops at the next file and
-func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
+// returns ctx's error.
+func (c *Cache) reconcileFiles(
+	ctx context.Context, baseDir string,
+	reconcileFile func(path string, entry fs.DirEntry, name string) error,
+) error {
 	return filepath.WalkDir(
-		c.variants.baseDir,
+		baseDir,
 		func(path string, entry fs.DirEntry, err error) error {
 			if ctx.Err() != nil {
 				return ctx.Err()
@@ -648,15 +539,21 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
 				return nil
 			}
 
-			if strings.HasSuffix(name, variantMetaSuffix) {
+			return reconcileFile(path, entry, name)
-				return nil
-			}
-
-			return c.reconcileVariantFile(ctx, path, entry, imagerequest.VariantKey(name))
 		},
 	)
 }
 
+// reconcileVariantFiles walks the variant storage directory, checking
+// each variant file against its accounting row (reconcileVariantFile)
+// and sweeping stale temp files.
+func (c *Cache) reconcileVariantFiles(ctx context.Context) error {
+	return c.reconcileFiles(ctx, c.variants.baseDir,
+		func(path string, entry fs.DirEntry, name string) error {
+			return c.reconcileVariantFile(ctx, path, entry, imagerequest.VariantKey(name))
+		})
+}
+
 // reconcileVariantFile checks one variant file against its accounting
 // row. A file without a row is adopted into the accounting, unless it is
 // empty. A file that is empty, or not the size its row records, as a
@@ -678,7 +575,7 @@ func (c *Cache) reconcileVariantFile(
 
 	switch {
 	case errors.Is(err, sql.ErrNoRows) && info.Size() > 0:
-		return c.adoptVariantFile(ctx, path, info, cacheKey)
+		return c.adoptVariantFile(ctx, info, cacheKey)
 	case err != nil && !errors.Is(err, sql.ErrNoRows):
 		return fmt.Errorf("failed to check variant accounting row: %w", err)
 	case info.Size() > 0 && info.Size() == recordedSize:
@@ -693,14 +590,21 @@ func (c *Cache) reconcileVariantFile(
 }
 
 // adoptVariantFile inserts an accounting row for a variant file that
-// has none, using the file's size and modification time.
+// has none, using the file's size and modification time, and the content
+// type its first bytes show, as a hit reads it.
 func (c *Cache) adoptVariantFile(
-	ctx context.Context, path string, info fs.FileInfo, cacheKey imagerequest.VariantKey,
+	ctx context.Context, info fs.FileInfo, cacheKey imagerequest.VariantKey,
 ) error {
 	modTime := info.ModTime().UTC().Format(sqliteTimestampLayout)
-	contentType := c.variantContentTypeFromSidecar(path)
+	contentType := fallbackContentType
 
-	_, err := c.db.ExecContext(ctx, `
+	content, _, detectedType, err := c.variants.LoadWithContentType(cacheKey)
+	if err == nil {
+		_ = content.Close()
+		contentType = detectedType
+	}
+
+	_, err = c.db.ExecContext(ctx, `
 		INSERT INTO variant_content
 			(cache_key, size_bytes, content_type, created_at, last_accessed_at)
 		VALUES (?, ?, ?, ?, ?)
@@ -715,23 +619,6 @@ func (c *Cache) adoptVariantFile(
 	return nil
 }
 
-// variantContentTypeFromSidecar reads the content type from a variant
-// .meta sidecar, falling back to application/octet-stream.
-func (c *Cache) variantContentTypeFromSidecar(variantPath string) string {
-	//nolint:gosec // path from cache walk
-	metaData, err := os.ReadFile(variantPath + variantMetaSuffix)
-	if err != nil {
-		return fallbackContentType
-	}
-
-	var meta VariantMeta
-	if json.Unmarshal(metaData, &meta) != nil || meta.ContentType == "" {
-		return fallbackContentType
-	}
-
-	return meta.ContentType
-}
-
 // reconcileVariantRows drops accounting rows whose variant files are
 // missing, so the database never references deleted content.
 func (c *Cache) reconcileVariantRows(ctx context.Context) error {
@@ -823,38 +710,20 @@ func queryStringColumn[T ~string](
 // each blob file against its source_content row (reconcileSourceFile)
 // and sweeping stale temp files.
 func (c *Cache) reconcileSourceFiles(ctx context.Context) error {
-	return filepath.WalkDir(
+	return c.reconcileFiles(ctx, c.srcContent.baseDir,
-		c.srcContent.baseDir,
+		func(path string, entry fs.DirEntry, name string) error {
-		func(path string, entry fs.DirEntry, err error) error {
+			return c.reconcileSourceFile(ctx, path, entry, imagerequest.SourceKey(name))
-			if ctx.Err() != nil {
+		})
-				return ctx.Err()
-			}
-
-			if err != nil || entry.IsDir() {
-				return err
-			}
-
-			name := entry.Name()
-
-			if strings.HasPrefix(name, tempFilePrefix) {
-				c.sweepStaleTempFile(path, entry)
-
-				return nil
-			}
-
-			return c.reconcileSourceFile(ctx, path, entry, ContentHash(name))
-		},
-	)
 }
 
 // reconcileSourceFile checks one source blob file against its
-// source_content row. A file without a row is removed: it is
+// source_content row. A file without a row is removed, as no lookup
-// unreachable, as source lookups always go through source_metadata. A
+// serves it and no eviction removes it. A file that is empty, or not the
-// file that is empty, or not the size its row records, as a crash or a
+// size its row records, as a crash or a restored snapshot can leave it,
-// restored snapshot can leave it, is removed with its rows and a warning
+// is removed with its row and a warning naming it, so the source is
-// naming it, so the source is fetched again.
+// fetched again.
 func (c *Cache) reconcileSourceFile(
-	ctx context.Context, path string, entry fs.DirEntry, contentHash ContentHash,
+	ctx context.Context, path string, entry fs.DirEntry, key imagerequest.SourceKey,
 ) error {
 	info, err := entry.Info()
 	if err != nil {
@@ -864,13 +733,13 @@ func (c *Cache) reconcileSourceFile(
 	var recordedSize int64
 
 	err = c.db.QueryRowContext(ctx,
-		`SELECT size_bytes FROM source_content WHERE content_hash = ?`,
+		`SELECT size_bytes FROM source_content WHERE source_key = ?`,
-		string(contentHash),
+		string(key),
 	).Scan(&recordedSize)
 
 	switch {
 	case errors.Is(err, sql.ErrNoRows):
-		return c.removeUntrackedSourceFile(ctx, path, contentHash)
+		return c.removeUntrackedSourceFile(path, key)
 	case err != nil:
 		return fmt.Errorf("failed to check source content row: %w", err)
 	case info.Size() > 0 && info.Size() == recordedSize:
@@ -881,96 +750,34 @@ func (c *Cache) reconcileSourceFile(
 	c.log.Warn("removing damaged cached source file", "path", path,
 		"file_bytes", info.Size(), "recorded_bytes", recordedSize)
 
-	return c.evictSourceBlob(ctx, contentHash)
+	return c.evictSourceBlob(ctx, key)
 }
 
 // removeUntrackedSourceFile deletes a source blob file that has no
-// source_content row. Any source_metadata rows referencing the hash
+// source_content row.
-// are removed first so no row ever points at a deleted file.
 func (c *Cache) removeUntrackedSourceFile(
-	ctx context.Context, path string, contentHash ContentHash,
+	path string, key imagerequest.SourceKey,
 ) error {
-	_, err := c.db.ExecContext(ctx,
+	err := os.Remove(path)
-		`DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash))
-	if err != nil {
-		return fmt.Errorf("failed to delete metadata rows for untracked blob: %w", err)
-	}
-
-	err = os.Remove(path)
 	if err != nil && !os.IsNotExist(err) {
 		return fmt.Errorf("failed to remove untracked source file: %w", err)
 	}
 
-	c.log.Info("removed untracked source content file", "content_hash", contentHash)
+	c.log.Info("removed untracked source content file", "source_key", key)
 
 	return nil
 }
 
-// reconcileSourceRows removes source_content rows (and their metadata
-// references and sidecars) whose blob files are missing on disk.
-func (c *Cache) reconcileSourceRows(ctx context.Context) error {
-	var after ContentHash
-
-	for {
-		hashes, err := c.sourceContentHashesAfter(ctx, after)
-		if err != nil {
-			return err
-		}
-
-		if c.reconciliationReadTestHook != nil {
-			c.reconciliationReadTestHook(len(hashes))
-		}
-
-		if len(hashes) == 0 {
-			return nil
-		}
-
-		for _, hash := range hashes {
-			if ctx.Err() != nil {
-				return ctx.Err()
-			}
-
-			if c.srcContent.Exists(hash) {
-				continue
-			}
-
-			// The blob file is already gone; evictSourceBlob removes the
-			// rows and sidecars and tolerates the missing file.
-			err := c.evictSourceBlob(ctx, hash)
-			if err != nil {
-				return err
-			}
-
-			c.log.Info("dropped rows for missing source content file", "content_hash", hash)
-		}
-
-		after = hashes[len(hashes)-1]
-	}
-}
-
-// sourceContentHashesAfter returns, in order, up to
-// c.reconciliationPageSize tracked source content hashes that sort after
-// the given one.
-func (c *Cache) sourceContentHashesAfter(
-	ctx context.Context, after ContentHash,
-) ([]ContentHash, error) {
-	return queryStringColumn[ContentHash](ctx, c.db, `
-		SELECT content_hash FROM source_content
-		WHERE content_hash > ? ORDER BY content_hash LIMIT ?
-	`, "source content hashes", "content hash", string(after),
-		c.reconciliationPageSize)
-}
-
 // Each of these queries sums size_bytes over the next page of rows of
 // one content table, the rows that sort after a key, and returns the
 // page's last key, the sum and the number of rows in the page. Past the
 // last row the page has no rows and the key is NULL.
 const (
 	sourceSizePageQuery = `
-		SELECT MAX(content_hash), COALESCE(SUM(size_bytes), 0), COUNT(*)
+		SELECT MAX(source_key), COALESCE(SUM(size_bytes), 0), COUNT(*)
 		FROM (
-			SELECT content_hash, size_bytes FROM source_content
+			SELECT source_key, size_bytes FROM source_content
-			WHERE content_hash > ? ORDER BY content_hash LIMIT ?
+			WHERE source_key > ? ORDER BY source_key LIMIT ?
 		)`
 	variantSizePageQuery = `
 		SELECT MAX(cache_key), COALESCE(SUM(size_bytes), 0), COUNT(*)

internal/imgcache/eviction_internal_test.go

@@ -10,10 +10,11 @@ import (
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 
-	_ "modernc.org/sqlite"
+	"modernc.org/sqlite"
 	"sneak.berlin/go/pixa/internal/database"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -83,10 +84,10 @@ func newEvictionTestCache(t *testing.T, maxBytes int64) (*Cache, string) {
 }
 
 // storeEvictionTestSource stores content as a fetched source for
-// host/path and returns the resulting content hash.
+// host/path and returns the resulting source key.
 func storeEvictionTestSource(
 	t *testing.T, cache *Cache, host, path string, content []byte,
-) ContentHash {
+) imagerequest.SourceKey {
 	t.Helper()
 
 	req := &imagerequest.Request{
@@ -110,14 +111,14 @@ func storeEvictionTestSource(
 		},
 	}
 
-	hash, err := cache.StoreSource(
+	key, err := cache.StoreSource(
 		context.Background(), req, bytes.NewReader(content), result,
 	)
 	if err != nil {
 		t.Fatalf("StoreSource(%s%s) failed: %v", host, path, err)
 	}
 
-	return hash
+	return key
 }
 
 // storeEvictionTestVariant stores content as a processed variant under
@@ -162,13 +163,13 @@ func setVariantLastAccessed(
 // setSourceLastAccessed backdates the last access time of a tracked
 // source content blob.
 func setSourceLastAccessed(
-	t *testing.T, cache *Cache, hash ContentHash, when time.Time,
+	t *testing.T, cache *Cache, key imagerequest.SourceKey, when time.Time,
 ) {
 	t.Helper()
 
 	res, err := cache.db.ExecContext(t.Context(),
-		`UPDATE source_content SET last_accessed_at = ? WHERE content_hash = ?`,
+		`UPDATE source_content SET last_accessed_at = ? WHERE source_key = ?`,
-		when.UTC().Format(sqliteTimestampFormat), string(hash),
+		when.UTC().Format(sqliteTimestampFormat), string(key),
 	)
 	if err != nil {
 		t.Fatalf("failed to set source last_accessed_at: %v", err)
@@ -180,7 +181,7 @@ func setSourceLastAccessed(
 	}
 
 	if affected != 1 {
-		t.Fatalf("source %s has no accounting row (affected=%d)", hash, affected)
+		t.Fatalf("source %s has no accounting row (affected=%d)", key, affected)
 	}
 }
 
@@ -205,32 +206,31 @@ func assertNoDanglingReferences(t *testing.T, cache *Cache) {
 	t.Helper()
 
 	rows, err := cache.db.QueryContext(t.Context(),
-		`SELECT content_hash FROM source_metadata
+		`SELECT source_key FROM source_content`,
-		 WHERE content_hash IS NOT NULL AND content_hash != ''`,
 	)
 	if err != nil {
-		t.Fatalf("failed to query source_metadata: %v", err)
+		t.Fatalf("failed to query source_content: %v", err)
 	}
 
 	defer func() { _ = rows.Close() }()
 
 	for rows.Next() {
-		var hash string
+		var key string
 
-		err := rows.Scan(&hash)
+		err := rows.Scan(&key)
 		if err != nil {
-			t.Fatalf("failed to scan content_hash: %v", err)
+			t.Fatalf("failed to scan source_key: %v", err)
 		}
 
-		if !cache.srcContent.Exists(ContentHash(hash)) {
+		if !cache.srcContent.Exists(imagerequest.SourceKey(key)) {
-			t.Errorf("source_metadata references content %s but the file is missing",
+			t.Errorf("source_content references content %s but the file is missing",
-				hash)
+				key)
 		}
 	}
 
 	err = rows.Err()
 	if err != nil {
-		t.Fatalf("source_metadata iteration failed: %v", err)
+		t.Fatalf("source_content iteration failed: %v", err)
 	}
 
 	variantRows, err := cache.db.QueryContext(t.Context(),
@@ -313,33 +313,6 @@ func TestUsageBytesAccountsSourceAndVariantBytes(t *testing.T) {
 	}
 }
 
-func TestUsageBytesCountsMultiReferencedBlobOnce(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<30)
-
-	content := bytes.Repeat([]byte{0xCC}, 1200)
-
-	hashOne := storeEvictionTestSource(t, cache,
-		"src.example.com", "/one.jpg", content)
-	hashTwo := storeEvictionTestSource(t, cache,
-		"src.example.com", "/two.jpg", content)
-
-	if hashOne != hashTwo {
-		t.Fatalf("identical content produced different hashes: %s vs %s",
-			hashOne, hashTwo)
-	}
-
-	usage, err := cache.UsageBytes(context.Background())
-	if err != nil {
-		t.Fatalf("UsageBytes failed: %v", err)
-	}
-
-	if usage != 1200 {
-		t.Errorf("UsageBytes = %d, want 1200 (deduplicated blob counted once)", usage)
-	}
-}
-
 func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) {
 	t.Parallel()
 
@@ -393,102 +366,15 @@ func TestEvictToLimitEvictsLeastRecentlyUsedFirst(t *testing.T) {
 	assertNoDanglingReferences(t, cache)
 }
 
-func TestEvictionRemovesMultiReferencedBlobTogetherWithAllReferences(t *testing.T) {
-	t.Parallel()
-
-	const limit = 1000
-
-	cache, _ := newEvictionTestCache(t, limit)
-
-	now := time.Now()
-
-	// One 800-byte blob referenced by two source paths.
-	sharedContent := bytes.Repeat([]byte{0xDD}, 800)
-	sharedHash := storeEvictionTestSource(t, cache,
-		"src.example.com", "/a.jpg", sharedContent)
-
-	h := storeEvictionTestSource(t, cache,
-		"src.example.com", "/b.jpg", sharedContent)
-	if h != sharedHash {
-		t.Fatalf("identical content produced different hashes: %s vs %s",
-			h, sharedHash)
-	}
-
-	// A newer 600-byte blob referenced by one source path.
-	recentHash := storeEvictionTestSource(t, cache, "src.example.com", "/c.jpg",
-		bytes.Repeat([]byte{0xEE}, 600))
-
-	setSourceLastAccessed(t, cache, sharedHash, now.Add(-2*time.Hour))
-	setSourceLastAccessed(t, cache, recentHash, now.Add(-time.Minute))
-
-	err := cache.EvictToLimit(context.Background())
-	if err != nil {
-		t.Fatalf("EvictToLimit failed: %v", err)
-	}
-
-	usage, err := cache.UsageBytes(context.Background())
-	if err != nil {
-		t.Fatalf("UsageBytes failed: %v", err)
-	}
-
-	if usage > limit {
-		t.Errorf("usage after eviction = %d, want <= %d", usage, limit)
-	}
-
-	// The multi-referenced blob must be gone from disk, from
-	// source_content, and from BOTH source_metadata rows: references
-	// are removed together with the blob, never left dangling.
-	if cache.srcContent.Exists(sharedHash) {
-		t.Errorf("evicted blob %s still exists on disk", sharedHash)
-	}
-
-	if n := countRows(t, cache,
-		`SELECT COUNT(*) FROM source_content WHERE content_hash = ?`, string(sharedHash),
-	); n != 0 {
-		t.Errorf("evicted blob %s still has %d source_content rows", sharedHash, n)
-	}
-
-	if n := countRows(t, cache,
-		`SELECT COUNT(*) FROM source_metadata WHERE content_hash = ?`, string(sharedHash),
-	); n != 0 {
-		t.Errorf("evicted blob %s still has %d source_metadata references", sharedHash, n)
-	}
-
-	// The JSON metadata sidecars for both referencing paths must be
-	// removed along with the rows.
-	for _, path := range []string{"/a.jpg", "/b.jpg"} {
-		pathHash := HashPath(path + "?")
-		if cache.srcMetadata.Exists("src.example.com", pathHash) {
-			t.Errorf("metadata sidecar for %s must be removed with its row", path)
-		}
-	}
-
-	// The more recently used blob survives fully intact.
-	if !cache.srcContent.Exists(recentHash) {
-		t.Errorf("recently used blob %s must survive eviction", recentHash)
-	}
-
-	if n := countRows(t, cache,
-		`SELECT COUNT(*) FROM source_metadata WHERE content_hash = ?`, string(recentHash),
-	); n != 1 {
-		t.Errorf("recently used blob %s has %d source_metadata rows, want 1",
-			recentHash, n)
-	}
-
-	assertNoDanglingReferences(t, cache)
-}
-
 func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
 	t.Parallel()
 
 	cache, _ := newEvictionTestCache(t, 1<<30)
 
 	content := bytes.Repeat([]byte{0xDF}, 800)
-	hash := storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg", content)
+	keys := []imagerequest.SourceKey{
-
+		storeEvictionTestSource(t, cache, "src.example.com", "/a.jpg", content),
-	h := storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", content)
+		storeEvictionTestSource(t, cache, "src.example.com", "/b.jpg", content),
-	if h != hash {
-		t.Fatalf("identical content produced different hashes: %s vs %s", h, hash)
 	}
 
 	storeEvictionTestVariant(t, cache, testVariantKeyOne,
@@ -499,14 +385,16 @@ func TestEvictionKeepsEverythingWhenUnderLimit(t *testing.T) {
 		t.Fatalf("EvictToLimit failed: %v", err)
 	}
 
-	if !cache.srcContent.Exists(hash) {
+	for _, key := range keys {
-		t.Errorf("blob %s must not be evicted while usage is under the limit", hash)
+		if !cache.srcContent.Exists(key) {
-	}
+			t.Errorf("source %s must not be evicted while usage is under the limit", key)
+		}
 
-	if n := countRows(t, cache,
+		if n := countRows(t, cache,
-		`SELECT COUNT(*) FROM source_metadata WHERE content_hash = ?`, string(hash),
+			`SELECT COUNT(*) FROM source_content WHERE source_key = ?`, string(key),
-	); n != 2 {
+		); n != 1 {
-		t.Errorf("blob %s has %d source_metadata rows, want 2", hash, n)
+			t.Errorf("source %s has %d source_content rows, want 1", key, n)
+		}
 	}
 
 	if !cache.variants.Exists(testVariantKeyOne) {
@@ -562,13 +450,13 @@ func assertDisabledCacheWritesAreNoOps(
 		Headers:       map[string][]string{},
 	}
 
-	hash, err := cache.StoreSource(ctx, req, bytes.NewReader([]byte("data")), result)
+	key, err := cache.StoreSource(ctx, req, bytes.NewReader([]byte("data")), result)
 	if err != nil {
 		t.Fatalf("StoreSource on disabled cache must be a no-op, got error: %v", err)
 	}
 
-	if hash != "" {
+	if key != "" {
-		t.Errorf("StoreSource on disabled cache returned hash %q, want empty", hash)
+		t.Errorf("StoreSource on disabled cache returned key %q, want empty", key)
 	}
 }
 
@@ -581,23 +469,14 @@ func assertDisabledCacheReadsAlwaysMiss(
 
 	ctx := t.Context()
 
-	lookup, err := cache.Lookup(ctx, req)
+	key, srcType, err := cache.LookupSource(ctx, req)
-	if err != nil {
-		t.Fatalf("Lookup on disabled cache failed: %v", err)
-	}
-
-	if lookup.Hit {
-		t.Error("Lookup on disabled cache must always miss")
-	}
-
-	srcHash, srcType, err := cache.LookupSource(ctx, req)
 	if err != nil {
 		t.Fatalf("LookupSource on disabled cache failed: %v", err)
 	}
 
-	if srcHash != "" || srcType != "" {
+	if key != "" || srcType != "" {
 		t.Errorf("LookupSource on disabled cache = (%q, %q), want empty",
-			srcHash, srcType)
+			key, srcType)
 	}
 }
 
@@ -618,10 +497,6 @@ func assertDisabledCacheTracksNothing(t *testing.T, cache *Cache) {
 	if n := countRows(t, cache, `SELECT COUNT(*) FROM source_content`); n != 0 {
 		t.Errorf("disabled cache wrote %d source_content rows, want 0", n)
 	}
-
-	if n := countRows(t, cache, `SELECT COUNT(*) FROM source_metadata`); n != 0 {
-		t.Errorf("disabled cache wrote %d source_metadata rows, want 0", n)
-	}
 }
 
 // assertDisabledCacheWroteNothingToDisk verifies that a disabled cache
@@ -732,7 +607,7 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 	for i, key := range keys {
 		content := bytes.Repeat([]byte{fills[i]}, 1000)
 
-		_, err = cache.variants.Store(key, bytes.NewReader(content), "image/webp")
+		_, err = cache.variants.Store(key, bytes.NewReader(content))
 		if err != nil {
 			t.Fatalf("failed to store variant file: %v", err)
 		}
@@ -784,7 +659,7 @@ func TestStartEvictionReconcilesAccountingWithDisk(t *testing.T) {
 	untracked := bytes.Repeat([]byte{0x31}, 1000)
 
 	_, err := cache.variants.Store(
-		testVariantKeyOne, bytes.NewReader(untracked), "image/webp",
+		testVariantKeyOne, bytes.NewReader(untracked),
 	)
 	if err != nil {
 		t.Fatalf("failed to store untracked variant file: %v", err)
@@ -886,7 +761,7 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
 	untracked := bytes.Repeat([]byte{0x41}, 900)
 
 	_, err = cache.variants.Store(
-		"aabbccdd0099", bytes.NewReader(untracked), "image/webp",
+		"aabbccdd0099", bytes.NewReader(untracked),
 	)
 	if err != nil {
 		t.Fatalf("failed to store untracked variant file: %v", err)
@@ -1064,9 +939,15 @@ func TestStopEvictionInterruptsPassInProgress(t *testing.T) {
 }
 
 // TestEvictToLimitStopsAtNextCandidateOnceCancelled cancels the context
-// while the oldest of three source blobs is being evicted, and checks that
+// once the oldest of three sources is evicted, while the next one waits
-// EvictToLimit then returns context.Canceled without evicting the other
+// for the database, and checks that EvictToLimit then returns
-// two or logging a warning for either of them.
+// context.Canceled, with that source gone and the other two kept, and
+// logs no warning. The test database has one connection. When the
+// deletion of the oldest source's row commits, a commit hook holds the
+// deletion until the test waits for the connection, so the test takes it
+// as soon as the deletion is done. The eviction of the next source then
+// waits for the connection, and the test cancels the context while it
+// waits.
 func TestEvictToLimitStopsAtNextCandidateOnceCancelled(t *testing.T) {
 	t.Parallel()
 
@@ -1076,7 +957,7 @@ func TestEvictToLimitStopsAtNextCandidateOnceCancelled(t *testing.T) {
 
 	cache.log = slog.New(slog.NewJSONHandler(&logBuf, nil))
 
-	hashes := []ContentHash{
+	keys := []imagerequest.SourceKey{
 		storeEvictionTestSource(t, cache, "cancel.example.com", "/a.jpg",
 			bytes.Repeat([]byte{0x61}, 1000)),
 		storeEvictionTestSource(t, cache, "cancel.example.com", "/b.jpg",
@@ -1087,69 +968,155 @@ func TestEvictToLimitStopsAtNextCandidateOnceCancelled(t *testing.T) {
 
 	base := time.Now().Add(-time.Hour)
 
-	for i, hash := range hashes {
+	for i, key := range keys {
-		setSourceLastAccessed(t, cache, hash, base.Add(time.Duration(i)*time.Minute))
+		setSourceLastAccessed(t, cache, key, base.Add(time.Duration(i)*time.Minute))
 	}
 
+	waitsBefore := cache.db.Stats().WaitCount
+	oldestRowDeleted := make(chan struct{})
+
+	var oldestRowDeletedOnce sync.Once
+
+	setCommitHook(t, cache, func() int32 {
+		oldestRowDeletedOnce.Do(func() {
+			close(oldestRowDeleted)
+
+			deadline := time.Now().Add(5 * time.Second)
+
+			for cache.db.Stats().WaitCount == waitsBefore && time.Now().Before(deadline) {
+				time.Sleep(time.Millisecond)
+			}
+		})
+
+		return 0
+	})
+
 	ctx, cancel := context.WithCancel(t.Context())
 	defer cancel()
 
-	cache.evictSourceBlobTestHook = func(ContentHash) { cancel() }
+	evicted := make(chan error, 1)
 
-	err := cache.EvictToLimit(ctx)
+	go func() { evicted <- cache.EvictToLimit(ctx) }()
+
+	select {
+	case <-oldestRowDeleted:
+	case err := <-evicted:
+		t.Fatalf("EvictToLimit() returned %v without deleting a row", err)
+	case <-time.After(5 * time.Second):
+		t.Fatal("EvictToLimit never deleted a row")
+	}
+
+	releaseDatabase := holdDatabase(t, cache)
+	defer releaseDatabase()
+
+	waitForConnectionWaits(t, cache.db, waitsBefore+2)
+	cancel()
+
+	err := <-evicted
 	t.Logf("EvictToLimit() error = %v", err)
 	t.Logf("log output: %s", logBuf.String())
 
+	// The checks below read the database, so the connection goes back.
+	releaseDatabase()
+
+	setCommitHook(t, cache, nil)
+
 	if !errors.Is(err, context.Canceled) {
 		t.Errorf("EvictToLimit() error = %v, want context.Canceled", err)
 	}
 
-	if cache.srcContent.Exists(hashes[0]) {
+	assertOnlyOldestSourceEvicted(t, cache, keys)
-		t.Errorf("source blob %s, evicted when the context was cancelled, "+
-			"is still on disk", hashes[0])
-	}
-
-	for _, hash := range hashes[1:] {
-		if !cache.srcContent.Exists(hash) {
-			t.Errorf("source blob %s was evicted after the context was cancelled", hash)
-		}
-	}
 
 	if strings.Contains(logBuf.String(), `"level":"WARN"`) {
 		t.Errorf("EvictToLimit logged a warning after the context was cancelled")
 	}
+}
+
+// setCommitHook makes the SQLite driver call hook each time a write on
+// the test database's one connection commits, or stops it when hook is
+// nil.
+func setCommitHook(t *testing.T, cache *Cache, hook sqlite.CommitHookFn) {
+	t.Helper()
+
+	conn, err := cache.db.Conn(t.Context())
+	if err != nil {
+		t.Fatalf("failed to take the database connection: %v", err)
+	}
+
+	defer func() { _ = conn.Close() }()
+
+	err = conn.Raw(func(driverConn any) error {
+		hooks, ok := driverConn.(sqlite.HookRegisterer)
+		if !ok {
+			t.Fatalf("the database connection is a %T, which takes no hooks",
+				driverConn)
+		}
+
+		hooks.RegisterCommitHook(hook)
+
+		return nil
+	})
+	if err != nil {
+		t.Fatalf("failed to set the commit hook: %v", err)
+	}
+}
+
+// assertOnlyOldestSourceEvicted checks that the first of keys has neither
+// its file nor its row, and that every other one has both.
+func assertOnlyOldestSourceEvicted(
+	t *testing.T, cache *Cache, keys []imagerequest.SourceKey,
+) {
+	t.Helper()
+
+	if cache.srcContent.Exists(keys[0]) {
+		t.Errorf("source %s, evicted when the context was cancelled, "+
+			"is still on disk", keys[0])
+	}
+
+	for _, key := range keys[1:] {
+		if !cache.srcContent.Exists(key) {
+			t.Errorf("source %s was evicted after the context was cancelled", key)
+		}
+
+		if n := countRows(t, cache,
+			`SELECT COUNT(*) FROM source_content WHERE source_key = ?`, string(key),
+		); n != 1 {
+			t.Errorf("source %s, kept, has %d source_content rows, want 1", key, n)
+		}
+	}
 
 	assertNoDanglingReferences(t, cache)
 }
 
-// TestStopEvictionReturnsWhenItsContextEnds pauses an eviction pass where
+// TestStopEvictionReturnsWhenItsContextEnds pauses the startup
-// cancellation cannot reach it, after a source blob's rows are deleted and
+// reconciliation pass in the hook called after each of its reads, where
-// before its file is removed, and checks that StopEviction returns its
+// cancellation cannot reach it, and checks that StopEviction returns an
-// context's error when that context ends instead of waiting for the pass.
+// error wrapping its context's error when that context ends, instead of
-// Once the pass goes on, the goroutine exits and no row points at a
+// waiting for the pass. Once the pass goes on, the goroutine exits, and a
-// missing file.
+// second StopEviction returns nil.
 func TestStopEvictionReturnsWhenItsContextEnds(t *testing.T) {
 	t.Parallel()
 
-	cache, _ := newEvictionTestCache(t, 1)
+	cache, _ := newEvictionTestCache(t, 1<<30)
 
 	paused := make(chan struct{})
 	resume := make(chan struct{})
 
-	cache.evictSourceBlobTestHook = func(ContentHash) {
+	var pauseOnce sync.Once
-		close(paused)
-		<-resume
-	}
 
-	hash := storeEvictionTestSource(t, cache, "stop.example.com", "/a.jpg",
+	cache.reconciliationReadTestHook = func(int) {
-		bytes.Repeat([]byte{0x61}, 1000))
+		pauseOnce.Do(func() {
+			close(paused)
+			<-resume
+		})
+	}
 
 	cache.StartEviction(time.Hour)
 
 	select {
 	case <-paused:
 	case <-time.After(5 * time.Second):
-		t.Fatal("the eviction pass never reached the source blob")
+		t.Fatal("the reconciliation pass never made a read")
 	}
 
 	ctx, cancel := context.WithTimeout(t.Context(), 50*time.Millisecond)
@@ -1168,12 +1135,6 @@ func TestStopEvictionReturnsWhenItsContextEnds(t *testing.T) {
 	if err != nil {
 		t.Fatalf("second StopEviction() error = %v, want nil", err)
 	}
-
-	assertNoDanglingReferences(t, cache)
-
-	if cache.srcContent.Exists(hash) {
-		t.Errorf("source blob %s is still on disk after its rows were deleted", hash)
-	}
 }
 
 // TestReconciliationWalksStopOnceCancelled checks that both directory
@@ -1245,131 +1206,3 @@ func TestReconciliationPassLogsNoWarningOnceCancelled(t *testing.T) {
 		t.Errorf("runReconciliationPass logged a warning with a cancelled context")
 	}
 }
-
-// TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent exercises
-// the exact TOCTOU window between evictSourceBlob's row-deletion
-// transaction commit and its content file unlink: a concurrent
-// StoreSource for a different source path whose content hashes to the
-// same value (real SHA-256 dedup, not a contrived case) must not be
-// able to insert a fresh row referencing the file while eviction is
-// mid-unlink, and must not lose its own store once eviction has fully
-// released the content hash.
-func TestEvictSourceBlobExcludesConcurrentStoreOfIdenticalContent(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<30)
-	ctx := context.Background()
-
-	content := bytes.Repeat([]byte{0x55}, 400)
-
-	hash := storeEvictionTestSource(t, cache,
-		"race.example.com", "/first.jpg", content)
-
-	proceed := make(chan struct{})
-	storeAttempted := make(chan struct{})
-
-	cache.evictSourceBlobTestHook = func(gotHash ContentHash) {
-		if gotHash != hash {
-			t.Errorf("test hook invoked for hash %s, want %s", gotHash, hash)
-		}
-
-		close(storeAttempted)
-		<-proceed
-	}
-
-	evictDone := make(chan error, 1)
-
-	go func() {
-		evictDone <- cache.evictSourceBlob(ctx, hash)
-	}()
-
-	// Wait until eviction has committed its delete transaction and is
-	// paused (inside the test hook) immediately before unlinking the
-	// content file: exactly the window the review flagged.
-	<-storeAttempted
-
-	storeDone := storeIdenticalContentConcurrently(ctx, cache, content)
-
-	// The concurrent store must not be able to complete while eviction
-	// still holds the content hash (i.e. before the file is unlinked):
-	// if it could, it would insert a row referencing a file about to be
-	// removed out from under it.
-	select {
-	case err := <-storeDone:
-		t.Fatalf("StoreSource for identical content completed (err=%v) while eviction "+
-			"still held the content hash open between commit and unlink; the store and "+
-			"the evict of identical content are not mutually exclusive", err)
-	case <-time.After(200 * time.Millisecond):
-		// Expected: the store is blocked behind eviction's exclusion.
-	}
-
-	close(proceed)
-
-	err := <-evictDone
-	if err != nil {
-		t.Fatalf("evictSourceBlob failed: %v", err)
-	}
-
-	err = <-storeDone
-	if err != nil {
-		t.Fatalf("StoreSource failed: %v", err)
-	}
-
-	assertNoDanglingReferences(t, cache)
-
-	dupHash, _, err := cache.LookupSource(ctx, &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: "race.example.com",
-			Path: "/dup.jpg",
-		},
-	})
-	if err != nil {
-		t.Fatalf("LookupSource failed: %v", err)
-	}
-
-	if dupHash == "" {
-		t.Fatal("re-stored blob was lost: the store legitimately ran after eviction " +
-			"released the content hash and must have recreated the file and row")
-	}
-
-	if !cache.srcContent.Exists(dupHash) {
-		t.Errorf("source_content/source_metadata references %s but its file is missing",
-			dupHash)
-	}
-}
-
-// storeIdenticalContentConcurrently starts a StoreSource for a second
-// source path whose body hashes to the same content hash, returning the
-// channel its error is delivered on.
-func storeIdenticalContentConcurrently(
-	ctx context.Context, cache *Cache, content []byte,
-) chan error {
-	storeDone := make(chan error, 1)
-
-	go func() {
-		req := &imagerequest.Request{
-			Source: imagerequest.Source{
-				Host: "race.example.com",
-				Path: "/dup.jpg",
-			},
-			Transform: imagerequest.Transform{
-				Format:  imagerequest.FormatJPEG,
-				Quality: 85,
-				FitMode: imagerequest.FitCover,
-			},
-		}
-		result := &httpfetcher.FetchResult{
-			StatusCode:    200,
-			ContentType:   testContentTypeJPEG,
-			ContentLength: int64(len(content)),
-			Headers: map[string][]string{
-				testHeaderContentType: {testContentTypeJPEG},
-			},
-		}
-
-		_, err := cache.StoreSource(ctx, req, bytes.NewReader(content), result)
-		storeDone <- err
-	}()
-
-	return storeDone
-}

internal/imgcache/max_concurrent_processing_internal_test.go

@@ -77,11 +77,11 @@ func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	contentHash, _, err := svc.cache.LookupSource(t.Context(),
+	sourceKey, _, err := svc.cache.LookupSource(t.Context(),
 		widthOnlyRequest(fixtures, 50))
-	if err != nil || contentHash == "" {
+	if err != nil || sourceKey == "" {
 		t.Fatalf("LookupSource() = %q, %v; want the cached source",
-			contentHash, err)
+			sourceKey, err)
 	}
 
 	release := holdProcessingSlot(t, svc.processor)
@@ -103,7 +103,7 @@ func TestService_Get_WaitsForSlotBeforeReadingCachedSource(t *testing.T) {
 	// before waiting, it would have read it by now.
 	time.Sleep(100 * time.Millisecond)
 
-	err = os.WriteFile(svc.cache.srcContent.hashToPath(contentHash),
+	err = os.WriteFile(svc.cache.srcContent.keyToPath(sourceKey),
 		generateTestJPEG(t, 100, 50, color.RGBA{0, 0, 255, 255}), 0o600)
 	if err != nil {
 		t.Fatalf("failed to rewrite the cached source: %v", err)

internal/imgcache/metacache_internal_test.go

@@ -1,361 +0,0 @@
-package imgcache
-
-import (
-	"bytes"
-	"encoding/json"
-	"errors"
-	"fmt"
-	"io"
-	"os"
-	"sync"
-	"testing"
-	"time"
-
-	"sneak.berlin/go/pixa/internal/imagerequest"
-)
-
-// webpRequest returns a request for a 100x100 WebP variant of path.
-func webpRequest(path string) *imagerequest.Request {
-	return &imagerequest.Request{
-		Source: imagerequest.Source{
-			Host: testHostCDN,
-			Path: path,
-		},
-		Transform: imagerequest.Transform{
-			Size:    imagerequest.Size{Width: 100, Height: 100},
-			Format:  imagerequest.FormatWebP,
-			Quality: 85,
-			FitMode: imagerequest.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 imagerequest.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 imagerequest.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.Request,
-) {
-	t.Helper()
-
-	lookup, err := cache.Lookup(t.Context(), request)
-	if err != nil {
-		t.Fatalf("Lookup(%s) error = %v", request.Source.Path, err)
-	}
-
-	if lookup.Hit {
-		t.Errorf("Lookup(%s) is a hit, want a miss", request.Source.Path)
-	}
-}
-
-// 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
-// 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)
-}
-
-// 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)
-}
-
-// 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
-// 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.
-func TestEvictedVariantIsNotServed(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1500)
-
-	oldRequest := webpRequest("/old.jpg")
-	newRequest := webpRequest("/new.jpg")
-	oldKey := imagerequest.CacheKey(oldRequest)
-	newKey := imagerequest.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 := imagerequest.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 := imagerequest.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()
-}

internal/imgcache/negative_cache_internal_test.go

@@ -147,13 +147,13 @@ func TestService_Get_ServesCachedVariantOfNegativeCachedURL(t *testing.T) {
 
 	_ = resp.Content.Close()
 
-	contentHash, _, err := svc.cache.LookupSource(t.Context(),
+	sourceKey, _, err := svc.cache.LookupSource(t.Context(),
 		photoVariant(fixtures, 85, imagerequest.FitCover))
-	if err != nil || contentHash == "" {
+	if err != nil || sourceKey == "" {
-		t.Fatalf("LookupSource() = %q, %v, want the cached source", contentHash, err)
+		t.Fatalf("LookupSource() = %q, %v, want the cached source", sourceKey, err)
 	}
 
-	err = svc.cache.evictSourceBlob(t.Context(), contentHash)
+	err = svc.cache.evictSourceBlob(t.Context(), sourceKey)
 	if err != nil {
 		t.Fatalf("evictSourceBlob() error = %v", err)
 	}

internal/imgcache/partial_cache_files_internal_test.go

@@ -2,8 +2,6 @@ package imgcache
 
 import (
 	"bytes"
-	"crypto/sha256"
-	"encoding/hex"
 	"io"
 	"log/slog"
 	"os"
@@ -67,12 +65,12 @@ func cachedSourcePath(
 ) string {
 	t.Helper()
 
-	contentHash, _, err := svc.cache.LookupSource(t.Context(), req)
+	sourceKey, _, err := svc.cache.LookupSource(t.Context(), req)
-	if err != nil || contentHash == "" {
+	if err != nil || sourceKey == "" {
-		t.Fatalf("LookupSource() = %q, %v; want the cached source", contentHash, err)
+		t.Fatalf("LookupSource() = %q, %v; want the cached source", sourceKey, err)
 	}
 
-	return svc.cache.srcContent.hashToPath(contentHash)
+	return svc.cache.srcContent.keyToPath(sourceKey)
 }
 
 // reconcileAsAtStartup runs the cache's reconciliation pass, as pixa does
@@ -130,6 +128,8 @@ func TestService_Get_FetchesAgainForEmptyCachedSource(t *testing.T) {
 	getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover))
 
+	waitForPendingCountsWritten(t, svc.cache)
+
 	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
 	t.Logf("upstream fetches = %d", fetches)
 
@@ -166,6 +166,8 @@ func TestService_Get_FetchesAgainForPartialCachedSource(t *testing.T) {
 	getImage(t, svc, photoVariant(fixtures, 40, imagerequest.FitCover))
 	getImage(t, svc, photoVariant(fixtures, 30, imagerequest.FitCover))
 
+	waitForPendingCountsWritten(t, svc.cache)
+
 	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
 	t.Logf("upstream fetches = %d", fetches)
 
@@ -241,8 +243,7 @@ func TestReconciliationRemovesEmptyVariantFileWithoutRow(t *testing.T) {
 
 	cache, _ := newEvictionTestCache(t, 1<<30)
 
-	_, err := cache.variants.Store(testVariantKeyOne, bytes.NewReader(nil),
+	_, err := cache.variants.Store(testVariantKeyOne, bytes.NewReader(nil))
-		testContentTypeWebP)
 	if err != nil {
 		t.Fatalf("failed to store an empty variant file: %v", err)
 	}
@@ -263,63 +264,10 @@ func TestReconciliationRemovesEmptyVariantFileWithoutRow(t *testing.T) {
 	}
 }
 
-// TestVariantStorage_StoreReplacesMetaFile stores a variant over one
+// TestContentStorage_StoreReplacesFileOfAnotherSize checks that a
-// already stored and checks that Store puts a new .meta file in place of
+// store over a file of another size at the source's path, such as a
-// the old one, as it does the image, rather than writing into the old
-// file, which a crash could leave empty. The old .meta file is given a
-// second name before the store, so that it can be told apart from the new
-// one after it.
-func TestVariantStorage_StoreReplacesMetaFile(t *testing.T) {
-	t.Parallel()
-
-	storage, key := storeTestVariant(t)
-	metaPath := storage.keyToPath(key) + variantMetaSuffix
-	oldMetaPath := metaPath + ".old"
-
-	err := os.Link(metaPath, oldMetaPath)
-	if err != nil {
-		t.Fatalf("giving the .meta file a second name: %v", err)
-	}
-
-	_, err = storage.Store(key, bytes.NewReader([]byte("new variant data")),
-		testContentTypeJPEG)
-	if err != nil {
-		t.Fatalf("Store() error = %v", err)
-	}
-
-	oldMeta, err := os.Stat(oldMetaPath)
-	if err != nil {
-		t.Fatalf("stat of the old .meta file: %v", err)
-	}
-
-	newMeta, err := os.Stat(metaPath)
-	if err != nil {
-		t.Fatalf("stat of the new .meta file: %v", err)
-	}
-
-	if os.SameFile(oldMeta, newMeta) {
-		t.Error("Store() wrote into the old .meta file instead of putting " +
-			"a new one in its place")
-	}
-
-	reader, _, contentType, err := storage.LoadWithMeta(key)
-	if err != nil {
-		t.Fatalf("LoadWithMeta() error = %v", err)
-	}
-
-	_ = reader.Close()
-
-	t.Logf("content type after the store: %s", contentType)
-
-	if contentType != testContentTypeJPEG {
-		t.Errorf("content type = %q, want %q", contentType, testContentTypeJPEG)
-	}
-}
-
-// TestContentStorage_StoreHashedReplacesFileOfAnotherSize checks that a
-// store over a file of another size at the content's path, such as a
 // partial file a crash left, replaces it with the content.
-func TestContentStorage_StoreHashedReplacesFileOfAnotherSize(t *testing.T) {
+func TestContentStorage_StoreReplacesFileOfAnotherSize(t *testing.T) {
 	t.Parallel()
 
 	storage, err := NewContentStorage(t.TempDir())
@@ -328,13 +276,12 @@ func TestContentStorage_StoreHashedReplacesFileOfAnotherSize(t *testing.T) {
 	}
 
 	content := []byte("the whole source image")
-	sum := sha256.Sum256(content)
+	key := imagerequest.Source{Host: testHostCDN, Path: testPathCat}.Key()
-	hash := ContentHash(hex.EncodeToString(sum[:]))
+	path := storage.keyToPath(key)
-	path := storage.hashToPath(hash)
 
 	err = os.MkdirAll(filepath.Dir(path), StorageDirPerm)
 	if err != nil {
-		t.Fatalf("creating the content's directory: %v", err)
+		t.Fatalf("creating the source's directory: %v", err)
 	}
 
 	err = os.WriteFile(path, content[:5], StorageFilePerm)
@@ -342,12 +289,12 @@ func TestContentStorage_StoreHashedReplacesFileOfAnotherSize(t *testing.T) {
 		t.Fatalf("writing a partial file: %v", err)
 	}
 
-	_, err = storage.StoreHashed(hash, content)
+	_, err = storage.Store(key, content)
 	if err != nil {
-		t.Fatalf("StoreHashed() error = %v", err)
+		t.Fatalf("Store() error = %v", err)
 	}
 
-	stored, err := storage.Load(hash)
+	stored, err := storage.Load(key)
 	if err != nil {
 		t.Fatalf("Load() error = %v", err)
 	}

internal/imgcache/pendingcounts.go

@@ -2,17 +2,26 @@ package imgcache
 
 import (
 	"context"
+	"errors"
 	"fmt"
-	"sync/atomic"
+	"time"
+
+	"sneak.berlin/go/pixa/internal/imagerequest"
 )
 
-// StartPendingCountWrites starts the goroutine that writes the pending
+// pendingWriteInterval is how often the goroutine StartPendingCountWrites
-// counts to the database whenever there are some, one UPDATE at a time:
+// starts writes the pending access times and counts. SQLite's
-// however many requests pass their deadline before their counts are
+// CURRENT_TIMESTAMP, which an access time is written as, counts whole
-// written, at most this one write waits for the database. It is a no-op
+// seconds.
-// when already started. The goroutine outlives the caller, so it runs with
+const pendingWriteInterval = time.Second
-// its own context, which StopPendingCountWrites cancels to tell it to
+
-// finish.
+// StartPendingCountWrites starts the goroutine that writes, every
+// pendingWriteInterval, the pending access times in one transaction and then
+// the pending counts in one UPDATE. Requests only hand these to it, so none
+// waits for the database to record what it did, and only this goroutine's
+// write ever waits for it. It is a no-op when already started. The goroutine
+// outlives the caller, so it runs with its own context, which
+// StopPendingCountWrites cancels to tell it to finish.
 func (c *Cache) StartPendingCountWrites() {
 	if c.pendingCountsCancel != nil {
 		return
@@ -28,12 +37,12 @@ func (c *Cache) StartPendingCountWrites() {
 
 // StopPendingCountWrites tells the goroutine StartPendingCountWrites
 // started, if it did, to finish, and waits for its write under way to end
-// and for it to return. It then writes the pending counts left, so that
+// and for it to return. It then writes the pending access times and counts
-// they are written before the database closes. It waits for all of this at
+// left, once, so that they are written before the database closes. It waits
-// most until ctx ends, then logs the counts not written and returns an
+// for all of this at most until ctx ends. It logs what was not written,
-// error.
+// access times or counts, and returns an error.
 func (c *Cache) StopPendingCountWrites(ctx context.Context) error {
-	var err error
+	var accessTimesErr, countsErr error
 
 	if c.pendingCountsCancel != nil {
 		c.pendingCountsCancel()
@@ -41,71 +50,134 @@ func (c *Cache) StopPendingCountWrites(ctx context.Context) error {
 		select {
 		case <-c.pendingCountsDone:
 		case <-ctx.Done():
-			// The write given up on here may not reach the database:
+			// The write given up on here, of access times or of counts,
-			// database/sql closes without waiting for a query, and a
+			// may not reach the database: database/sql closes without
-			// write still waiting for the connection fails when the
+			// waiting for a query, and a write still waiting for the
-			// database closes.
+			// connection fails when the database closes. Neither is
-			err = fmt.Errorf("pending counts still being written: %w", ctx.Err())
+			// written here.
+			accessTimesErr = fmt.Errorf("access times still being written: %w", ctx.Err())
+			countsErr = fmt.Errorf("pending counts still being written: %w", ctx.Err())
 		}
 	}
 
-	if err == nil {
+	if accessTimesErr == nil && countsErr == nil {
-		err = c.writePendingCounts(ctx)
+		accessTimesErr = c.writePendingAccessTimes(ctx)
+		countsErr = c.writePendingCounts(ctx)
 	}
 
-	if err != nil {
+	if accessTimesErr != nil {
+		c.log.Error("access times not written at shutdown", "error", accessTimesErr)
+	}
+
+	if countsErr != nil {
 		c.log.Error("counts not written at shutdown",
 			"hits", c.pendingHits.Load(),
 			"misses", c.pendingMisses.Load(),
 			"upstream_fetches", c.pendingUpstreamFetches.Load(),
 			"upstream_fetch_bytes", c.pendingUpstreamFetchBytes.Load(),
 			"transforms", c.pendingTransforms.Load(),
-			"error", err,
+			"error", countsErr,
 		)
-
-		return err
 	}
 
-	return nil
+	return errors.Join(accessTimesErr, countsErr)
 }
 
 // pendingCountWriteLoop is the body of the goroutine
 // StartPendingCountWrites starts. Once ctx is cancelled it starts no write
-// and returns, but does not cut off a write under way then. A write that
+// and returns, but does not cut off a write under way then. Counts whose
-// fails leaves the counts pending, for the next write.
+// write fails stay pending, for the next write; access times whose write
+// fails are dropped.
 func (c *Cache) pendingCountWriteLoop(ctx context.Context) {
 	defer close(c.pendingCountsDone)
 
+	ticker := time.NewTicker(pendingWriteInterval)
+	defer ticker.Stop()
+
 	for {
 		select {
 		case <-ctx.Done():
 			return
-		case <-c.pendingCountsAdded:
+		case <-ticker.C:
 		}
 
-		// select picks at random when ctx is cancelled and a wakeup
+		// select picks at random when ctx is cancelled and a tick waits
-		// waits too.
+		// too.
 		if ctx.Err() != nil {
 			return
 		}
 
-		err := c.writePendingCounts(context.WithoutCancel(ctx))
+		err := c.writePendingAccessTimes(context.WithoutCancel(ctx))
+		if err != nil {
+			c.log.Warn("failed to write access times", "error", err)
+		}
+
+		err = c.writePendingCounts(context.WithoutCancel(ctx))
 		if err != nil {
 			c.log.Warn("failed to write pending counts", "error", err)
 		}
 	}
 }
 
-// addPendingCount adds n to pendingCount, one of the pending counts, and
+// writePendingAccessTimes takes out the variants waiting in
-// wakes the goroutine that writes them. pendingCountsAdded has capacity one,
+// pendingVariantAccessTimes and the sources waiting in
-// so a wakeup already waiting is enough.
+// pendingSourceAccessTimes, and sets their last_accessed_at to now in one
-func (c *Cache) addPendingCount(pendingCount *atomic.Int64, n int64) {
+// transaction, once for each however often it was used. Nothing else takes
-	pendingCount.Add(n)
+// from those channels while it runs: only the goroutine
+// StartPendingCountWrites starts calls it, or StopPendingCountWrites once
+// that goroutine has returned.
+func (c *Cache) writePendingAccessTimes(ctx context.Context) error {
+	variants := make(map[imagerequest.VariantKey]struct{})
 
-	select {
+	for range len(c.pendingVariantAccessTimes) {
-	case c.pendingCountsAdded <- struct{}{}:
+		variants[<-c.pendingVariantAccessTimes] = struct{}{}
-	default:
 	}
+
+	sources := make(map[imagerequest.SourceKey]struct{})
+
+	for range len(c.pendingSourceAccessTimes) {
+		sources[<-c.pendingSourceAccessTimes] = struct{}{}
+	}
+
+	accessTimes := len(variants) + len(sources)
+	if accessTimes == 0 {
+		return nil
+	}
+
+	tx, err := c.db.BeginTx(ctx, nil)
+	if err != nil {
+		return fmt.Errorf("failed to write %d access times: %w", accessTimes, err)
+	}
+
+	// Does nothing once the transaction is committed
+	defer func() { _ = tx.Rollback() }()
+
+	for cacheKey := range variants {
+		_, err = tx.ExecContext(ctx, `
+			UPDATE variant_content SET last_accessed_at = CURRENT_TIMESTAMP
+			WHERE cache_key = ?
+		`, string(cacheKey))
+		if err != nil {
+			return fmt.Errorf("failed to write %d access times: %w", accessTimes, err)
+		}
+	}
+
+	for sourceKey := range sources {
+		_, err = tx.ExecContext(ctx, `
+			UPDATE source_content SET last_accessed_at = CURRENT_TIMESTAMP
+			WHERE source_key = ?
+		`, string(sourceKey))
+		if err != nil {
+			return fmt.Errorf("failed to write %d access times: %w", accessTimes, err)
+		}
+	}
+
+	err = tx.Commit()
+	if err != nil {
+		return fmt.Errorf("failed to write %d access times: %w", accessTimes, err)
+	}
+
+	return nil
 }
 
 // writePendingCounts adds the pending counts to the cache_stats row in one

internal/imgcache/pendingcounts_internal_test.go

@@ -1,9 +1,12 @@
 package imgcache
 
 import (
+	"bytes"
 	"context"
 	"database/sql"
 	"errors"
+	"log/slog"
+	"strings"
 	"testing"
 	"time"
 
@@ -73,6 +76,35 @@ func waitForCounters(t *testing.T, cache *Cache, want cacheStatsCounters) {
 	}
 }
 
+// waitForPendingCountsWritten starts the goroutine that writes cache's
+// pending counts, unless it is running, and waits until it has written them
+// all, so that the database holds every count made so far. The goroutine is
+// stopped when the test ends. The database is limited to one connection
+// first: each connection to an in-memory SQLite database opens a new, empty
+// one, so the goroutine must share the connection that holds the schema.
+func waitForPendingCountsWritten(t *testing.T, cache *Cache) {
+	t.Helper()
+
+	if cache.pendingCountsCancel == nil {
+		cache.db.SetMaxOpenConns(1)
+		cache.StartPendingCountWrites()
+
+		t.Cleanup(func() { _ = cache.StopPendingCountWrites(context.Background()) })
+	}
+
+	deadline := time.Now().Add(pendingCountsWait)
+
+	for cache.pendingHits.Load()+cache.pendingMisses.Load()+
+		cache.pendingUpstreamFetches.Load()+cache.pendingUpstreamFetchBytes.Load()+
+		cache.pendingTransforms.Load() > 0 {
+		if time.Now().After(deadline) {
+			t.Fatal("the pending counts were not written")
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
+}
+
 // TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy holds the
 // database's one connection while a request whose fetch is held reaches its
 // deadline. The request must return with the deadline's error while the
@@ -123,11 +155,12 @@ func TestService_Get_ReturnsByItsDeadlineWhileTheDatabaseIsBusy(t *testing.T) {
 	waitForCounters(t, svc.cache, cacheStatsCounters{missCount: 1})
 }
 
-// TestIncrementStats_ManyCountsPastTheirDeadlineLeaveOneWriteWaiting holds the
+// TestIncrementStats_ManyCountsLeaveOneWriteWaiting holds the database's one
-// database's one connection while many misses are counted past their
+// connection while many misses are added to the pending counts, which the
-// deadline. At most one write may then wait for the connection, and every
+// goroutine StartPendingCountWrites starts writes to the database. At most one
-// miss must reach the database once it is free.
+// write may then wait for the connection, and every miss must reach the
-func TestIncrementStats_ManyCountsPastTheirDeadlineLeaveOneWriteWaiting(t *testing.T) {
+// database once it is free.
+func TestIncrementStats_ManyCountsLeaveOneWriteWaiting(t *testing.T) {
 	t.Parallel()
 
 	cache, _ := newEvictionTestCache(t, 1<<30)
@@ -140,14 +173,10 @@ func TestIncrementStats_ManyCountsPastTheirDeadlineLeaveOneWriteWaiting(t *testi
 
 	waitsBefore := cache.db.Stats().WaitCount
 
-	// Writes given a context past its deadline fail at once.
-	ended, cancel := context.WithDeadline(t.Context(), time.Now())
-	defer cancel()
-
 	const misses = 20
 
 	for range misses {
-		cache.IncrementStats(ended, false, 0)
+		cache.IncrementStats(false, 0)
 	}
 
 	// Once one write waits for the connection, any others start waiting too.
@@ -166,9 +195,9 @@ func TestIncrementStats_ManyCountsPastTheirDeadlineLeaveOneWriteWaiting(t *testi
 	waitForCounters(t, cache, cacheStatsCounters{missCount: misses})
 }
 
-// TestStats_IncludesPendingCounts counts hits and misses whose writes miss
+// TestStats_IncludesPendingCounts adds hits and misses to the pending counts,
-// their deadline, and checks that Stats adds them to the database's counts
+// with no goroutine started to write them, and checks that Stats adds them to
-// before they are written.
+// the database's counts before they are written.
 func TestStats_IncludesPendingCounts(t *testing.T) {
 	t.Parallel()
 
@@ -180,13 +209,9 @@ func TestStats_IncludesPendingCounts(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Writes given a context past its deadline fail at once.
+	cache.IncrementStats(true, 0)
-	ended, cancel := context.WithDeadline(t.Context(), time.Now())
+	cache.IncrementStats(true, 0)
-	defer cancel()
+	cache.IncrementStats(false, 0)
-
-	cache.IncrementStats(ended, true, 0)
-	cache.IncrementStats(ended, true, 0)
-	cache.IncrementStats(ended, false, 0)
 
 	stats, err := cache.Stats(t.Context())
 	if err != nil {
@@ -222,19 +247,15 @@ func TestStopPendingCountWrites_WritesPendingCounts(t *testing.T) {
 
 	waitsBefore := cache.db.Stats().WaitCount
 
-	// Writes given a context past its deadline fail at once.
+	cache.IncrementStats(true, 0)
-	ended, cancel := context.WithDeadline(t.Context(), time.Now())
-	defer cancel()
-
-	cache.IncrementStats(ended, true, 0)
 
 	// The goroutine's write waits for the connection.
 	waitForConnectionWaits(t, cache.db, waitsBefore+1)
 
 	// Counted after that write read the pending counts
-	cache.IncrementStats(ended, false, 0)
+	cache.IncrementStats(false, 0)
-	cache.IncrementUpstreamFetch(ended, 1024)
+	cache.IncrementUpstreamFetch(1024)
-	cache.IncrementTransformCount(ended)
+	cache.IncrementTransformCount()
 
 	// The goroutine is told to finish here as well as by the stop, so that it
 	// is told before its write under way ends, however late the stop runs.
@@ -295,11 +316,7 @@ func TestStopPendingCountWrites_ReturnsWhenItsContextEnds(t *testing.T) {
 
 	waitsBefore := cache.db.Stats().WaitCount
 
-	// Writes given a context past its deadline fail at once.
+	cache.IncrementStats(true, 0)
-	ended, cancel := context.WithDeadline(t.Context(), time.Now())
-	defer cancel()
-
-	cache.IncrementStats(ended, true, 0)
 
 	// The goroutine's write waits for the connection.
 	waitForConnectionWaits(t, cache.db, waitsBefore+1)
@@ -327,3 +344,88 @@ func TestStopPendingCountWrites_ReturnsWhenItsContextEnds(t *testing.T) {
 
 	waitForCounters(t, cache, cacheStatsCounters{hitCount: 1})
 }
+
+// TestLookupSource_HandsAccessTimeToTheBackgroundWriter finds a cached source
+// whose access time is an hour old. LookupSource must not write the access
+// time itself but hand it over to be written, and StopPendingCountWrites must
+// then write it.
+func TestLookupSource_HandsAccessTimeToTheBackgroundWriter(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	key := storeEvictionTestSource(t, cache, "example.com", "/photo.jpg",
+		[]byte("source bytes"))
+	anHourAgo := time.Now().Add(-time.Hour)
+	setSourceLastAccessed(t, cache, key, anHourAgo)
+
+	touchedQuery := `
+		SELECT COUNT(*) FROM source_content
+		WHERE source_key = ? AND last_accessed_at > ?
+	`
+	anHourAgoInDatabase := anHourAgo.UTC().Format(sqliteTimestampFormat)
+
+	found, _, err := cache.LookupSource(t.Context(), &imagerequest.Request{
+		Source: imagerequest.Source{Host: "example.com", Path: "/photo.jpg"},
+	})
+	if err != nil || found != key {
+		t.Fatalf("LookupSource() = %q, %v; want %q", found, err, key)
+	}
+
+	if countRows(t, cache, touchedQuery, string(key), anHourAgoInDatabase) != 0 {
+		t.Error("LookupSource wrote the source's access time itself")
+	}
+
+	if waiting := len(cache.pendingSourceAccessTimes); waiting != 1 {
+		t.Errorf("%d access times of sources wait to be written, want 1", waiting)
+	}
+
+	err = cache.StopPendingCountWrites(t.Context())
+	if err != nil {
+		t.Fatalf("StopPendingCountWrites() error = %v", err)
+	}
+
+	if countRows(t, cache, touchedQuery, string(key), anHourAgoInDatabase) != 1 {
+		t.Error("the source's access time was not written")
+	}
+}
+
+// TestStopPendingCountWrites_LogsWhatWasNotWritten makes the write of the
+// access times fail at shutdown, by dropping the table they are written to,
+// while a hit count is written. The stop must return an error and log that
+// access times were not written, and must not log the counts as not written.
+func TestStopPendingCountWrites_LogsWhatWasNotWritten(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	var logs bytes.Buffer
+
+	cache.log = slog.New(slog.NewJSONHandler(&logs, nil))
+
+	cache.touchVariant(testVariantKeyOne)
+	cache.IncrementStats(true, 0)
+
+	_, err := cache.db.ExecContext(t.Context(), `DROP TABLE variant_content`)
+	if err != nil {
+		t.Fatalf("dropping variant_content: %v", err)
+	}
+
+	err = cache.StopPendingCountWrites(t.Context())
+	t.Logf("StopPendingCountWrites() error = %v; logged:\n%s", err, logs.String())
+
+	if err == nil {
+		t.Error("StopPendingCountWrites() error = nil, want the access times' error")
+	}
+
+	if !strings.Contains(logs.String(), "access times not written at shutdown") {
+		t.Error("the access times not written were not logged")
+	}
+
+	if strings.Contains(logs.String(), "counts not written at shutdown") {
+		t.Error("the counts, which were written, were logged as not written")
+	}
+
+	if got := readCacheStatsCounters(t, cache); got.hitCount != 1 {
+		t.Errorf("hit count in the database = %d, want 1", got.hitCount)
+	}
+}

internal/imgcache/service.go

@@ -140,36 +140,37 @@ var ErrProcessingTimedOut = errors.New(
 // While its source URL is in the negative cache, a cached variant is still
 // served and a variant not in the cache is still made from a cached source;
 // only a request that needs the source fetched is refused.
+//
+// A cache hit opens the variant file and makes no database call: its access
+// time and its hit count are handed to the goroutine that
+// StartPendingCountWrites starts, without waiting for it. A variant file
+// that cannot be served, such as one whose first bytes are not those of an
+// output format, is a miss, and the variant is made again.
 func (s *Service) Get(
 	ctx context.Context, req *imagerequest.Request,
 ) (*ImageResponse, error) {
 	// Propagate AllowHTTP setting to the request
 	req.AllowHTTP = s.allowHTTP
 
-	// Check variant cache first (disk only, no DB)
+	cacheKey := imagerequest.CacheKey(req)
-	result, err := s.cache.Lookup(ctx, req)
+
-	if err != nil {
+	reader, size, contentType, err := s.cache.GetVariant(cacheKey)
-		s.log.Warn("cache lookup failed", "error", err)
+	if err == nil {
+		s.cache.touchVariant(cacheKey)
+		s.cache.IncrementStats(true, 0)
+
+		return &ImageResponse{
+			Content:       reader,
+			ContentLength: size,
+			ContentType:   contentType,
+			CacheStatus:   CacheHit,
+			ETag:          formatETag(cacheKey),
+		}, nil
 	}
 
-	// Cache hit - serve directly from disk
+	if !errors.Is(err, ErrNotFound) {
-	if result != nil && result.Hit {
+		s.log.Warn("cached variant not served, making it again",
-		reader, size, contentType, err := s.cache.GetVariant(result.CacheKey)
+			"cache_key", cacheKey, "error", err)
-		if err != nil {
-			s.log.Error("failed to get cached variant", "key", result.CacheKey, "error", err)
-			// Fall through to re-process
-		} else {
-			// Counted also when the request context has ended meanwhile
-			s.cache.IncrementStats(ctx, true, 0)
-
-			return &ImageResponse{
-				Content:       reader,
-				ContentLength: size,
-				ContentType:   contentType,
-				CacheStatus:   CacheHit,
-				ETag:          formatETag(result.CacheKey),
-			}, nil
-		}
 	}
 
 	// Cache miss - get the variant, processed once for all the requests that
@@ -177,7 +178,7 @@ func (s *Service) Get(
 	// failed or the request context has ended meanwhile
 	response, err := s.processOrWait(ctx, req)
 
-	s.cache.IncrementStats(ctx, false, 0)
+	s.cache.IncrementStats(false, 0)
 
 	if err != nil {
 		return nil, err
@@ -359,9 +360,9 @@ func withoutCancelKeepingDeadline(
 // returns it with its size; nil if the cached data is unavailable, empty or
 // exceeds maxResponseSize.
 func (s *Service) loadCachedSource(
-	contentHash ContentHash,
+	sourceKey imagerequest.SourceKey,
 ) (io.ReadCloser, int64) {
-	reader, size, err := s.cache.GetSourceContent(contentHash)
+	reader, size, err := s.cache.GetSourceContent(sourceKey)
 	if err != nil {
 		s.log.Warn("failed to load cached source, fetching", "error", err)
 
@@ -372,7 +373,7 @@ func (s *Service) loadCachedSource(
 		_ = reader.Close()
 
 		s.log.Warn("cached source exceeds max response size, discarding",
-			"hash", contentHash,
+			"source_key", sourceKey,
 			"max_bytes", s.maxResponseSize,
 		)
 
@@ -396,7 +397,7 @@ func (s *Service) processFromSourceOrFetch(
 	cacheKey imagerequest.VariantKey,
 ) (*processedVariant, error) {
 	// Check if we have cached source content
-	contentHash, _, err := s.cache.LookupSource(ctx, req)
+	sourceKey, _, err := s.cache.LookupSource(ctx, req)
 	if err != nil {
 		s.log.Warn("source lookup failed", "error", err)
 	}
@@ -406,9 +407,9 @@ func (s *Service) processFromSourceOrFetch(
 		sourceSize int64
 	)
 
-	if contentHash != "" {
+	if sourceKey != "" {
-		s.log.Debug("using cached source", "hash", contentHash)
+		s.log.Debug("using cached source", "source_key", sourceKey)
-		source, sourceSize = s.loadCachedSource(contentHash)
+		source, sourceSize = s.loadCachedSource(sourceKey)
 	}
 
 	// Fetch from upstream if we don't have source data or it's empty
@@ -474,7 +475,7 @@ func (s *Service) fetchAndProcess(
 	fetchBytes := int64(len(sourceData))
 
 	// Counted also when the request context has ended meanwhile
-	s.cache.IncrementUpstreamFetch(ctx, fetchBytes)
+	s.cache.IncrementUpstreamFetch(fetchBytes)
 
 	if err != nil {
 		return nil, fmt.Errorf("failed to read upstream response: %w", err)
@@ -575,7 +576,7 @@ func (s *Service) processAndStore(
 	processDuration := time.Since(processStart)
 
 	// Counted also when the request context has ended meanwhile
-	s.cache.IncrementTransformCount(ctx)
+	s.cache.IncrementTransformCount()
 
 	// Read processed content
 	processedData, err := io.ReadAll(processResult.Content)

internal/imgcache/service_internal_test.go

@@ -2,7 +2,6 @@ package imgcache
 
 import (
 	"context"
-	"encoding/json"
 	"io"
 	"os"
 	"testing"
@@ -550,21 +549,17 @@ func TestService_Get_Caching(t *testing.T) {
 
 // TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo caches a source and
 // a variant of it and sets back two days every time pixa stored for them.
-// For the source: fetched_at in source_metadata, fetched_at and
+// For the source: fetched_at and last_accessed_at in source_content, and the
-// last_accessed_at in source_content, the fetched_at in its metadata file,
+// access and modification times of its file. For the variant: created_at and
-// and the access and modification times of its file and of its metadata
+// last_accessed_at in variant_content, and the access and modification times
-// file. For the variant: created_at and last_accessed_at in variant_content,
+// of its file. It then checks that the variant is still served from the
-// the created_at in its .meta file, and the access and modification times of
+// cache and that a new variant is made from the cached source without
-// its file and of its .meta file. It then checks that the variant is still
+// fetching it again.
-// served from the cache and that a new variant is made from the cached source
-// without fetching it again.
 //
 // Left at the present: the files' status change and birth times, which a
 // program cannot set, and the times of the directories holding the files,
-// which other entries share. The source's stored response headers are empty,
+// which other entries share. The request_cache and output_content tables are
-// as the mock fetcher sends none. expires_at, etag and last_modified in
+// never written.
-// source_metadata and in the metadata file are never written, nor are the
-// request_cache and output_content tables.
 func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
 	t.Parallel()
 
@@ -602,6 +597,8 @@ func TestService_Get_ServesSourceAndVariantCachedTwoDaysAgo(t *testing.T) {
 
 	_ = resp.Content.Close()
 
+	waitForPendingCountsWritten(t, svc.cache)
+
 	fetches := readCacheStatsCounters(t, svc.cache).upstreamFetchCount
 	if fetches != 1 {
 		t.Errorf("upstream fetches = %d, want 1", fetches)
@@ -615,39 +612,19 @@ func backdateSource(
 ) {
 	t.Helper()
 
-	pathHash := HashPath(req.Source.Path + "?" + req.Source.Query)
+	key := req.Source.Key()
-
-	meta, err := cache.srcMetadata.Load(req.Source.Host, pathHash)
-	if err != nil {
-		t.Fatalf("reading the source's metadata file: %v", err)
-	}
-
-	meta.FetchedAt = when.Unix()
-
-	err = cache.srcMetadata.Store(req.Source.Host, pathHash, meta)
-	if err != nil {
-		t.Fatalf("writing the source's metadata file: %v", err)
-	}
-
 	dbTime := when.UTC().Format(sqliteTimestampFormat)
 
-	updateOneRow(t, cache, `
-		UPDATE source_metadata SET fetched_at = ?
-		WHERE source_host = ? AND source_path = ? AND source_query = ?
-	`, dbTime, req.Source.Host, req.Source.Path, req.Source.Query)
 	updateOneRow(t, cache,
-		`UPDATE source_content SET fetched_at = ? WHERE content_hash = ?`,
+		`UPDATE source_content SET fetched_at = ? WHERE source_key = ?`,
-		dbTime, meta.ContentHash)
+		dbTime, string(key))
-	setSourceLastAccessed(t, cache, ContentHash(meta.ContentHash), when)
+	setSourceLastAccessed(t, cache, key, when)
 
-	for _, path := range []string{
+	path := cache.srcContent.keyToPath(key)
-		cache.srcContent.hashToPath(ContentHash(meta.ContentHash)),
+
-		cache.srcMetadata.metaPath(req.Source.Host, pathHash),
+	err := os.Chtimes(path, when, when)
-	} {
+	if err != nil {
-		err = os.Chtimes(path, when, when)
+		t.Fatalf("setting back the times of %s: %v", path, err)
-		if err != nil {
-			t.Fatalf("setting back the times of %s: %v", path, err)
-		}
 	}
 }
 
@@ -660,41 +637,16 @@ func backdateVariant(
 	t.Helper()
 
 	key := imagerequest.CacheKey(req)
-
-	content, size, contentType, err := cache.variants.LoadWithMeta(key)
-	if err != nil || contentType == "" {
-		t.Fatalf("reading the variant's .meta file: content type %q, error %v",
-			contentType, err)
-	}
-
-	_ = content.Close()
-
-	meta, err := json.Marshal(VariantMeta{
-		ContentType: contentType,
-		Size:        size,
-		CreatedAt:   when.Unix(),
-	})
-	if err != nil {
-		t.Fatalf("encoding the variant's .meta file: %v", err)
-	}
-
 	path := cache.variants.keyToPath(key)
 
-	err = os.WriteFile(path+variantMetaSuffix, meta, StorageFilePerm)
-	if err != nil {
-		t.Fatalf("writing the variant's .meta file: %v", err)
-	}
-
 	updateOneRow(t, cache,
 		`UPDATE variant_content SET created_at = ? WHERE cache_key = ?`,
 		when.UTC().Format(sqliteTimestampFormat), string(key))
 	setVariantLastAccessed(t, cache, key, when)
 
-	for _, file := range []string{path, path + variantMetaSuffix} {
+	err := os.Chtimes(path, when, when)
-		err = os.Chtimes(file, when, when)
+	if err != nil {
-		if err != nil {
+		t.Fatalf("setting back the times of %s: %v", path, err)
-			t.Fatalf("setting back the times of %s: %v", file, err)
-		}
 	}
 }
 

internal/imgcache/source_content_internal_test.go

@@ -0,0 +1,146 @@
+package imgcache
+
+import (
+	"bytes"
+	"io"
+	"os"
+	"path/filepath"
+	"testing"
+
+	"sneak.berlin/go/pixa/internal/httpfetcher"
+	"sneak.berlin/go/pixa/internal/imagerequest"
+)
+
+// TestStoreSource_StoresIdenticalBytesOfTwoURLsTwice stores the same bytes
+// for two source URLs and checks that each URL has a source_content row of
+// its own and a file of its own, named by its source key under
+// cache/sources/<ab>/<cd>/.
+func TestStoreSource_StoresIdenticalBytesOfTwoURLsTwice(t *testing.T) {
+	t.Parallel()
+
+	cache, stateDir := newEvictionTestCache(t, 1<<30)
+	content := bytes.Repeat([]byte{0x5A}, 400)
+
+	keys := map[string]imagerequest.SourceKey{
+		"/a.jpg": storeEvictionTestSource(t, cache, testHostCDN, "/a.jpg", content),
+		"/b.jpg": storeEvictionTestSource(t, cache, testHostCDN, "/b.jpg", content),
+	}
+	t.Logf("source keys: %v", keys)
+
+	if keys["/a.jpg"] == keys["/b.jpg"] {
+		t.Fatalf("both source URLs got the key %s", keys["/a.jpg"])
+	}
+
+	for sourcePath, key := range keys {
+		rows := countRows(t, cache, `
+			SELECT COUNT(*) FROM source_content
+			WHERE source_key = ? AND source_host = ? AND source_path = ?
+		`, string(key), testHostCDN, sourcePath)
+		if rows != 1 {
+			t.Errorf("%s has %d source_content rows, want 1", sourcePath, rows)
+		}
+
+		name := string(key)
+		file := filepath.Join(stateDir, "cache", "sources", name[0:2], name[2:4], name)
+
+		stored, err := os.ReadFile(file) //nolint:gosec // path in the test's state dir
+		if err != nil || !bytes.Equal(stored, content) {
+			t.Errorf("file of %s at %s holds %d bytes (error %v), want the %d stored",
+				sourcePath, file, len(stored), err, len(content))
+		}
+	}
+
+	usage, err := cache.UsageBytes(t.Context())
+	if err != nil {
+		t.Fatalf("UsageBytes() error = %v", err)
+	}
+
+	if usage != 800 {
+		t.Errorf("UsageBytes() = %d, want 800, the 400 bytes once for each URL", usage)
+	}
+}
+
+// TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed stores the same
+// bytes for two source URLs and evicts the first. Its row and file must be
+// gone, and the second must still be found and read from the cache.
+func TestEvictSourceBlob_LeavesSourceWithIdenticalBytesServed(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+	content := bytes.Repeat([]byte{0x5B}, 400)
+
+	evicted := storeEvictionTestSource(t, cache, testHostCDN, "/evicted.jpg", content)
+	storeEvictionTestSource(t, cache, testHostCDN, "/kept.jpg", content)
+
+	err := cache.evictSourceBlob(t.Context(), evicted)
+	if err != nil {
+		t.Fatalf("evictSourceBlob() error = %v", err)
+	}
+
+	if cache.srcContent.Exists(evicted) {
+		t.Error("the evicted source's file is still on disk")
+	}
+
+	if n := countRows(t, cache,
+		`SELECT COUNT(*) FROM source_content WHERE source_key = ?`, string(evicted),
+	); n != 0 {
+		t.Errorf("the evicted source still has %d source_content rows", n)
+	}
+
+	kept, contentType, err := cache.LookupSource(t.Context(), &imagerequest.Request{
+		Source: imagerequest.Source{Host: testHostCDN, Path: "/kept.jpg"},
+	})
+	t.Logf("LookupSource() of the kept source = %q, %q, error %v", kept, contentType, err)
+
+	if err != nil || kept == "" {
+		t.Fatalf("LookupSource() of the kept source = %q, %v; want it cached", kept, err)
+	}
+
+	reader, _, err := cache.GetSourceContent(kept)
+	if err != nil {
+		t.Fatalf("GetSourceContent() of the kept source error = %v", err)
+	}
+
+	defer func() { _ = reader.Close() }()
+
+	served, err := io.ReadAll(reader)
+	if err != nil {
+		t.Fatalf("reading the kept source: %v", err)
+	}
+
+	if !bytes.Equal(served, content) {
+		t.Errorf("the kept source reads as %d bytes, want the %d stored",
+			len(served), len(content))
+	}
+}
+
+// TestStoreSource_FailedRowWriteLeavesNoFile drops source_content, so that
+// StoreSource cannot write the source's row, and checks that the store fails
+// and leaves no file under the source's key.
+func TestStoreSource_FailedRowWriteLeavesNoFile(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	_, err := cache.db.ExecContext(t.Context(), `DROP TABLE source_content`)
+	if err != nil {
+		t.Fatalf("failed to drop source_content: %v", err)
+	}
+
+	req := &imagerequest.Request{
+		Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat},
+	}
+	result := &httpfetcher.FetchResult{ContentType: testContentTypeJPEG}
+
+	key, err := cache.StoreSource(t.Context(), req,
+		bytes.NewReader([]byte("source bytes")), result)
+	t.Logf("StoreSource() = %q, error = %v", key, err)
+
+	if err == nil {
+		t.Error("StoreSource() error = nil, want the failed row write")
+	}
+
+	if cache.srcContent.Exists(req.Source.Key()) {
+		t.Error("the source's file is on disk although its row was not written")
+	}
+}

internal/imgcache/stats_internal_test.go

@@ -148,43 +148,6 @@ func TestStats_LogsFailedCountQueries(t *testing.T) {
 	}
 }
 
-// TestIncrementStats_LogsFailedUpdates verifies that a failed hit or miss
-// count update and a failed upstream fetch count update are each logged at
-// warn.
-func TestIncrementStats_LogsFailedUpdates(t *testing.T) {
-	t.Parallel()
-
-	db := setupStatsTestDB(t)
-
-	var logBuf bytes.Buffer
-
-	cache, err := NewCache(db, CacheConfig{
-		StateDir:    t.TempDir(),
-		NegativeTTL: 5 * time.Minute,
-		Logger:      slog.New(slog.NewJSONHandler(&logBuf, nil)),
-	})
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	_, err = db.ExecContext(t.Context(), `DROP TABLE cache_stats`)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	cache.IncrementStats(t.Context(), false, 1024)
-
-	for _, msg := range []string{
-		"failed to count cache hit or miss",
-		"failed to count upstream fetch",
-	} {
-		want := `"level":"WARN","msg":"` + msg + `"`
-		if !strings.Contains(logBuf.String(), want) {
-			t.Errorf("log missing %s; got %q", want, logBuf.String())
-		}
-	}
-}
-
 // TestStats_TotalsCountSourcesAndVariants verifies that TotalItems and
 // TotalSizeBytes cover the stored source images and processed variants.
 func TestStats_TotalsCountSourcesAndVariants(t *testing.T) {
@@ -340,6 +303,8 @@ func TestService_Get_CountsStats(t *testing.T) {
 			_ = resp.Content.Close()
 		}
 
+		waitForPendingCountsWritten(t, svc.cache)
+
 		got := readCacheStatsCounters(t, svc.cache)
 		if got != step.want {
 			t.Fatalf("after the %s: counters = %+v, want %+v", step.name, got, step.want)
@@ -446,6 +411,8 @@ func TestService_Get_CountsInterruptedMisses(t *testing.T) {
 				_ = resp.Content.Close()
 			}
 
+			waitForPendingCountsWritten(t, svc.cache)
+
 			got := readCacheStatsCounters(t, svc.cache)
 			if got != tc.want {
 				t.Errorf("counters = %+v, want %+v", got, tc.want)
@@ -483,6 +450,8 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 
 	_ = resp.Content.Close()
 
+	waitForPendingCountsWritten(t, svc.cache)
+
 	want := readCacheStatsCounters(t, svc.cache)
 	want.hitCount++
 
@@ -500,6 +469,8 @@ func TestService_Get_CountsHitAfterRequestEnds(t *testing.T) {
 		t.Fatalf("CacheStatus = %v, want %v", resp.CacheStatus, CacheHit)
 	}
 
+	waitForPendingCountsWritten(t, svc.cache)
+
 	got := readCacheStatsCounters(t, svc.cache)
 	if got != want {
 		t.Errorf("counters = %+v, want %+v", got, want)

internal/imgcache/storage.go

@@ -1,18 +1,14 @@
 package imgcache
 
 import (
-	"crypto/sha256"
-	"encoding/hex"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"io"
-	"log/slog"
 	"os"
 	"path/filepath"
-	"time"
 
 	"sneak.berlin/go/pixa/internal/imagerequest"
+	"sneak.berlin/go/pixa/internal/magic"
 )
 
 // Storage constants.
@@ -30,14 +26,8 @@ var (
 	ErrNotFound = errors.New("content not found")
 )
 
-// ContentHash is a SHA256 hash of file content (hex-encoded).
+// ContentStorage handles source image file storage, one file per source
-type ContentHash string
+// URL. Files are stored at: <basedir>/<ab>/<cd>/<source key>
-
-// PathHash is a SHA256 hash of a URL path (hex-encoded).
-type PathHash string
-
-// ContentStorage handles content-addressable file storage.
-// Files are stored at: <basedir>/<ab>/<cd>/<abcdef...sha256>
 type ContentStorage struct {
 	baseDir string
 }
@@ -52,38 +42,19 @@ func NewContentStorage(baseDir string) (*ContentStorage, error) {
 	return &ContentStorage{baseDir: baseDir}, nil
 }
 
-// Store writes content to storage and returns its SHA256 hash.
+// Store writes data with writeFileSynced to the path derived from key,
-// The content is read fully into memory to compute the hash before writing.
+// replacing any file already there, and returns its size.
-func (s *ContentStorage) Store(r io.Reader) (ContentHash, int64, error) {
+func (s *ContentStorage) Store(
-	// Read all content to compute hash
+	key imagerequest.SourceKey, data []byte,
-	data, err := io.ReadAll(r)
-	if err != nil {
-		return "", 0, fmt.Errorf("failed to read content: %w", err)
-	}
-
-	// Compute hash
-	h := sha256.Sum256(data)
-	hash := ContentHash(hex.EncodeToString(h[:]))
-	size := int64(len(data))
-
-	err = s.writeIfAbsent(hash, data)
-	if err != nil {
-		return "", 0, err
-	}
-
-	return hash, size, nil
-}
-
-// StoreHashed writes pre-hashed content to storage at the path derived
-// from hash, without recomputing it. Callers that already know the
-// hash before writing (e.g. because they must hold a hash-keyed lock
-// across the whole store operation) use this instead of Store. Like
-// Store, it is idempotent: a file of the content's size already at that
-// path is left untouched, and a file of another size is replaced.
-func (s *ContentStorage) StoreHashed(
-	hash ContentHash, data []byte,
 ) (int64, error) {
-	err := s.writeIfAbsent(hash, data)
+	path := s.keyToPath(key)
+
+	err := os.MkdirAll(filepath.Dir(path), StorageDirPerm)
+	if err != nil {
+		return 0, fmt.Errorf("failed to create directory: %w", err)
+	}
+
+	err = writeFileSynced(path, data)
 	if err != nil {
 		return 0, err
 	}
@@ -91,11 +62,11 @@ func (s *ContentStorage) StoreHashed(
 	return int64(len(data)), nil
 }
 
-// Load returns a reader for the content with the given hash.
+// Load returns a reader for the content with the given key.
-func (s *ContentStorage) Load(hash ContentHash) (io.ReadCloser, error) {
+func (s *ContentStorage) Load(key imagerequest.SourceKey) (io.ReadCloser, error) {
-	path := s.hashToPath(hash)
+	path := s.keyToPath(key)
 
-	f, err := os.Open(path) //nolint:gosec // path derived from content hash
+	f, err := os.Open(path) //nolint:gosec // path derived from source key
 	if err != nil {
 		if os.IsNotExist(err) {
 			return nil, ErrNotFound
@@ -107,11 +78,13 @@ func (s *ContentStorage) Load(hash ContentHash) (io.ReadCloser, error) {
 	return f, nil
 }
 
-// LoadWithSize returns a reader and file size for the content with the given hash.
+// LoadWithSize returns a reader and file size for the content with the given key.
-func (s *ContentStorage) LoadWithSize(hash ContentHash) (io.ReadCloser, int64, error) {
+func (s *ContentStorage) LoadWithSize(
-	path := s.hashToPath(hash)
+	key imagerequest.SourceKey,
+) (io.ReadCloser, int64, error) {
+	path := s.keyToPath(key)
 
-	f, err := os.Open(path) //nolint:gosec // path derived from content hash
+	f, err := os.Open(path) //nolint:gosec // path derived from source key
 	if err != nil {
 		if os.IsNotExist(err) {
 			return nil, 0, ErrNotFound
@@ -130,9 +103,9 @@ func (s *ContentStorage) LoadWithSize(hash ContentHash) (io.ReadCloser, int64, e
 	return f, stat.Size(), nil
 }
 
-// Delete removes content with the given hash.
+// Delete removes content with the given key.
-func (s *ContentStorage) Delete(hash ContentHash) error {
+func (s *ContentStorage) Delete(key imagerequest.SourceKey) error {
-	path := s.hashToPath(hash)
+	path := s.keyToPath(key)
 
 	err := os.Remove(path)
 	if err != nil && !os.IsNotExist(err) {
@@ -142,34 +115,14 @@ func (s *ContentStorage) Delete(hash ContentHash) error {
 	return nil
 }
 
-// Exists checks if content with the given hash exists.
+// Exists checks if content with the given key exists.
-func (s *ContentStorage) Exists(hash ContentHash) bool {
+func (s *ContentStorage) Exists(key imagerequest.SourceKey) bool {
-	path := s.hashToPath(hash)
+	path := s.keyToPath(key)
 	_, err := os.Stat(path)
 
 	return err == nil
 }
 
-// writeIfAbsent writes data with writeFileSynced to the path derived
-// from hash, unless a file of the data's size is already there.
-func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error {
-	// Build path: <basedir>/<ab>/<cd>/<hash>
-	path := s.hashToPath(hash)
-
-	// A file of another size there is damaged, and is replaced
-	info, statErr := os.Stat(path)
-	if statErr == nil && info.Size() == int64(len(data)) {
-		return nil
-	}
-
-	err := os.MkdirAll(filepath.Dir(path), StorageDirPerm)
-	if err != nil {
-		return fmt.Errorf("failed to create directory: %w", err)
-	}
-
-	return writeFileSynced(path, data)
-}
-
 // writeFileSynced writes data to path through a temp file in the same
 // directory: the temp file is synced to disk, renamed to path, and the
 // directory synced after, so that neither a concurrent reader nor a
@@ -244,237 +197,58 @@ func syncDirectory(dir string) error {
 	return nil
 }
 
-// hashToPath converts a hash to a file path: <basedir>/<ab>/<cd>/<hash>
+// keyToPath converts a key to a file path: <basedir>/<ab>/<cd>/<key>
-func (s *ContentStorage) hashToPath(hash ContentHash) string {
+func (s *ContentStorage) keyToPath(key imagerequest.SourceKey) string {
-	h := string(hash)
+	k := string(key)
-	if len(h) < MinHashLength {
+	if len(k) < MinHashLength {
-		return filepath.Clean(filepath.Join(s.baseDir, h))
+		return filepath.Clean(filepath.Join(s.baseDir, k))
 	}
 
-	return filepath.Clean(filepath.Join(s.baseDir, h[0:2], h[2:4], h))
+	return filepath.Clean(filepath.Join(s.baseDir, k[0:2], k[2:4], k))
-}
-
-// MetadataStorage handles JSON metadata file storage.
-// Files are stored at: <basedir>/<hostname>/<path_hash>.json
-type MetadataStorage struct {
-	baseDir string
-}
-
-// NewMetadataStorage creates a new metadata storage at the given base directory.
-func NewMetadataStorage(baseDir string) (*MetadataStorage, error) {
-	err := os.MkdirAll(baseDir, StorageDirPerm)
-	if err != nil {
-		return nil, fmt.Errorf("failed to create metadata directory: %w", err)
-	}
-
-	return &MetadataStorage{baseDir: baseDir}, nil
-}
-
-// SourceMetadata represents cached metadata about a source URL.
-//
-//nolint:tagliatelle // stored metadata format uses snake_case
-type SourceMetadata struct {
-	Host            string              `json:"host"`
-	Path            string              `json:"path"`
-	Query           string              `json:"query,omitempty"`
-	ContentHash     string              `json:"content_hash,omitempty"`
-	StatusCode      int                 `json:"status_code"`
-	ContentType     string              `json:"content_type,omitempty"`
-	ContentLength   int64               `json:"content_length,omitempty"`
-	ResponseHeaders map[string][]string `json:"response_headers,omitempty"`
-	FetchedAt       int64               `json:"fetched_at"`
-	FetchDurationMs int64               `json:"fetch_duration_ms,omitempty"`
-	ExpiresAt       int64               `json:"expires_at,omitempty"`
-	ETag            string              `json:"etag,omitempty"`
-	LastModified    string              `json:"last_modified,omitempty"`
-	RemoteAddr      string              `json:"remote_addr,omitempty"`
-}
-
-// Store writes metadata to storage.
-func (s *MetadataStorage) Store(
-	host string, pathHash PathHash, meta *SourceMetadata,
-) error {
-	path := s.metaPath(host, pathHash)
-
-	// Create directory structure
-	dir := filepath.Dir(path)
-
-	err := os.MkdirAll(dir, StorageDirPerm)
-	if err != nil {
-		return fmt.Errorf("failed to create directory: %w", err)
-	}
-
-	// Marshal to JSON
-	data, err := json.MarshalIndent(meta, "", "  ")
-	if err != nil {
-		return fmt.Errorf("failed to marshal metadata: %w", err)
-	}
-
-	// Write to temp file first, then rename for atomicity
-	tmpFile, err := os.CreateTemp(dir, ".tmp-*.json")
-	if err != nil {
-		return fmt.Errorf("failed to create temp file: %w", err)
-	}
-
-	tmpPath := tmpFile.Name()
-
-	_, err = tmpFile.Write(data)
-	if err != nil {
-		_ = tmpFile.Close()
-		_ = os.Remove(tmpPath)
-
-		return fmt.Errorf("failed to write metadata: %w", err)
-	}
-
-	err = tmpFile.Close()
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return fmt.Errorf("failed to close temp file: %w", err)
-	}
-
-	// Atomic rename
-	err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path))
-	if err != nil {
-		_ = os.Remove(tmpPath)
-
-		return fmt.Errorf("failed to rename temp file: %w", err)
-	}
-
-	return nil
-}
-
-// Load reads metadata from storage.
-func (s *MetadataStorage) Load(
-	host string, pathHash PathHash,
-) (*SourceMetadata, error) {
-	path := s.metaPath(host, pathHash)
-
-	data, err := os.ReadFile(path) //nolint:gosec // path derived from host+hash
-	if err != nil {
-		if os.IsNotExist(err) {
-			return nil, ErrNotFound
-		}
-
-		return nil, fmt.Errorf("failed to read metadata: %w", err)
-	}
-
-	var meta SourceMetadata
-
-	err = json.Unmarshal(data, &meta)
-	if err != nil {
-		return nil, fmt.Errorf("failed to unmarshal metadata: %w", err)
-	}
-
-	return &meta, nil
-}
-
-// Delete removes metadata for the given host and path hash.
-func (s *MetadataStorage) Delete(host string, pathHash PathHash) error {
-	path := s.metaPath(host, pathHash)
-
-	err := os.Remove(path)
-	if err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to delete metadata: %w", err)
-	}
-
-	return nil
-}
-
-// Exists checks if metadata exists for the given host and path hash.
-func (s *MetadataStorage) Exists(host string, pathHash PathHash) bool {
-	path := s.metaPath(host, pathHash)
-	_, err := os.Stat(path)
-
-	return err == nil
-}
-
-// metaPath returns the file path for metadata: <basedir>/<host>/<path_hash>.json
-func (s *MetadataStorage) metaPath(host string, pathHash PathHash) string {
-	return filepath.Clean(filepath.Join(s.baseDir, host, string(pathHash)+".json"))
-}
-
-// HashPath computes the SHA256 hash of a path string.
-func HashPath(path string) PathHash {
-	h := sha256.Sum256([]byte(path))
-
-	return PathHash(hex.EncodeToString(h[:]))
 }
 
 // VariantStorage handles key-based file storage for processed image variants.
 // Files are stored at: <basedir>/<ab>/<cd>/<cache_key>
-// Metadata is stored at: <basedir>/<ab>/<cd>/<cache_key>.meta
-// Unlike ContentStorage, the key is provided by the caller (not computed from content).
 type VariantStorage struct {
 	baseDir string
-	log     *slog.Logger
 }
 
-// VariantMeta contains metadata about a cached variant.
+// errNotAnOutputFormat is returned for a variant file whose first bytes are
-//
+// those of a format pixa reads but never writes, such as SVG.
-//nolint:tagliatelle // stored metadata format uses snake_case
+var errNotAnOutputFormat = errors.New("variant file is not in an output format")
-type VariantMeta struct {
-	ContentType string `json:"content_type"`
-	Size        int64  `json:"size"`
-	CreatedAt   int64  `json:"created_at"`
-}
 
 // NewVariantStorage creates a new variant storage at the given base directory.
-// A failed .meta write is logged to log.
+func NewVariantStorage(baseDir string) (*VariantStorage, error) {
-func NewVariantStorage(baseDir string, log *slog.Logger) (*VariantStorage, error) {
 	err := os.MkdirAll(baseDir, StorageDirPerm)
 	if err != nil {
 		return nil, fmt.Errorf("failed to create variant storage directory: %w", err)
 	}
 
-	return &VariantStorage{baseDir: baseDir, log: log}, nil
+	return &VariantStorage{baseDir: baseDir}, nil
 }
 
-// Store writes content and metadata to storage at the given key, each
+// Store writes content to storage at the given key with writeFileSynced.
-// with writeFileSynced. The .meta file is written first, so that a crash
-// cannot leave the content in place without it.
 func (s *VariantStorage) Store(
-	key imagerequest.VariantKey, r io.Reader, contentType string,
+	key imagerequest.VariantKey, r io.Reader,
 ) (int64, error) {
 	data, err := io.ReadAll(r)
 	if err != nil {
 		return 0, fmt.Errorf("failed to read content: %w", err)
 	}
 
-	size := int64(len(data))
 	path := s.keyToPath(key)
-	metaPath := path + ".meta"
 
 	err = os.MkdirAll(filepath.Dir(path), StorageDirPerm)
 	if err != nil {
 		return 0, fmt.Errorf("failed to create directory: %w", err)
 	}
 
-	meta := VariantMeta{
-		ContentType: contentType,
-		Size:        size,
-		CreatedAt:   time.Now().UTC().Unix(),
-	}
-
-	metaData, err := json.Marshal(meta)
-	if err != nil {
-		return 0, fmt.Errorf("failed to marshal metadata: %w", err)
-	}
-
-	// Metadata write failure is non-fatal: the content is then served as
-	// application/octet-stream.
-	err = writeFileSynced(metaPath, metaData)
-	if err != nil {
-		s.log.Warn("failed to write variant metadata sidecar",
-			"path", metaPath, "error", err)
-	}
-
 	err = writeFileSynced(path, data)
 	if err != nil {
 		return 0, err
 	}
 
-	return size, nil
+	return int64(len(data)), nil
 }
 
 // Load returns a reader for the content at the given key.
@@ -493,56 +267,63 @@ func (s *VariantStorage) Load(key imagerequest.VariantKey) (io.ReadCloser, error
 	return f, nil
 }
 
-// LoadWithSize returns a reader and file size for the content at the
+// LoadWithContentType opens the content at the given key and returns a
-// given key.
+// reader, the file size and the content type, which is read from the
-func (s *VariantStorage) LoadWithSize(
+// file's first bytes. A missing file is ErrNotFound. A file whose first
+// bytes are not those of an output format, an empty one included, is
+// closed again and the reason returned.
+func (s *VariantStorage) LoadWithContentType(
 	key imagerequest.VariantKey,
-) (io.ReadCloser, int64, error) {
+) (io.ReadCloser, int64, string, error) {
 	path := s.keyToPath(key)
 
 	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
+	contentType, err := outputContentType(f)
-}
-
-// LoadWithMeta returns a reader, size, and content type for the content at
-// 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 imagerequest.VariantKey,
-) (io.ReadCloser, int64, string, error) {
-	f, size, err := s.LoadWithSize(key)
 	if err != nil {
+		_ = f.Close()
+
 		return nil, 0, "", err
 	}
 
-	var contentType string
+	return f, stat.Size(), contentType, nil
+}
 
-	metaPath := s.keyToPath(key) + ".meta"
+// outputContentType reads the first bytes of content, without moving its
+// offset, and returns the content type of the output format they show.
+func outputContentType(content io.ReaderAt) (string, error) {
+	firstBytes := make([]byte, magic.MinMagicBytes)
 
-	metaData, err := os.ReadFile(metaPath) //nolint:gosec // path derived from cache key
+	_, err := content.ReadAt(firstBytes, 0)
-	if err == nil {
+	if err != nil {
-		var meta VariantMeta
+		return "", fmt.Errorf("failed to read the first bytes: %w", err)
-		if json.Unmarshal(metaData, &meta) == nil && meta.ContentType != "" {
-			contentType = meta.ContentType
-		}
 	}
 
-	return f, size, contentType, nil
+	detected, err := magic.DetectFormat(firstBytes)
+	if err != nil {
+		return "", err
+	}
+
+	_, isOutputFormat := magic.MIMEToImageFormat(string(detected))
+	if !isOutputFormat {
+		return "", fmt.Errorf("%w: %s", errNotAnOutputFormat, detected)
+	}
+
+	return string(detected), nil
 }
 
 // Exists checks if content exists at the given key.
@@ -553,8 +334,8 @@ func (s *VariantStorage) Exists(key imagerequest.VariantKey) bool {
 	return err == nil
 }
 
-// Delete removes the content at the given key together with its .meta
+// Delete removes the content at the given key. A missing file is not an
-// sidecar file. A missing file is not an error.
+// error.
 func (s *VariantStorage) Delete(key imagerequest.VariantKey) error {
 	path := s.keyToPath(key)
 
@@ -563,13 +344,6 @@ func (s *VariantStorage) Delete(key imagerequest.VariantKey) error {
 		return fmt.Errorf("failed to delete content: %w", err)
 	}
 
-	metaPath := path + ".meta"
-
-	err = os.Remove(metaPath)
-	if err != nil && !os.IsNotExist(err) {
-		return fmt.Errorf("failed to delete variant metadata: %w", err)
-	}
-
 	return nil
 }
 

internal/imgcache/storage_internal_test.go

@@ -4,10 +4,8 @@ import (
 	"bytes"
 	"errors"
 	"io"
-	"log/slog"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"sneak.berlin/go/pixa/internal/imagerequest"
@@ -24,8 +22,9 @@ func TestContentStorage_StoreAndLoad(t *testing.T) {
 	}
 
 	content := []byte("hello world")
+	key := imagerequest.Source{Host: testHostCDN, Path: testPathCat}.Key()
 
-	hash, size, err := storage.Store(bytes.NewReader(content))
+	size, err := storage.Store(key, content)
 	if err != nil {
 		t.Fatalf("Store() error = %v", err)
 	}
@@ -34,14 +33,10 @@ func TestContentStorage_StoreAndLoad(t *testing.T) {
 		t.Errorf("Store() size = %d, want %d", size, len(content))
 	}
 
-	if hash == "" {
-		t.Error("Store() returned empty hash")
-	}
-
 	// Verify file exists at expected path
-	hashStr := string(hash)
+	keyStr := string(key)
 
-	expectedPath := filepath.Join(tmpDir, hashStr[0:2], hashStr[2:4], hashStr)
+	expectedPath := filepath.Join(tmpDir, keyStr[0:2], keyStr[2:4], keyStr)
 
 	_, err = os.Stat(expectedPath)
 	if err != nil {
@@ -49,7 +44,7 @@ func TestContentStorage_StoreAndLoad(t *testing.T) {
 	}
 
 	// Load and verify content
-	r, err := storage.Load(hash)
+	r, err := storage.Load(key)
 	if err != nil {
 		t.Fatalf("Load() error = %v", err)
 	}
@@ -66,33 +61,6 @@ func TestContentStorage_StoreAndLoad(t *testing.T) {
 	}
 }
 
-func TestContentStorage_StoreIdempotent(t *testing.T) {
-	t.Parallel()
-
-	tmpDir := t.TempDir()
-
-	storage, err := NewContentStorage(tmpDir)
-	if err != nil {
-		t.Fatalf("NewContentStorage() error = %v", err)
-	}
-
-	content := []byte("same content")
-
-	hash1, _, err := storage.Store(bytes.NewReader(content))
-	if err != nil {
-		t.Fatalf("Store() first error = %v", err)
-	}
-
-	hash2, _, err := storage.Store(bytes.NewReader(content))
-	if err != nil {
-		t.Fatalf("Store() second error = %v", err)
-	}
-
-	if hash1 != hash2 {
-		t.Errorf("Store() hashes differ: %s vs %s", hash1, hash2)
-	}
-}
-
 func TestContentStorage_LoadNotFound(t *testing.T) {
 	t.Parallel()
 
@@ -103,7 +71,7 @@ func TestContentStorage_LoadNotFound(t *testing.T) {
 		t.Fatalf("NewContentStorage() error = %v", err)
 	}
 
-	_, err = storage.Load(ContentHash("nonexistent"))
+	_, err = storage.Load(imagerequest.SourceKey("nonexistent"))
 	if !errors.Is(err, ErrNotFound) {
 		t.Errorf("Load() error = %v, want ErrNotFound", err)
 	}
@@ -120,22 +88,23 @@ func TestContentStorage_Delete(t *testing.T) {
 	}
 
 	content := []byte("to be deleted")
+	key := imagerequest.Source{Host: testHostCDN, Path: testPathCat}.Key()
 
-	hash, _, err := storage.Store(bytes.NewReader(content))
+	_, err = storage.Store(key, content)
 	if err != nil {
 		t.Fatalf("Store() error = %v", err)
 	}
 
-	if !storage.Exists(hash) {
+	if !storage.Exists(key) {
 		t.Error("Exists() = false, want true")
 	}
 
-	err = storage.Delete(hash)
+	err = storage.Delete(key)
 	if err != nil {
 		t.Fatalf("Delete() error = %v", err)
 	}
 
-	if storage.Exists(hash) {
+	if storage.Exists(key) {
 		t.Error("Exists() = true after delete, want false")
 	}
 }
@@ -151,13 +120,13 @@ func TestContentStorage_DeleteNonexistent(t *testing.T) {
 	}
 
 	// Should not error
-	err = storage.Delete(ContentHash("nonexistent"))
+	err = storage.Delete(imagerequest.SourceKey("nonexistent"))
 	if err != nil {
 		t.Errorf("Delete() error = %v, want nil", err)
 	}
 }
 
-func TestContentStorage_HashToPath(t *testing.T) {
+func TestContentStorage_KeyToPath(t *testing.T) {
 	t.Parallel()
 
 	tmpDir := t.TempDir()
@@ -169,263 +138,19 @@ func TestContentStorage_HashToPath(t *testing.T) {
 
 	// Test by storing and verifying the resulting path structure
 	content := []byte("test content for path verification")
+	key := imagerequest.Source{Host: testHostCDN, Path: testPathCat}.Key()
 
-	hash, _, err := storage.Store(bytes.NewReader(content))
+	_, err = storage.Store(key, content)
 	if err != nil {
 		t.Fatalf("Store() error = %v", err)
 	}
 
-	hashStr := string(hash)
+	keyStr := string(key)
 
-	expectedPath := filepath.Join(tmpDir, hashStr[0:2], hashStr[2:4], hashStr)
+	expectedPath := filepath.Join(tmpDir, keyStr[0:2], keyStr[2:4], keyStr)
 
 	_, err = os.Stat(expectedPath)
 	if err != nil {
 		t.Errorf("File not at expected path %s: %v", expectedPath, err)
 	}
 }
-
-func TestMetadataStorage_StoreAndLoad(t *testing.T) {
-	t.Parallel()
-
-	tmpDir := t.TempDir()
-
-	storage, err := NewMetadataStorage(tmpDir)
-	if err != nil {
-		t.Fatalf("NewMetadataStorage() error = %v", err)
-	}
-
-	meta := &SourceMetadata{
-		Host:        testHostCDN,
-		Path:        testPathCat,
-		ContentHash: "abc123",
-		StatusCode:  200,
-		ContentType: testContentTypeJPEG,
-		FetchedAt:   1704067200,
-		ETag:        `"etag123"`,
-	}
-
-	pathHash := HashPath(testPathCat)
-
-	err = storage.Store(testHostCDN, pathHash, meta)
-	if err != nil {
-		t.Fatalf("Store() error = %v", err)
-	}
-
-	// Verify file exists at expected path
-	expectedPath := filepath.Join(tmpDir, testHostCDN, string(pathHash)+".json")
-
-	_, err = os.Stat(expectedPath)
-	if err != nil {
-		t.Errorf("File not at expected path %s: %v", expectedPath, err)
-	}
-
-	// Load and verify
-	loaded, err := storage.Load(testHostCDN, pathHash)
-	if err != nil {
-		t.Fatalf("Load() error = %v", err)
-	}
-
-	if loaded.Host != meta.Host {
-		t.Errorf("Host = %q, want %q", loaded.Host, meta.Host)
-	}
-
-	if loaded.Path != meta.Path {
-		t.Errorf("Path = %q, want %q", loaded.Path, meta.Path)
-	}
-
-	if loaded.ContentHash != meta.ContentHash {
-		t.Errorf("ContentHash = %q, want %q", loaded.ContentHash, meta.ContentHash)
-	}
-
-	if loaded.StatusCode != meta.StatusCode {
-		t.Errorf("StatusCode = %d, want %d", loaded.StatusCode, meta.StatusCode)
-	}
-
-	if loaded.ETag != meta.ETag {
-		t.Errorf("ETag = %q, want %q", loaded.ETag, meta.ETag)
-	}
-}
-
-func TestMetadataStorage_LoadNotFound(t *testing.T) {
-	t.Parallel()
-
-	tmpDir := t.TempDir()
-
-	storage, err := NewMetadataStorage(tmpDir)
-	if err != nil {
-		t.Fatalf("NewMetadataStorage() error = %v", err)
-	}
-
-	_, err = storage.Load(testHostExample, PathHash("nonexistent"))
-	if !errors.Is(err, ErrNotFound) {
-		t.Errorf("Load() error = %v, want ErrNotFound", err)
-	}
-}
-
-func TestMetadataStorage_Delete(t *testing.T) {
-	t.Parallel()
-
-	tmpDir := t.TempDir()
-
-	storage, err := NewMetadataStorage(tmpDir)
-	if err != nil {
-		t.Fatalf("NewMetadataStorage() error = %v", err)
-	}
-
-	meta := &SourceMetadata{
-		Host:       testHostExample,
-		Path:       "/test.jpg",
-		StatusCode: 200,
-	}
-
-	pathHash := HashPath("/test.jpg")
-
-	err = storage.Store(testHostExample, pathHash, meta)
-	if err != nil {
-		t.Fatalf("Store() error = %v", err)
-	}
-
-	if !storage.Exists(testHostExample, pathHash) {
-		t.Error("Exists() = false, want true")
-	}
-
-	err = storage.Delete(testHostExample, pathHash)
-	if err != nil {
-		t.Fatalf("Delete() error = %v", err)
-	}
-
-	if storage.Exists(testHostExample, pathHash) {
-		t.Error("Exists() = true after delete, want false")
-	}
-}
-
-func TestHashPath(t *testing.T) {
-	t.Parallel()
-
-	// Same input should produce same hash
-	hash1 := HashPath(testPathCat)
-	hash2 := HashPath(testPathCat)
-
-	if hash1 != hash2 {
-		t.Errorf("HashPath() not deterministic: %s vs %s", hash1, hash2)
-	}
-
-	// Different input should produce different hash
-	hash3 := HashPath("/photos/dog.jpg")
-
-	if hash1 == hash3 {
-		t.Error("HashPath() produced same hash for different inputs")
-	}
-
-	// Hash should be 64 hex chars (256 bits)
-	if len(string(hash1)) != 64 {
-		t.Errorf("HashPath() length = %d, want 64", len(string(hash1)))
-	}
-}
-
-// TestVariantStorage_StoreLogsFailedMetaWrite verifies that a .meta write
-// that fails is logged at warn and the store still succeeds.
-func TestVariantStorage_StoreLogsFailedMetaWrite(t *testing.T) {
-	t.Parallel()
-
-	var logBuf bytes.Buffer
-
-	storage, err := NewVariantStorage(
-		t.TempDir(), slog.New(slog.NewJSONHandler(&logBuf, nil)))
-	if err != nil {
-		t.Fatalf("NewVariantStorage() error = %v", err)
-	}
-
-	key := imagerequest.CacheKey(&imagerequest.Request{
-		Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat},
-	})
-
-	// A directory where the .meta file goes makes the .meta write fail.
-	err = os.MkdirAll(storage.keyToPath(key)+".meta", StorageDirPerm)
-	if err != nil {
-		t.Fatalf("failed to create directory: %v", err)
-	}
-
-	_, err = storage.Store(key, bytes.NewReader([]byte("variant data")), "image/webp")
-	if err != nil {
-		t.Fatalf("Store() error = %v, want nil", err)
-	}
-
-	want := `"level":"WARN","msg":"failed to write variant metadata sidecar"`
-	if !strings.Contains(logBuf.String(), want) {
-		t.Errorf("log missing %s; got %q", want, logBuf.String())
-	}
-}
-
-// storeTestVariant stores one variant, with its .meta file, in a new
-// VariantStorage and returns the storage and the variant's key.
-func storeTestVariant(t *testing.T) (*VariantStorage, imagerequest.VariantKey) {
-	t.Helper()
-
-	storage, err := NewVariantStorage(t.TempDir(), slog.New(slog.DiscardHandler))
-	if err != nil {
-		t.Fatalf("NewVariantStorage() error = %v", err)
-	}
-
-	key := imagerequest.CacheKey(&imagerequest.Request{
-		Source: imagerequest.Source{Host: testHostCDN, Path: testPathCat},
-	})
-
-	_, err = storage.Store(key, bytes.NewReader([]byte("variant data")), "image/webp")
-	if err != nil {
-		t.Fatalf("Store() error = %v", err)
-	}
-
-	return storage, key
-}
-
-// TestVariantStorage_DeleteRemovesMeta verifies that Delete removes the
-// variant's .meta file along with the variant file.
-func TestVariantStorage_DeleteRemovesMeta(t *testing.T) {
-	t.Parallel()
-
-	storage, key := storeTestVariant(t)
-	metaPath := storage.keyToPath(key) + ".meta"
-
-	_, err := os.Stat(metaPath)
-	if err != nil {
-		t.Fatalf("Store() wrote no .meta file: %v", err)
-	}
-
-	err = storage.Delete(key)
-	if err != nil {
-		t.Fatalf("Delete() error = %v", err)
-	}
-
-	if storage.Exists(key) {
-		t.Error("Exists() = true after delete, want false")
-	}
-
-	_, err = os.Stat(metaPath)
-	if !os.IsNotExist(err) {
-		t.Errorf(".meta file left after Delete() (stat err=%v)", err)
-	}
-}
-
-// TestVariantStorage_DeleteWithoutMeta verifies that Delete succeeds for a
-// variant whose .meta file is missing.
-func TestVariantStorage_DeleteWithoutMeta(t *testing.T) {
-	t.Parallel()
-
-	storage, key := storeTestVariant(t)
-
-	err := os.Remove(storage.keyToPath(key) + ".meta")
-	if err != nil {
-		t.Fatalf("removing .meta file: %v", err)
-	}
-
-	err = storage.Delete(key)
-	if err != nil {
-		t.Fatalf("Delete() error = %v, want nil", err)
-	}
-
-	if storage.Exists(key) {
-		t.Error("Exists() = true after delete, want false")
-	}
-}

internal/server/image_proxy_internal_test.go

@@ -3,9 +3,7 @@ package server
 import (
 	"bytes"
 	"context"
-	"crypto/sha256"
 	"database/sql"
-	"encoding/hex"
 	"image"
 	"image/color"
 	"image/jpeg"
@@ -30,6 +28,7 @@ import (
 	"sneak.berlin/go/pixa/internal/handlers"
 	"sneak.berlin/go/pixa/internal/healthcheck"
 	"sneak.berlin/go/pixa/internal/httpfetcher"
+	"sneak.berlin/go/pixa/internal/imagerequest"
 	"sneak.berlin/go/pixa/internal/logger"
 	"sneak.berlin/go/pixa/internal/middleware"
 )
@@ -254,39 +253,26 @@ func getImage(t *testing.T, s *Server, target string) *httptest.ResponseRecorder
 	return rec
 }
 
-// checkSourceCached checks that source is stored under its SHA-256 in
+// checkSourceCached checks that source is stored in cache/sources under the
-// cache/sources, recorded in source_content, and that the source URL's row in
+// source key of upstreamHost and /photo.png, and recorded under that key in
-// source_metadata points at it.
+// source_content.
 func checkSourceCached(t *testing.T, db *sql.DB, stateDir string, source []byte) {
 	t.Helper()
 
-	sum := sha256.Sum256(source)
+	key := string(imagerequest.Source{Host: upstreamHost, Path: "/photo.png"}.Key())
-	hash := hex.EncodeToString(sum[:])
 
-	checkFile(t, filepath.Join(stateDir, "cache", "sources", hash[0:2], hash[2:4], hash),
+	checkFile(t, filepath.Join(stateDir, "cache", "sources", key[0:2], key[2:4], key),
 		source)
 
 	var rows int
 
 	err := db.QueryRowContext(t.Context(),
-		"SELECT COUNT(*) FROM source_content WHERE content_hash = ?", hash,
+		"SELECT COUNT(*) FROM source_content WHERE source_key = ?", key,
 	).Scan(&rows)
 	if err != nil || rows != 1 {
 		t.Errorf("source_content rows for the source = %d (error %v), want 1",
 			rows, err)
 	}
-
-	var metadataHash string
-
-	err = db.QueryRowContext(t.Context(),
-		`SELECT content_hash FROM source_metadata
-			WHERE source_host = ? AND source_path = ?`,
-		upstreamHost, "/photo.png",
-	).Scan(&metadataHash)
-	if err != nil || metadataHash != hash {
-		t.Errorf("source_metadata content_hash = %q (error %v), want %q",
-			metadataHash, err, hash)
-	}
 }
 
 // checkVariantCached checks that the converted image served is recorded in

script/bench

@@ -0,0 +1,19 @@
+#!/bin/sh
+# script/bench: run the Go benchmarks of internal/handlers, which serve
+# cache hits in parallel through the image handler and report the hits
+# served per second. A benchmark, not a check: script/check does not run
+# it. It runs Go on the host, as script/loadtest does, and compiles pixa
+# there with CGO, so it needs the C compiler and libraries that
+# `script/bootstrap --cgo` installs. -run '^$' runs no test, only the
+# benchmarks.
+set -eu
+
+SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd -P)"
+ROOT="$(cd "$SCRIPT_DIR/.." && pwd -P)"
+
+main() {
+    cd "$ROOT"
+    CGO_ENABLED=1 go test -run '^$' -bench . ./internal/handlers
+}
+
+main "$@"