Stop TestEvictionRunsOnPeriodicSchedule racing the evictor (closes #183)
check / check (push) Failing after 1s
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 was merged in pull request #187.
This commit is contained in:
@@ -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
|
||||||
|
|||||||
@@ -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)
|
||||||
if err != nil {
|
deadline = time.Now().Add(5 * time.Second)
|
||||||
t.Fatalf("failed to insert variant accounting row: %v", err)
|
|
||||||
|
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 {
|
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)
|
||||||
|
|||||||
Reference in New Issue
Block a user