Stop TestEvictionRunsOnPeriodicSchedule racing the evictor (closes #183)
check / check (push) Failing after 1s

The test wrote each variant file and then inserted its accounting row by
hand while the evictor was running. A reconciliation pass between the two
steps adopted the file first, and the hand insert failed on the unique key.

The test now writes the files only, while it holds the test database's
only connection, so the evictor's startup pass waits after walking the
still empty variant directory. A periodic reconciliation pass then adopts
the files and the eviction pass after it evicts them; no write-pressure
notification fires. The test waits until two of the three files are gone,
then checks that usage is within the limit and that no row points at a
missing file.

Model: opus-5-5
This commit is contained in:
2026-10-04 13:30:34 +00:00
parent be6c715b36
commit d6a7bd9ab6
2 changed files with 63 additions and 15 deletions
+9
View File
@@ -29,6 +29,15 @@ P2: security: referer blacklist
# Completed Steps # 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): - 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 "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 do (terminate TLS; pass `Host`, `Origin` and `Referer` on unchanged; set
+54 -15
View File
@@ -681,6 +681,11 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
assertNoDanglingReferences(t, cache) 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) { func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
t.Parallel() t.Parallel()
@@ -688,13 +693,29 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
cache, _ := newEvictionTestCache(t, limit) cache, _ := newEvictionTestCache(t, limit)
// Start the evictor while the cache is empty, then create tracked // Hold the test database's only connection, so the startup pass
// over-limit state WITHOUT going through the store methods, so no // waits for it after walking the still empty variant directory: the
// write-pressure notification fires and only the periodic ticker // files written while it waits are first seen by a periodic pass.
// can trigger eviction. 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) cache.StartEviction(100 * time.Millisecond)
defer func() { _ = cache.StopEviction(t.Context()) }() 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{ keys := []VariantKey{
testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree, testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
} }
@@ -703,25 +724,43 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
for i, key := range keys { for i, key := range keys {
content := bytes.Repeat([]byte{fills[i]}, 1000) 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 { if err != nil {
t.Fatalf("failed to store variant file: %v", err) t.Fatalf("failed to store variant file: %v", err)
} }
}
_, err = cache.db.ExecContext(t.Context(), _ = conn.Close()
`INSERT INTO variant_content (cache_key, size_bytes, content_type)
VALUES (?, ?, ?)`, // Only one of the 1000-byte files fits under the limit: wait until
string(key), len(content), "image/webp", // 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++
}
}
}
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 { if err != nil {
t.Fatalf("failed to insert variant accounting row: %v", err) t.Fatalf("UsageBytes failed: %v", err)
}
} }
usage := waitForUsageAtOrBelow(t, cache, limit, 5*time.Second)
if usage > limit { if usage > limit {
t.Errorf("periodic schedule did not trigger eviction: usage = %d, want <= %d", t.Errorf("usage after eviction = %d, want <= %d", usage, limit)
usage, limit)
} }
assertNoDanglingReferences(t, cache) assertNoDanglingReferences(t, cache)