Remove expired negative cache rows in the eviction loop (closes #244)

A negative cache row was deleted only when its source URL was requested
again after it expired, and CleanExpired was never called, so a client
asking for a new missing path each time grew the database without
limit. The eviction goroutine now calls CleanExpired at startup and on
every periodic tick, so a row stays at most NegativeTTL plus one
interval. The goroutine also starts with the disk cache disabled, where
reconciliation and eviction already do nothing, since the negative
cache is used either way.

Model: opus-5-5

TODO.md

@@ -38,6 +38,15 @@ P2: security: per-origin rate limiting
   commented entry under `example.com`. Getting Started and "Running under upaas"
   in `README.md` say that every image on an allowlisted host is served without a
   signature, so an allowlisted host must be one the operator controls.
+- 2026-10-15 expired negative cache rows are removed on a schedule (closes
+  #244): the row of a failed fetch, which makes pixa answer 502 for that source
+  URL for 5 minutes, was deleted only when the same URL was requested again
+  after that, so a client asking for a new missing path each time added rows
+  that stayed for good. The eviction goroutine now calls `CleanExpired` at
+  startup and on each periodic tick, every 5 minutes in pixad, so a row stays at
+  most about 10 minutes. It runs with the disk cache disabled
+  (`cache_max_bytes: 0`) too, where it does nothing else, as the negative cache
+  is used either way.
 - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side
   (closes #257): it scales the image to cover the requested size, and the side
   scaled past that size was held to no maximum, so `1x8192?fit=outside` of a

internal/handlers/handlers.go

@@ -124,7 +124,8 @@ func (s *Handlers) initImageService() error {
 	s.imgCache = cache
 
 	// Background eviction: startup reconciliation, then periodic and
-	// write-pressure passes. No-op when the disk cache is disabled.
+	// write-pressure passes, and the removal of expired negative cache
+	// 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

internal/imgcache/cache.go

@@ -50,8 +50,9 @@ type CacheConfig struct {
 
 	// DisableDiskCache turns the disk cache off entirely: no cache
 	// directories are created, lookups always miss, stores are
-	// no-ops, and no eviction machinery runs. The config layer sets
-	// this when the operator configures cache_max_bytes: 0.
+	// no-ops, and the eviction goroutine only removes the expired
+	// negative cache rows. The config layer sets this when the
+	// operator configures cache_max_bytes: 0.
 	DisableDiskCache bool
 
 	// Logger receives accounting and eviction log output. A nil
@@ -482,7 +483,10 @@ func (c *Cache) GetSourceContent(
 	return c.srcContent.LoadWithSize(contentHash)
 }
 
-// CleanExpired removes expired entries from the cache.
+// CleanExpired removes the negative cache rows past their expiry. The
+// eviction goroutine calls it at startup and on every periodic tick, so
+// a row stays at most NegativeTTL plus one eviction interval, whether or
+// not its URL is requested again.
 func (c *Cache) CleanExpired(ctx context.Context) error {
 	// Clean expired negative cache entries
 	_, err := c.db.ExecContext(ctx, `

internal/imgcache/eviction.go

@@ -437,14 +437,16 @@ func (c *Cache) notifyWritePressure() {
 }
 
 // StartEviction launches the background eviction goroutine, which
-// 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. The goroutine outlives the caller, so it runs
-// with its own context, which StopEviction cancels.
+// reconciles the database accounting with the cache directories and
+// removes the expired negative cache rows 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. On a
+// disabled cache it only removes the expired negative cache rows, as
+// the negative cache is used either way. It is a no-op when already
+// started. The goroutine outlives the caller, so it runs with its own
+// context, which StopEviction cancels.
 func (c *Cache) StartEviction(interval time.Duration) {
-	if c.disabled || c.evictionCancel != nil {
+	if c.evictionCancel != nil {
 		return
 	}
 
@@ -480,6 +482,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 	defer close(c.evictionDone)
 
 	c.runReconciliationPass(ctx)
+	c.runCleanExpired(ctx)
 	c.runEvictionPass(ctx)
 
 	ticker := time.NewTicker(interval)
@@ -500,6 +503,7 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 			// unaccounted for to one interval, on a process that is
 			// already running this loop regardless.
 			c.runReconciliationPass(ctx)
+			c.runCleanExpired(ctx)
 		case <-c.evictionPressure:
 		}
 
@@ -535,6 +539,20 @@ func (c *Cache) runReconciliationPass(ctx context.Context) {
 	}
 }
 
+// runCleanExpired runs CleanExpired, logging a failure instead of
+// propagating it (the loop must keep running). It does nothing once ctx
+// is cancelled.
+func (c *Cache) runCleanExpired(ctx context.Context) {
+	if ctx.Err() != nil {
+		return
+	}
+
+	err := c.CleanExpired(ctx)
+	if err != nil {
+		c.log.Warn("negative cache cleanup failed", "error", err)
+	}
+}
+
 // reconcileAccounting synchronizes the database size accounting with
 // the actual contents of the cache directories. It runs at startup and
 // again on every periodic eviction tick thereafter, off the request