diff --git a/TODO.md b/TODO.md index 1fa426a..ec2ee54 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,15 @@ P2: security: referer blacklist # Completed Steps +- 2026-10-04 `TestEvictionRunsOnPeriodicSchedule` no longer races the evictor + (closes #183): it wrote each variant file and then inserted its accounting row + by hand, and a reconciliation pass between the two adopted the file first, so + the insert failed. It now writes the files only, while holding the test + database's only connection so the evictor's startup pass waits after walking + the empty variant directory; a periodic reconciliation pass then adopts the + files and the eviction pass after it evicts them. No other test in + `internal/imgcache` inserts a row by hand after starting the evictor. Test + only. - 2026-10-04 deployment guide and example Caddy config (closes #89): "Deployment" in `README.md` says what the reverse proxy in front of pixa must do (terminate TLS; pass `Host`, `Origin` and `Referer` on unchanged; set diff --git a/internal/imgcache/eviction_internal_test.go b/internal/imgcache/eviction_internal_test.go index 5b51f18..4b0bbf7 100644 --- a/internal/imgcache/eviction_internal_test.go +++ b/internal/imgcache/eviction_internal_test.go @@ -681,6 +681,11 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) { assertNoDanglingReferences(t, cache) } +// TestEvictionRunsOnPeriodicSchedule writes three variant files straight +// to disk, bypassing StoreVariant, so they have no accounting rows and no +// write-pressure notification fires. Only a periodic reconciliation pass +// can then adopt them, and only the eviction pass that follows it can +// evict them. func TestEvictionRunsOnPeriodicSchedule(t *testing.T) { t.Parallel() @@ -688,13 +693,29 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) { cache, _ := newEvictionTestCache(t, limit) - // Start the evictor while the cache is empty, then create tracked - // over-limit state WITHOUT going through the store methods, so no - // write-pressure notification fires and only the periodic ticker - // can trigger eviction. + // Hold the test database's only connection, so the startup pass + // waits for it after walking the still empty variant directory: the + // files written while it waits are first seen by a periodic pass. + conn, err := cache.db.Conn(t.Context()) + if err != nil { + t.Fatalf("failed to take the database connection: %v", err) + } + + defer func() { _ = conn.Close() }() + cache.StartEviction(100 * time.Millisecond) defer func() { _ = cache.StopEviction(t.Context()) }() + deadline := time.Now().Add(5 * time.Second) + + for cache.db.Stats().WaitCount == 0 { + if time.Now().After(deadline) { + t.Fatal("the startup pass never waited for the database") + } + + time.Sleep(10 * time.Millisecond) + } + keys := []VariantKey{ testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree, } @@ -703,25 +724,43 @@ 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), "image/webp") if err != nil { t.Fatalf("failed to store variant file: %v", err) } + } - _, err = cache.db.ExecContext(t.Context(), - `INSERT INTO variant_content (cache_key, size_bytes, content_type) - VALUES (?, ?, ?)`, - string(key), len(content), "image/webp", - ) - if err != nil { - t.Fatalf("failed to insert variant accounting row: %v", err) + _ = conn.Close() + + // Only one of the 1000-byte files fits under the limit: wait until + // the evictor has removed the other two. + stored := len(keys) + deadline = time.Now().Add(5 * time.Second) + + for stored > 1 && time.Now().Before(deadline) { + time.Sleep(25 * time.Millisecond) + + stored = 0 + + for _, key := range keys { + if cache.variants.Exists(key) { + stored++ + } } } - usage := waitForUsageAtOrBelow(t, cache, limit, 5*time.Second) + if stored > 1 { + t.Fatalf("periodic schedule did not trigger eviction: %d of %d "+ + "variant files still on disk, want at most 1", stored, len(keys)) + } + + usage, err := cache.UsageBytes(t.Context()) + if err != nil { + t.Fatalf("UsageBytes failed: %v", err) + } + if usage > limit { - t.Errorf("periodic schedule did not trigger eviction: usage = %d, want <= %d", - usage, limit) + t.Errorf("usage after eviction = %d, want <= %d", usage, limit) } assertNoDanglingReferences(t, cache)