From e7964fe7778cc2ad146aa4a1a3ba7844e8def42f Mon Sep 17 00:00:00 2001
From: sneak <sneak@sneak.berlin>
Date: Sun, 9 Aug 2026 00:48:03 +0000
Subject: [PATCH] fix: run accounting reconciliation periodically, not just at
 startup

reconcileAccounting previously ran exactly once, when the evictor
goroutine started. Combined with StoreVariant's best-effort accounting
insert (warns and continues on failure), a long-running process could
accumulate untracked disk usage past cache_max_bytes indefinitely --
the disk-exhaustion failure mode issue #51 exists to close -- with
recovery gated on a process restart.

evictionLoop now also runs a reconciliation pass on every periodic
ticker tick (the same interval eviction itself uses; reconciliation
walks the cache directories so it deliberately does not run on every
write-pressure wakeup, to stay off the per-store hot path). This
bounds unaccounted drift to at most one eviction interval regardless
of how long the process has been running.

TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup proves it:
introduces an untracked variant file only after startup reconciliation
has already completed and asserts a later periodic pass adopts it.
---
 internal/imgcache/eviction.go | 48 +++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 14 deletions(-)

diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go
index 7574822..cd2d6fe 100644
--- a/internal/imgcache/eviction.go
+++ b/internal/imgcache/eviction.go
@@ -399,10 +399,11 @@ func (c *Cache) notifyWritePressure() {
 }
 
 // StartEviction launches the background eviction goroutine, which
-// reconciles the database accounting with the cache directories once
-// at startup and then evicts to the configured limit on the given
-// periodic interval and on write-pressure notifications. It is a
-// no-op on a disabled cache or when already started.
+// reconciles the database accounting with the cache directories at
+// startup and again on every periodic tick thereafter, and evicts to
+// the configured limit on the given periodic interval and on
+// write-pressure notifications. It is a no-op on a disabled cache or
+// when already started.
 func (c *Cache) StartEviction(interval time.Duration) {
 	if c.disabled || c.evictionStarted {
 		return
@@ -433,10 +434,7 @@ func (c *Cache) evictionLoop(interval time.Duration) {
 
 	ctx := context.Background()
 
-	if err := c.reconcileAccounting(ctx); err != nil {
-		c.log.Warn("cache accounting reconciliation failed", "error", err)
-	}
-
+	c.runReconciliationPass(ctx)
 	c.runEvictionPass(ctx)
 
 	ticker := time.NewTicker(interval)
@@ -447,6 +445,16 @@ func (c *Cache) evictionLoop(interval time.Duration) {
 		case <-c.evictionStop:
 			return
 		case <-ticker.C:
+			// Reconciliation walks the cache directories, so it only
+			// runs on the periodic ticker rather than on every
+			// write-pressure wakeup, keeping it off the per-store hot
+			// path. Reusing the eviction interval itself (rather than a
+			// separate, longer one) is a deliberate choice: it is the
+			// simplest option that still bounds how long a store's
+			// best-effort accounting insert can stay silently
+			// unaccounted for to one interval, on a process that is
+			// already running this loop regardless.
+			c.runReconciliationPass(ctx)
 		case <-c.evictionPressure:
 		}
 
@@ -462,13 +470,25 @@ func (c *Cache) runEvictionPass(ctx context.Context) {
 	}
 }
 
+// runReconciliationPass runs one reconciliation pass, logging failures
+// instead of propagating them (the loop must keep running).
+func (c *Cache) runReconciliationPass(ctx context.Context) {
+	if err := c.reconcileAccounting(ctx); err != nil {
+		c.log.Warn("cache accounting reconciliation failed", "error", err)
+	}
+}
+
 // reconcileAccounting synchronizes the database size accounting with
-// the actual contents of the cache directories. It runs once when the
-// background evictor starts, off the request hot path: it adopts
-// variant files that predate the accounting table, drops accounting
-// rows whose files are missing, removes source blob files the database
-// does not know (and rows whose files are gone), and sweeps stale temp
-// files left behind by crashed writes.
+// the actual contents of the cache directories. It runs at startup and
+// again on every periodic eviction tick thereafter, 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
+// files are missing, removes source blob files the database does not
+// know (and rows whose files are gone), and sweeps stale temp files
+// left behind by crashed writes. Running it periodically, not just
+// once, bounds how long such drift can accumulate unaccounted for on a
+// long-running process to one eviction interval.
 func (c *Cache) reconcileAccounting(ctx context.Context) error {
 	if c.disabled {
 		return nil