Run the reconciliation pass at startup and once a day (closes #250)

The pass makes a database query for every cached file and ran on every
eviction tick, so on a large cache one pass could outlast the tick and
the next started at once. It now runs on a timer of its own that starts
when a pass ends, a day long. Eviction and the removal of expired
negative cache rows keep the eviction interval.

Model: opus-5-5

README.md

@@ -131,34 +131,6 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
       the cache can grow to three quarters or more of the space they all have
       left, and to all of it when that is 500 MiB or less.
     - the rest are in the table under Configuration below
-- **Memory:** pixa can use up to `max_concurrent_processing` times 4.2 GiB for
-  the images it processes at once, plus `upstream_connections` times
-  `upstream_max_response_size` for the source images it has fetched that wait
-  for processing. 4.2 GiB is the most resident memory was measured to grow while
-  pixa, with pixad's libvips settings, processed one 8192x8192 source, the
-  largest it accepts, saving it at full size to each output format in turn, one
-  at a time. The sources were a JPEG, a GIF, a WebP, an 8-bit and a 16-bit PNG,
-  an 8-bit and a 12-bit AVIF, and an 8-bit, a 16-bit and a 32-bit floating-point
-  JPEG XL; memory grew most for the 16-bit JPEG XL, of random pixels, saved as
-  WebP. At the defaults that is half the CPUs pixa sees times 4.2 GiB, plus 64
-  times 50 MiB: on a host where pixa sees 48 CPUs, 24 times 4.2 GiB plus 3.1
-  GiB, about 104 GiB. Set `PIXA_MAX_CONCURRENT_PROCESSING` and
-  `PIXA_UPSTREAM_CONNECTIONS` so that this fits the memory the container may
-  use.
-- **Temporary files:** while pixa processes a large source, libvips writes its
-  decoded pixels to a file in `/tmp`. The file disappears from `/tmp` once it is
-  written, so a listing of `/tmp` does not show it, but it keeps its space, on
-  disk or, where `/tmp` is a tmpfs, in memory, until the image is done. For a
-  GIF, WebP, AVIF or JPEG XL source the decoded pixels are held in memory as
-  well as written to that file, and the 4.2 GiB above includes that memory. For
-  an 8192x8192 source the file was measured on pixad at 192 MiB for an 8-bit RGB
-  JPEG, 512 MiB for a 16-bit RGBA PNG or JPEG XL, and 1 GiB for a JPEG XL of
-  32-bit floating-point RGBA, the largest measured: up to 1 GiB per image
-  processed at once, 24 GiB at the defaults on a host where pixa sees 48 CPUs.
-  This `/tmp` is in the container's own filesystem, not on the volume at
-  `/var/lib/pixa`, and `cache_max_bytes` does not count it. The 4.2 GiB above
-  does not include these files; where `/tmp` is a tmpfs, they are memory on top
-  of it.
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck.json`. upaas reads the container's health 60 seconds
   after a deploy and marks the deploy failed unless it is `healthy`. The probe
@@ -196,7 +168,7 @@ references rather than using filesystem refcounting.
 Source and transformed image files, and the `.meta` file beside each transformed
 image, are written under a temporary name, synced to disk, and only then renamed
 into place, so a crash leaves no file under its own name with part of its data.
-When pixa starts, and every five minutes after, it removes each cached source or
+When pixa starts, and once a day after, it removes each cached source or
 transformed image whose file is empty, or not the size the database records for
 it, with a warning naming the file; the next request for it fetches the source
 or converts the image again.
@@ -594,7 +566,7 @@ tell. With no file, pixa uses the environment and the defaults.
 | `PIXA_ALLOW_HTTP`                    | `allow_http`                    | Allow plain-HTTP upstreams, for testing only; default `false`                |
 | `PIXA_UPSTREAM_CONNECTIONS_PER_HOST` | `upstream_connections_per_host` | Concurrent connections per upstream host; default `20`                       |
 | `PIXA_UPSTREAM_CONNECTIONS`          | `upstream_connections`          | Concurrent connections to all upstream hosts together; default `64`          |
-| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default half the CPUs, at least 1                  |
+| `PIXA_MAX_CONCURRENT_PROCESSING`     | `max_concurrent_processing`     | Images processed at once; default the number of CPUs                         |
 | `PIXA_IMAGE_REQUESTS_PER_MINUTE`     | `image_requests_per_minute`     | Image requests per client per minute; `0` turns it off; default `600`        |
 | `PIXA_UPSTREAM_FETCH_TIMEOUT`        | `upstream_fetch_timeout`        | Time allowed for one fetch from an upstream host; default `30s`              |
 | `PIXA_UPSTREAM_MAX_RESPONSE_SIZE`    | `upstream_max_response_size`    | Largest upstream response accepted, in bytes; default 50 MiB                 |
@@ -680,12 +652,10 @@ Key settings in more detail:
   does, and `downstream_timeout` has not ended first, the request is answered
   503 with the error `server busy, try again later`
 - `max_concurrent_processing` — the most images decoded and encoded at once;
-  default half the CPUs pixa can use (`GOMAXPROCS`, which follows a container's
+  default the number of CPUs pixa can use (`GOMAXPROCS`), which follows a
-  CPU limit), rounded down, and at least 1. A value that is not a whole number
+  container's CPU limit. A request that finds all of them in use waits up to 10
-  of at least 1 aborts startup. A request that finds all of them in use waits up
+  seconds for one to free up; if none does, and `downstream_timeout` has not
-  to 10 seconds for one to free up; if none does, and `downstream_timeout` has
+  ended first, it is answered 503 the same way
-  not ended first, it is answered 503 the same way. Each one can take a lot of
-  memory: see "Running under upaas"
 - `image_requests_per_minute` — how many requests one client may send to the
   image routes per minute (see the limit on them under Routes); default `600`.
   `0` turns the limit off, and a negative or non-integer value aborts startup.

TODO.md

@@ -30,18 +30,16 @@ P2: security: per-origin rate limiting
 
 # Completed Steps
 
-- 2026-10-15 `max_concurrent_processing` defaults to half the CPUs pixa sees,
+- 2026-10-15 the reconciliation pass runs at startup and then once a day (closes
-  rounded down, and at least 1 (closes #242): it defaulted to one image per CPU,
+  #250): it ran on every eviction tick, every 5 minutes in pixad, and makes a
-  and a container with no CPU limit sees every CPU of the host, so on a 48-CPU
+  database query for every cached file, on the one connection requests also use.
-  host pixa processed 48 images at once, each of which can take about 4.2 GiB.
+  On a large cache a pass could take longer than 5 minutes, and the next one
-  `config.DefaultMaxConcurrentProcessing` gives the default to the config and to
+  then started at once, so the passes never stopped. The next pass now starts a
-  `imageprocessor.New`; `PIXA_MAX_CONCURRENT_PROCESSING` still overrides it.
+  day after the previous one ends, so passes never run back to back. Eviction
-  "Running under upaas" in `README.md` gives the worst-case memory, measured,
+  and the removal of expired negative cache rows keep their 5-minute tick, and
-  and tells the operator to set `PIXA_MAX_CONCURRENT_PROCESSING` and
+  eviction also runs after each reconciliation pass. A variant file whose
-  `PIXA_UPSTREAM_CONNECTIONS` to fit the container's memory. It also gives the
+  database row failed to be written is now counted in the cache size up to about
-  file libvips writes to `/tmp` for each large source, up to 1 GiB, measured,
+  a day later, rather than 5 minutes.
-  which is outside `/var/lib/pixa` and `cache_max_bytes` and is memory where
-  `/tmp` is a tmpfs.
 - 2026-10-15 the canonical files are re-vendored from `sneak/prompts` commit
   `e3f79c4` (closes #221): `.dockerignore`, `.editorconfig`,
   `.gitea/workflows/check.yml`, `.gitignore`, `.golangci.yml`,
@@ -93,13 +91,13 @@ P2: security: per-origin rate limiting
   each transformed image's `.meta` file, are synced to disk before they are
   renamed into place, and their directory after; the `.meta` file is written
   before the image, and a failed directory sync removes the renamed file. The
-  reconciliation pass, at startup and every five minutes, removes a cached
+  reconciliation pass, at startup and once a day, removes a cached source or
-  source or transformed image whose file is empty, or not the size the database
+  transformed image whose file is empty, or not the size the database records
-  records for it, with its rows and a warning naming the file, so the next
+  for it, with its rows and a warning naming the file, so the next request
-  request fetches the source or converts the image again; requests compare no
+  fetches the source or converts the image again; requests compare no sizes.
-  sizes. Storing a source over a file of another size replaces that file. A
+  Storing a source over a file of another size replaces that file. A source that
-  source that fails to decode is not removed for that alone: a damaged upstream
+  fails to decode is not removed for that alone: a damaged upstream image would
-  image would then be fetched on every request.
+  then be fetched on every request.
 - 2026-10-15 a transcode stops once its request's `downstream_timeout` has
   ended, unless it has begun encoding (closes #243): the image processor looked
   at its context only while it waited for a processing slot, so an image was

configs/config.example.yml

@@ -108,11 +108,10 @@ upstream_connections_per_host: 20
 # downstream_timeout has ended first.
 upstream_connections: 64
 
-# Maximum number of images decoded and encoded at once (default: half the
+# Maximum number of images decoded and encoded at once (default: the
-# number of CPUs pixa can use, at least 1; the number of CPUs follows a
+# number of CPUs pixa can use, which follows a container's CPU limit). A
-# container's CPU limit). A request that finds none free waits up to 10
+# request that finds none free waits up to 10 seconds for one, and if none
-# seconds for one, and if none frees up it is answered 503, unless
+# frees up it is answered 503, unless downstream_timeout has ended first.
-# downstream_timeout has ended first.
 # max_concurrent_processing: 4
 
 # How many requests to the image routes (/v1/image/ and /v1/e/) one client

internal/config/concurrency_limits_internal_test.go

@@ -1,7 +1,6 @@
 package config
 
 import (
-	"fmt"
 	"runtime"
 	"testing"
 )
@@ -13,8 +12,8 @@ const (
 )
 
 // TestOmittedConcurrencyLimitsUseDefaults checks that an omitted
-// max_concurrent_processing is half the CPUs Go uses, and at least 1, and
+// max_concurrent_processing is the number of CPUs Go uses and an omitted
-// an omitted upstream_connections is 64.
+// upstream_connections is 64.
 func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 	t.Parallel()
 
@@ -23,9 +22,9 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 		t.Fatalf("minimal config should be valid, got error: %v", err)
 	}
 
-	if c.MaxConcurrentProcessing != max(runtime.GOMAXPROCS(0)/2, 1) {
+	if c.MaxConcurrentProcessing != runtime.GOMAXPROCS(0) {
-		t.Errorf("MaxConcurrentProcessing = %d, want %d, half the CPUs",
+		t.Errorf("MaxConcurrentProcessing = %d, want %d, one per CPU",
-			c.MaxConcurrentProcessing, max(runtime.GOMAXPROCS(0)/2, 1))
+			c.MaxConcurrentProcessing, runtime.GOMAXPROCS(0))
 	}
 
 	if c.UpstreamConnections != 64 {
@@ -33,36 +32,6 @@ func TestOmittedConcurrencyLimitsUseDefaults(t *testing.T) {
 	}
 }
 
-// TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne checks
-// that an omitted max_concurrent_processing is half the CPUs Go uses,
-// rounded down, and 1 when Go uses one CPU.
-//
-//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
-func TestOmittedMaxConcurrentProcessingIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
-	cpusBefore := runtime.GOMAXPROCS(0)
-
-	t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
-
-	for _, tc := range []struct{ cpus, want int }{
-		{1, 1}, {2, 1}, {3, 1}, {8, 4},
-	} {
-		runtime.GOMAXPROCS(tc.cpus)
-
-		c, err := configFromYAML(t, signingKeyLine)
-		if err != nil {
-			t.Fatalf("minimal config should be valid, got error: %v", err)
-		}
-
-		t.Logf("GOMAXPROCS %d: MaxConcurrentProcessing %d",
-			tc.cpus, c.MaxConcurrentProcessing)
-
-		if c.MaxConcurrentProcessing != tc.want {
-			t.Errorf("GOMAXPROCS %d: MaxConcurrentProcessing = %d, want %d",
-				tc.cpus, c.MaxConcurrentProcessing, tc.want)
-		}
-	}
-}
-
 // TestExplicitConcurrencyLimitsAreUsed checks that valid values for the
 // two limits are used as given.
 func TestExplicitConcurrencyLimitsAreUsed(t *testing.T) {
@@ -191,19 +160,3 @@ func TestConcurrencyLimitsFromEnvironment(t *testing.T) {
 		})
 	}
 }
-
-// TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup checks
-// that a PIXA_MAX_CONCURRENT_PROCESSING that is not a whole number of at
-// least 1, an empty one included, aborts startup naming the variable and
-// the key instead of taking the default.
-func TestInvalidMaxConcurrentProcessingFromEnvironmentAbortsStartup(t *testing.T) {
-	for _, value := range []string{"", " ", "0.5", "half", "-1", "4 images"} {
-		t.Run(fmt.Sprintf("value %q", value), func(t *testing.T) {
-			t.Setenv(testMaxConcurrentProcessingVar, value)
-
-			_, err := configFromYAML(t, signingKeyLine)
-			wantStartupError(t, err,
-				testMaxConcurrentProcessingVar, keyMaxConcurrentProcessing)
-		})
-	}
-}

internal/config/config.go

@@ -37,14 +37,6 @@ const (
 	DefaultImageRequestsPerMinute     = 600
 )
 
-// DefaultMaxConcurrentProcessing returns the most images processed at once
-// when max_concurrent_processing is omitted: half the CPUs Go uses
-// (runtime.GOMAXPROCS(0), which follows a container's CPU limit), rounded
-// down, and at least 1.
-func DefaultMaxConcurrentProcessing() int {
-	return max(runtime.GOMAXPROCS(0)/2, 1) //nolint:mnd // half the CPUs
-}
-
 // Configuration key names.
 const (
 	keyDebug                      = "debug"
@@ -305,8 +297,10 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 			keyUpstreamConnectionsPerHost, DefaultUpstreamConnectionsPerHost),
 		UpstreamConnections: loader.intVal(
 			keyUpstreamConnections, DefaultUpstreamConnections),
+		// Decoding and encoding are CPU-bound, so the default is one image
+		// per CPU Go uses, which follows a container's CPU limit.
 		MaxConcurrentProcessing: loader.intVal(
-			keyMaxConcurrentProcessing, DefaultMaxConcurrentProcessing()),
+			keyMaxConcurrentProcessing, runtime.GOMAXPROCS(0)),
 		UpstreamFetchTimeout: loader.durationVal(
 			keyUpstreamFetchTimeout, DefaultUpstreamFetchTimeout),
 		UpstreamMaxResponseSize: loader.int64Val(

internal/imageprocessor/imageprocessor.go

@@ -7,11 +7,11 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"runtime"
 	"sync"
 	"time"
 
 	"github.com/davidbyttow/govips/v2/vips"
-	"sneak.berlin/go/pixa/internal/config"
 )
 
 // vipsOnce ensures vips is initialized exactly once.
@@ -20,10 +20,11 @@ import (
 var vipsOnce sync.Once
 
 // initVips initializes libvips with quiet logging, one worker thread per
-// image and no operation cache. Each request decodes different source
+// image and no operation cache. Process already works on one image per CPU
-// bytes, so the operation cache would rarely be hit and would hold memory
+// by default, so more threads per image would only compete for the CPUs.
-// outside MaxConcurrentProcessing; repeated requests are served from
+// Each request decodes different source bytes, so the operation cache
-// pixa's disk cache instead.
+// would rarely be hit and would hold memory outside MaxConcurrentProcessing;
+// repeated requests are served from pixa's disk cache instead.
 func initVips() {
 	vipsOnce.Do(func() {
 		vips.LoggingSettings(nil, vips.LogLevelError)
@@ -178,7 +179,7 @@ type Params struct {
 	// If <= 0, DefaultMaxInputBytes is used.
 	MaxInputBytes int64
 	// MaxConcurrentProcessing is the most images processed at once.
-	// If <= 0, config.DefaultMaxConcurrentProcessing() is used.
+	// If <= 0, the number of CPUs Go uses (runtime.GOMAXPROCS(0)) is used.
 	MaxConcurrentProcessing int
 }
 
@@ -194,7 +195,7 @@ func New(params Params) *ImageProcessor {
 
 	maxConcurrentProcessing := params.MaxConcurrentProcessing
 	if maxConcurrentProcessing <= 0 {
-		maxConcurrentProcessing = config.DefaultMaxConcurrentProcessing()
+		maxConcurrentProcessing = runtime.GOMAXPROCS(0)
 	}
 
 	return &ImageProcessor{

internal/imageprocessor/max_concurrent_processing_internal_test.go

@@ -115,14 +115,14 @@ func waitForEntries(t *testing.T, entered <-chan struct{}, count int) {
 	}
 }
 
-func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
+func TestNewDefaultsMaxConcurrentProcessingToCPUs(t *testing.T) {
 	t.Parallel()
 
 	for _, limit := range []int{0, -1} {
 		proc := New(Params{MaxConcurrentProcessing: limit})
-		if got := cap(proc.processingSemaphore); got != max(runtime.GOMAXPROCS(0)/2, 1) {
+		if got := cap(proc.processingSemaphore); got != runtime.GOMAXPROCS(0) {
-			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, half the CPUs",
+			t.Errorf("MaxConcurrentProcessing %d: %d slots, want %d, one per CPU",
-				limit, got, max(runtime.GOMAXPROCS(0)/2, 1))
+				limit, got, runtime.GOMAXPROCS(0))
 		}
 	}
 
@@ -132,30 +132,6 @@ func TestNewDefaultsMaxConcurrentProcessingToHalfTheCPUs(t *testing.T) {
 	}
 }
 
-// TestNewDefaultIsHalfTheCPUsAndAtLeastOne checks that New, given no
-// MaxConcurrentProcessing, makes one slot for every two CPUs Go uses,
-// rounded down, and one slot when Go uses one CPU.
-//
-//nolint:paralleltest // it sets GOMAXPROCS, which other tests read
-func TestNewDefaultIsHalfTheCPUsAndAtLeastOne(t *testing.T) {
-	cpusBefore := runtime.GOMAXPROCS(0)
-
-	t.Cleanup(func() { runtime.GOMAXPROCS(cpusBefore) })
-
-	for _, tc := range []struct{ cpus, want int }{
-		{1, 1}, {2, 1}, {3, 1}, {8, 4},
-	} {
-		runtime.GOMAXPROCS(tc.cpus)
-
-		got := cap(New(Params{}).processingSemaphore)
-		t.Logf("GOMAXPROCS %d: %d slots", tc.cpus, got)
-
-		if got != tc.want {
-			t.Errorf("GOMAXPROCS %d: %d slots, want %d", tc.cpus, got, tc.want)
-		}
-	}
-}
-
 // TestProcessNeverExceedsMaxConcurrentProcessing starts more Process calls
 // than MaxConcurrentProcessing allows and holds the first ones inside
 // Process until the test lets them go. No more than the limit may be

internal/imgcache/eviction.go

@@ -18,6 +18,11 @@ import (
 // write-pressure wakeups triggered by stores.
 const DefaultEvictionInterval = 5 * time.Minute
 
+// reconciliationInterval is how long after one reconciliation pass ends
+// the next one starts. A pass reads a row for every cached file, so its
+// cost grows with the cache, and it runs this rarely for that reason.
+const reconciliationInterval = 24 * time.Hour
+
 // evictionBatchSize is how many LRU candidates of each class (variants
 // and source blobs) one eviction pass fetches from the database.
 const evictionBatchSize = 100
@@ -437,15 +442,17 @@ func (c *Cache) notifyWritePressure() {
 	}
 }
 
-// StartEviction launches the background eviction goroutine, which
+// StartEviction launches the background eviction goroutine. At startup
-// reconciles the database accounting with the cache directories and
+// it reconciles the database accounting with the cache directories,
-// removes the expired negative cache rows at startup and again on every
+// removes the expired negative cache rows and evicts to the configured
-// periodic tick thereafter, and evicts to the configured limit on the
+// limit. After that it reconciles again a day after each reconciliation
-// given periodic interval and on write-pressure notifications. On a
+// pass ends, removes the expired negative cache rows on every tick of
-// disabled cache it only removes the expired negative cache rows, as
+// the given interval, and evicts to the limit on every tick, after each
-// the negative cache is used either way. It is a no-op when already
+// reconciliation pass and on write-pressure notifications. On a disabled
-// started. The goroutine outlives the caller, so it runs with its own
+// cache it only removes the expired negative cache rows, as the negative
-// context, which StopEviction cancels.
+// 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.evictionCancel != nil {
 		return
@@ -486,6 +493,16 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 	c.runCleanExpired(ctx)
 	c.runEvictionPass(ctx)
 
+	// Reconciliation walks the cache directories and reads a row for
+	// every file, so it runs on a timer of its own rather than on the
+	// periodic tick or on write-pressure wakeups. The timer starts only
+	// once a pass has ended, so passes never run back to back, however
+	// long one takes. Running it daily, not just at startup, bounds how
+	// long a store's best-effort accounting insert can stay silently
+	// unaccounted for to about one day, plus the time the passes take.
+	reconciliationTimer := time.NewTimer(reconciliationInterval)
+	defer reconciliationTimer.Stop()
+
 	ticker := time.NewTicker(interval)
 	defer ticker.Stop()
 
@@ -493,17 +510,10 @@ func (c *Cache) evictionLoop(ctx context.Context, interval time.Duration) {
 		select {
 		case <-ctx.Done():
 			return
-		case <-ticker.C:
+		case <-reconciliationTimer.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)
+			reconciliationTimer.Reset(reconciliationInterval)
+		case <-ticker.C:
 			c.runCleanExpired(ctx)
 		case <-c.evictionPressure:
 		}
@@ -556,19 +566,18 @@ func (c *Cache) runCleanExpired(ctx context.Context) {
 
 // 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
+// then once a day, off the request hot path: it adopts variant files
-// hot path: it adopts variant files that predate the accounting table
+// that predate the accounting table (or whose accounting insert failed,
-// (or whose accounting insert failed, e.g. StoreVariant's best-effort
+// e.g. StoreVariant's best-effort insert under transient DB contention),
-// insert under transient DB contention), drops accounting rows whose
+// drops accounting rows whose files are missing, removes source blob
-// files are missing, removes source blob files the database does not
+// files the database does not know (and rows whose files are gone),
-// know (and rows whose files are gone), removes source and variant files
+// removes source and variant files that are empty or not the size their
-// that are empty or not the size their rows record, sweeps stale temp
+// rows record, sweeps stale temp files left behind by crashed writes,
-// files left behind by crashed writes, and last checks the total cache
+// and last checks the total cache usage against the tables. It reads the
-// usage against the tables. It reads the tables a page at a time.
+// tables a page at a time. Running it daily, not just once, bounds how
-// Running it periodically, not just once, bounds how long such drift can
+// long such drift can accumulate unaccounted for on a long-running
-// accumulate unaccounted for on a long-running process to one eviction
+// process to about one day. Once ctx is cancelled, it stops at the next
-// interval. Once ctx is cancelled, it stops at the next file or row and
+// file or row and returns ctx's error.
-// returns ctx's error.
 func (c *Cache) reconcileAccounting(ctx context.Context) error {
 	if c.disabled {
 		return nil

internal/imgcache/eviction_internal_test.go

@@ -9,7 +9,9 @@ import (
 	"log/slog"
 	"os"
 	"path/filepath"
+	"strconv"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -918,6 +920,84 @@ func TestPeriodicReconciliationAdoptsFileThatAppearsAfterStartup(t *testing.T) {
 	}
 }
 
+// TestEvictionTicksDoNotReconcile starts the eviction loop with an
+// interval of 10 ms. The startup pass reconciles before it removes the
+// expired negative cache rows, so once the expired row stored before the
+// start is gone, the startup reconciliation has made all its reads. The
+// test then waits for three periodic ticks, each seen when it removes an
+// expired row stored after the previous one went. The reconciliation
+// pass runs at startup and then once a day, so none of those ticks may
+// read a content table for reconciliation.
+func TestEvictionTicksDoNotReconcile(t *testing.T) {
+	t.Parallel()
+
+	cache, _ := newEvictionTestCache(t, 1<<30)
+
+	var reconciliationReads atomic.Int64
+
+	cache.reconciliationReadTestHook = func(int) { reconciliationReads.Add(1) }
+
+	insertExpiredNegativeCacheRow(t, cache, "/startup.jpg")
+
+	cache.StartEviction(10 * time.Millisecond)
+	defer func() { _ = cache.StopEviction(t.Context()) }()
+
+	waitForNegativeCacheRowRemoved(t, cache, "/startup.jpg")
+
+	startupReads := reconciliationReads.Load()
+	t.Logf("the startup pass made %d reconciliation reads", startupReads)
+
+	if startupReads == 0 {
+		t.Fatal("the startup pass made no reconciliation read")
+	}
+
+	for tick := range 3 {
+		path := "/tick-" + strconv.Itoa(tick) + ".jpg"
+
+		insertExpiredNegativeCacheRow(t, cache, path)
+		waitForNegativeCacheRowRemoved(t, cache, path)
+	}
+
+	tickReads := reconciliationReads.Load() - startupReads
+	t.Logf("three eviction ticks made %d reconciliation reads", tickReads)
+
+	if tickReads != 0 {
+		t.Errorf("three eviction ticks made %d reconciliation reads, want 0: "+
+			"the reconciliation pass ran on an eviction tick", tickReads)
+	}
+}
+
+// insertExpiredNegativeCacheRow adds a negative_cache row for sourcePath
+// that expired an hour ago.
+func insertExpiredNegativeCacheRow(t *testing.T, cache *Cache, sourcePath string) {
+	t.Helper()
+
+	_, err := cache.db.ExecContext(t.Context(), `
+		INSERT INTO negative_cache
+			(source_host, source_path, status_code, expires_at)
+		VALUES (?, ?, 404, datetime('now', '-1 hour'))
+	`, testHostExample, sourcePath)
+	if err != nil {
+		t.Fatalf("failed to insert an expired negative cache row: %v", err)
+	}
+}
+
+// waitForNegativeCacheRowRemoved waits up to five seconds for the
+// eviction loop to remove the negative_cache row for sourcePath.
+func waitForNegativeCacheRowRemoved(t *testing.T, cache *Cache, sourcePath string) {
+	t.Helper()
+
+	deadline := time.Now().Add(5 * time.Second)
+
+	for negativeCacheRowCount(t, cache, sourcePath) > 0 {
+		if time.Now().After(deadline) {
+			t.Fatalf("the expired negative cache row for %s was not removed", sourcePath)
+		}
+
+		time.Sleep(10 * time.Millisecond)
+	}
+}
+
 // TestStopEvictionInterruptsPassInProgress holds the test database's
 // only connection, so the startup reconciliation pass waits for it, and
 // checks that StopEviction stops that pass instead of waiting for the

internal/imgcache/service.go

@@ -53,7 +53,7 @@ type ServiceConfig struct {
 	// Allowlist is the list of hosts that don't require signatures
 	Allowlist []string
 	// MaxConcurrentProcessing is the most images processed at once; zero
-	// uses the image processor's default, half the CPUs
+	// uses the image processor's default, one per CPU
 	MaxConcurrentProcessing int
 	// Logger for logging
 	Logger *slog.Logger