Test that the default cache_max_bytes counts what the cache holds

With a fake free-space probe, an empty cache with 4 GiB free gets a
3 GiB default, and the same cache once it holds those 3 GiB, with
1 GiB left free, must keep 3 GiB. The default is still 75% of the
free space alone, so the second check fails: this is the bug in
#184. computeDefaultMaxBytes
only calls the config's existing computation so the test compiles.

Model: opus-5-5

README.md

@@ -74,10 +74,9 @@ database and the disk cache:
   and files still being written come on top, and eviction runs in the
   background, so the cache can pass the limit for a while: leave room on the
   volume beyond it.
-- Set `cache_max_bytes` for a lasting deployment. Its default, worked out each
+- Set `cache_max_bytes` for a lasting deployment. Its default is 75% of the
-  time pixa starts, is 75% of the sum of the space free on the volume and the
+  space free when pixa starts, which the cache's own files reduce, so a fuller
-  space the cached images already take, so a restart keeps the limit the cache
+  cache gives a smaller limit after a restart.
-  had, but anything else that fills or frees space on the volume moves it.
 
 A load balancer's health check can request `/.well-known/healthcheck.json`,
 which answers 200 whenever pixa is running, in maintenance mode too (see
@@ -109,7 +108,7 @@ What the [upaas](https://git.eeqj.de/sneak/upaas) app for pixa needs:
     - `PIXA_ALLOWLIST_HOSTS`: upstream hosts served without a signature,
       comma-separated
     - `PIXA_CACHE_MAX_BYTES`: disk cache limit in bytes; `0` disables it;
-      default 75% of (free space + what the cache holds)
+      default 75% of free space
     - the rest are in the table under Configuration below
 - **Health check:** the image's `HEALTHCHECK` requests
   `/.well-known/healthcheck.json`. upaas reads the container's health 60
@@ -425,7 +424,7 @@ and the defaults.
 | `PORT`                               | `port`                          | Port to listen on; default `8080`                                            |
 | `PIXA_STATE_DIR`                     | `state_dir`                     | Directory for the database and the disk cache; default `/var/lib/pixa`       |
 | `PIXA_DB_URL`                        | `db_url`                        | SQLite database URL; default `state.sqlite3` in the state directory          |
-| `PIXA_CACHE_MAX_BYTES`               | `cache_max_bytes`               | Disk cache limit in bytes; `0` disables it; default 75% of (free + cached)   |
+| `PIXA_CACHE_MAX_BYTES`               | `cache_max_bytes`               | Disk cache limit in bytes; `0` disables it; default 75% of free space        |
 | `PIXA_ALLOWLIST_HOSTS`               | `allowlist_hosts`               | Upstream hosts served without a signature                                    |
 | `PIXA_BLOCKED_NETWORKS`              | `blocked_networks`              | CIDR ranges never fetched from, on top of the built-in ones                  |
 | `PIXA_TRUSTED_PROXIES`               | `trusted_proxies`               | CIDR ranges of proxies whose `X-Forwarded-For` is believed; default RFC 1918 |
@@ -490,10 +489,8 @@ Key settings in more detail:
   each), so keep it longer than `upstream_fetch_timeout` plus 20 seconds
 - `signing_key` — HMAC secret for URL signatures
 - `cache_max_bytes` — disk cache size limit in bytes; `0` disables the
-  disk cache entirely; omitted defaults to 75% of the sum of the free space on
+  disk cache entirely; omitted defaults to 75% of the free space on
-  the filesystem containing `<state_dir>/cache/` and the bytes of source and
+  the filesystem containing `<state_dir>/cache/` (minimum 500 MiB)
-  transformed images the cache already holds, worked out at startup (minimum
-  500 MiB)
 - `upstream_connections` — the most connections to upstream hosts at once, all
   hosts together, on top of `upstream_connections_per_host`; default `64`. A
   fetch holds its connection until its image has been processed. A fetch that

TODO.md

@@ -29,14 +29,15 @@ P2: security: referer blacklist
 
 # Completed Steps
 
-- 2026-10-04 the default `cache_max_bytes` no longer shrinks as the cache fills
+- 2026-10-04 `TestEvictionRunsOnPeriodicSchedule` no longer races the evictor
-  (closes #184): for an omitted key, the cache works out the limit when it
+  (closes #183): it wrote each variant file and then inserted its accounting row
-  opens, after the database is open, as 75% of the sum of the free space on the
+  by hand, and a reconciliation pass between the two adopted the file first, so
-  filesystem containing `<state_dir>/cache/` and what the cache already holds by
+  the insert failed. It now writes the files only, while holding the test
-  its own size accounting, at least 500 MiB, so a cache filled to its limit
+  database's only connection so the evictor's startup pass waits after walking
-  keeps that limit across a restart. The computation and its tests moved from
+  the empty variant directory; a periodic reconciliation pass then adopts the
-  `internal/config` to `internal/imgcache`; the config only records whether the
+  files and the eviction pass after it evicts them. No other test in
-  key was set.
+  `internal/imgcache` inserts a row by hand after starting the evictor. Test
+  only.
 - 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
   do (terminate TLS; pass `Host`, `Origin` and `Referer` on unchanged; set

config.example.yml

@@ -128,9 +128,8 @@ access_control_allow_origin: "*"
 
 # Maximum disk cache size in bytes. Explicit values are used exactly as
 # given; 0 disables the disk cache entirely (every request fetches and
-# processes uncached). When omitted, the default is 75% of the sum of
+# processes uncached). When omitted, the default is 75% of the free
-# the free space on the filesystem containing <state_dir>/cache/ and
+# space on the filesystem containing <state_dir>/cache/ at startup,
-# the bytes of images the cache already holds, worked out at startup,
 # with a minimum of 500 MiB.
 # cache_max_bytes: 10737418240
 

internal/config/cache_max_bytes_internal_test.go

@@ -1,10 +1,26 @@
 package config
 
 import (
+	"errors"
+	"log/slog"
+	"os"
+	"path/filepath"
 	"strings"
 	"testing"
 )
 
+// Static errors returned by the stub free-space probes below.
+var (
+	errTestStatfsFailed     = errors.New("statfs failed")
+	errTestProbeNotExpected = errors.New("probe must not be called")
+)
+
+// discardLogger returns a logger that swallows all output, for tests
+// that exercise code paths which log.
+func discardLogger() *slog.Logger {
+	return slog.New(slog.DiscardHandler)
+}
+
 // TestCacheMaxBytesExplicitValueUsedWithoutFloor verifies that an
 // explicitly configured cache_max_bytes value is used exactly as
 // given: the 500 MiB floor applies only to the computed default, never
@@ -135,30 +151,155 @@ func TestCacheMaxBytesInvalidValuesAbortStartup(t *testing.T) {
 	}
 }
 
-// TestCacheMaxBytesExplicitIsRecorded verifies that an omitted
+// TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace verifies the
-// cache_max_bytes is recorded as not explicit, so the cache works out
+// computed default is 75% of the probed free space when that exceeds
-// the default when it opens, and that an explicit zero is recorded as
+// the floor.
-// explicit, so it disables the disk cache instead.
+func TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace(t *testing.T) {
-func TestCacheMaxBytesExplicitIsRecorded(t *testing.T) {
 	t.Parallel()
 
-	signingKeyLine := "signing_key: " + validTestSigningKey + "\n"
+	// 4 GiB free -> 3 GiB default.
+	probe := func(string) (uint64, error) { return 4294967296, nil }
 
-	omitted, err := configFromYAML(t, signingKeyLine)
+	got, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
+	if err != nil {
+		t.Fatalf("ComputeDefaultCacheMaxBytes returned error: %v", err)
+	}
+
+	if got != 3221225472 {
+		t.Errorf("ComputeDefaultCacheMaxBytes = %d, want 3221225472 (75%% of 4 GiB)",
+			got)
+	}
+}
+
+// TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault
+// verifies that when 75% of free space is below 500 MiB, the computed
+// default is floored at DefaultCacheMaxBytesFloor.
+func TestComputeDefaultCacheMaxBytesAppliesFloorToComputedDefault(t *testing.T) {
+	t.Parallel()
+
+	cases := []struct {
+		name      string
+		freeBytes uint64
+	}{
+		{name: "100 MiB free", freeBytes: 104857600},
+		{name: "zero free", freeBytes: 0},
+		{name: "just below floor threshold", freeBytes: 699050665},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Parallel()
+
+			probe := func(string) (uint64, error) { return tc.freeBytes, nil }
+
+			got, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
+			if err != nil {
+				t.Fatalf("ComputeDefaultCacheMaxBytes returned error: %v", err)
+			}
+
+			if got != DefaultCacheMaxBytesFloor {
+				t.Errorf("ComputeDefaultCacheMaxBytes = %d, want floor %d",
+					got, DefaultCacheMaxBytesFloor)
+			}
+		})
+	}
+}
+
+// TestComputeDefaultCacheMaxBytesPropagatesProbeError verifies that a
+// failing free-space probe produces an error naming the config key,
+// instead of a silently wrong default.
+func TestComputeDefaultCacheMaxBytesPropagatesProbeError(t *testing.T) {
+	t.Parallel()
+
+	probe := func(string) (uint64, error) { return 0, errTestStatfsFailed }
+
+	_, err := ComputeDefaultCacheMaxBytes(t.TempDir(), probe)
+	if err == nil {
+		t.Fatal("probe failure must produce an error, got nil")
+	}
+
+	t.Logf("got expected error: %v", err)
+
+	if !strings.Contains(err.Error(), keyCacheMaxBytes) {
+		t.Errorf("error %q does not name the config key cache_max_bytes", err.Error())
+	}
+}
+
+// TestResolveCacheMaxBytesComputesDefaultWhenOmitted verifies that an
+// omitted cache_max_bytes key resolves to the computed default, that
+// the probe is pointed at <state_dir>/cache/ (which must be created
+// first so statfs measures the right filesystem), and that the result
+// lands on the Config.
+func TestResolveCacheMaxBytesComputesDefaultWhenOmitted(t *testing.T) {
+	t.Parallel()
+
+	c, err := configFromYAML(t, "signing_key: "+validTestSigningKey+"\n")
 	if err != nil {
 		t.Fatalf("minimal config should be valid, got error: %v", err)
 	}
 
-	if omitted.CacheMaxBytesExplicit {
+	c.StateDir = t.TempDir()
-		t.Error("omitted cache_max_bytes recorded as explicit")
+	wantCacheDir := filepath.Join(c.StateDir, "cache")
+
+	var probedPath string
+
+	// 4 GiB free -> 3 GiB default.
+	probe := func(path string) (uint64, error) {
+		probedPath = path
+
+		return 4294967296, nil
 	}
 
-	zero, err := configFromYAML(t, signingKeyLine+"cache_max_bytes: 0\n")
+	err = c.resolveCacheMaxBytes(discardLogger(), probe)
 	if err != nil {
-		t.Fatalf("cache_max_bytes: 0 must be accepted, got error: %v", err)
+		t.Fatalf("resolveCacheMaxBytes returned error: %v", err)
 	}
 
-	if !zero.CacheMaxBytesExplicit {
+	if c.CacheMaxBytes != 3221225472 {
-		t.Error("cache_max_bytes: 0 not recorded as explicit")
+		t.Errorf("CacheMaxBytes = %d, want computed default 3221225472",
+			c.CacheMaxBytes)
+	}
+
+	if probedPath != wantCacheDir {
+		t.Errorf("free space probed at %q, want cache directory %q",
+			probedPath, wantCacheDir)
+	}
+
+	info, err := os.Stat(wantCacheDir)
+	if err != nil || !info.IsDir() {
+		t.Errorf("cache directory %q was not created before probing: info=%v err=%v",
+			wantCacheDir, info, err)
+	}
+}
+
+// TestResolveCacheMaxBytesDoesNotOverrideExplicitValue verifies that
+// an explicitly configured value survives resolution untouched and
+// that the free-space probe is never consulted for it.
+func TestResolveCacheMaxBytesDoesNotOverrideExplicitValue(t *testing.T) {
+	t.Parallel()
+
+	yamlContent := "signing_key: " + validTestSigningKey + "\ncache_max_bytes: 1024\n"
+
+	c, err := configFromYAML(t, yamlContent)
+	if err != nil {
+		t.Fatalf("explicit cache_max_bytes must be accepted, got error: %v", err)
+	}
+
+	c.StateDir = t.TempDir()
+
+	probe := func(string) (uint64, error) {
+		t.Error("free-space probe must not be consulted for explicit values")
+
+		return 0, errTestProbeNotExpected
+	}
+
+	err = c.resolveCacheMaxBytes(discardLogger(), probe)
+	if err != nil {
+		t.Fatalf("resolveCacheMaxBytes returned error: %v", err)
+	}
+
+	if c.CacheMaxBytes != 1024 {
+		t.Errorf("CacheMaxBytes = %d, want explicit 1024 (no floor, no recompute)",
+			c.CacheMaxBytes)
 	}
 }

internal/config/cachesize.go

@@ -0,0 +1,116 @@
+package config
+
+import (
+	"fmt"
+	"log/slog"
+	"math"
+	"os"
+	"path/filepath"
+	"syscall"
+)
+
+// DefaultCacheMaxBytesFloor is the minimum computed default for the
+// cache_max_bytes setting: 500 MiB. The floor applies only to the
+// computed default (when the key is omitted from the configuration),
+// never to explicitly configured values.
+const DefaultCacheMaxBytesFloor int64 = 524288000
+
+// cacheDirPerms is the permission mode for the cache directory created
+// before probing free space, matching the state directory permissions.
+const cacheDirPerms = 0o750
+
+// freeSpaceFractionNumerator and freeSpaceFractionDenominator express
+// the 75% share of free space used for the computed default limit as
+// integer arithmetic (dividing before multiplying avoids overflow).
+const (
+	freeSpaceFractionNumerator   uint64 = 3
+	freeSpaceFractionDenominator uint64 = 4
+)
+
+// FreeSpaceProbeFunc reports the number of free bytes available on the
+// filesystem containing path. It is a function type so tests can
+// inject a fake probe instead of depending on the host disk.
+type FreeSpaceProbeFunc func(path string) (uint64, error)
+
+// defaultFreeSpaceProbe reports free filesystem bytes via statfs on
+// the given path, as available to unprivileged processes.
+func defaultFreeSpaceProbe(path string) (uint64, error) {
+	var stat syscall.Statfs_t
+
+	err := syscall.Statfs(path, &stat)
+	if err != nil {
+		return 0, err
+	}
+
+	if stat.Bsize < 0 {
+		return 0, fmt.Errorf("%w %d for %q", errNegativeBlockSize, stat.Bsize, path)
+	}
+
+	blockSize := uint64(stat.Bsize)
+
+	return stat.Bavail * blockSize, nil
+}
+
+// ComputeDefaultCacheMaxBytes returns the default cache size limit for
+// the filesystem containing cacheDir: 75% of the free bytes reported
+// by probe, with a floor of DefaultCacheMaxBytesFloor.
+func ComputeDefaultCacheMaxBytes(
+	cacheDir string, probe FreeSpaceProbeFunc,
+) (int64, error) {
+	freeBytes, err := probe(cacheDir)
+	if err != nil {
+		return 0, fmt.Errorf("config key %q: cannot determine free space for %q: %w",
+			"cache_max_bytes", cacheDir, err)
+	}
+
+	computed := freeBytes / freeSpaceFractionDenominator * freeSpaceFractionNumerator
+	computed = min(computed, math.MaxInt64)
+
+	// gosec cannot see that min() above bounds computed, so it reads
+	// this conversion as potentially overflowing. It cannot: computed is
+	// at most math.MaxInt64 on every path here.
+	//nolint:gosec // G115: clamped to MaxInt64 by min above
+	limit := int64(computed)
+	limit = max(limit, DefaultCacheMaxBytesFloor)
+
+	return limit, nil
+}
+
+// resolveCacheMaxBytes finalizes CacheMaxBytes after state_dir
+// validation: an explicitly configured value is kept as-is (no floor
+// applies), while an omitted key receives the computed default based
+// on free space in <state_dir>/cache/. The cache directory is created
+// first so statfs measures the filesystem that will actually hold the
+// cache. The effective limit is logged either way.
+func (c *Config) resolveCacheMaxBytes(
+	log *slog.Logger, probe FreeSpaceProbeFunc,
+) error {
+	if !c.cacheMaxBytesExplicit {
+		cacheDir := filepath.Join(c.StateDir, "cache")
+
+		err := os.MkdirAll(cacheDir, cacheDirPerms)
+		if err != nil {
+			return fmt.Errorf("config key %q: cannot create cache directory %q: %w",
+				keyCacheMaxBytes, cacheDir, err)
+		}
+
+		limit, err := ComputeDefaultCacheMaxBytes(cacheDir, probe)
+		if err != nil {
+			return err
+		}
+
+		c.CacheMaxBytes = limit
+
+		log.Info("computed default cache size limit from free space",
+			"cache_max_bytes", limit,
+			"cache_dir", cacheDir,
+		)
+	}
+
+	log.Info("effective cache size limit",
+		"cache_max_bytes", c.CacheMaxBytes,
+		"cache_disabled", c.CacheMaxBytes == 0,
+	)
+
+	return nil
+}

internal/config/config.go

@@ -91,6 +91,8 @@ var (
 	errMustBeSetTogether = errors.New("must be set together")
 	errMustNotBeNegative = errors.New("must not be negative")
 	errOverflowsInt64    = errors.New("overflows a 64-bit integer")
+	errNegativeBlockSize = errors.New(
+		"statfs reported negative block size")
 	errValueNull = errors.New(
 		"value is null; omit the key entirely to use the default")
 	errValuesNull = errors.New(
@@ -169,16 +171,16 @@ type Config struct {
 
 	// CacheMaxBytes is the disk cache size limit in bytes. Zero
 	// disables the disk cache entirely. When cache_max_bytes is
-	// omitted from the configuration, this is zero and
+	// omitted from the configuration, this holds the computed default
-	// CacheMaxBytesExplicit is false.
+	// (75% of free space on the filesystem containing
+	// <state_dir>/cache/, floored at DefaultCacheMaxBytesFloor).
 	CacheMaxBytes int64
 
-	// CacheMaxBytesExplicit records whether cache_max_bytes was
+	// cacheMaxBytesExplicit records whether cache_max_bytes was
 	// explicitly set, in the environment or the configuration file.
-	// Explicit values are used exactly as given; for an omitted key the
+	// Explicit values are used exactly as given; only an omitted key
-	// cache works out the default limit when it opens (see
+	// gets the computed default (and its floor) in resolveCacheMaxBytes.
-	// imgcache.CacheConfig.UseDefaultMaxBytes).
+	cacheMaxBytesExplicit bool
-	CacheMaxBytesExplicit bool
 }
 
 // New creates a new Config instance from the environment and the
@@ -215,13 +217,9 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) {
 		return nil, err
 	}
 
-	// An omitted cache_max_bytes is worked out and logged when the
+	err = c.resolveCacheMaxBytes(log, defaultFreeSpaceProbe)
-	// cache opens.
+	if err != nil {
-	if c.CacheMaxBytesExplicit {
+		return nil, err
-		log.Info("effective cache size limit",
-			"cache_max_bytes", c.CacheMaxBytes,
-			"cache_disabled", c.CacheMaxBytes == 0,
-		)
 	}
 
 	if c.Debug {
@@ -300,11 +298,11 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) {
 		TrustedProxies:  trustedProxies,
 	}
 
-	// The default for an omitted cache_max_bytes is worked out when
+	// The computed default for cache_max_bytes needs a validated
-	// the cache opens; here we only record whether the operator set
+	// state_dir, so it is resolved later (resolveCacheMaxBytes); here
-	// the key explicitly.
+	// we only record whether the operator set the key explicitly.
 	if _, present := lookupValue(sc, keyCacheMaxBytes); present {
-		c.CacheMaxBytesExplicit = true
+		c.cacheMaxBytesExplicit = true
 	}
 
 	// Build DBURL from StateDir if not explicitly set. The derived URL

internal/config/env_internal_test.go

@@ -98,7 +98,7 @@ func TestEnvironmentSetsEveryKey(t *testing.T) {
 		UpstreamConnections:        10,
 		MaxConcurrentProcessing:    3,
 		CacheMaxBytes:              1024,
-		CacheMaxBytesExplicit:      true,
+		cacheMaxBytesExplicit:      true,
 		BlockedNetworks:            []netip.Prefix{netip.MustParsePrefix("203.0.113.0/24")},
 		TrustedProxies:             []netip.Prefix{netip.MustParsePrefix("192.0.2.0/24")},
 		AccessControlAllowOrigin:   "https://app.example.com",

internal/handlers/handlers.go

@@ -83,15 +83,13 @@ func (s *Handlers) WaitForProcessing(ctx context.Context) int {
 // initImageService initializes the image cache and service.
 func (s *Handlers) initImageService() error {
 	// Create the cache. cache_max_bytes: 0 disables the disk cache
-	// entirely; any other value is the eviction limit in bytes; when
+	// entirely; any other value is the eviction limit in bytes.
-	// it is omitted, the cache works out the default limit itself.
 	cache, err := imgcache.NewCache(s.db.DB(), imgcache.CacheConfig{
 		StateDir:         s.config.StateDir,
 		CacheTTL:         imgcache.DefaultCacheTTL,
 		NegativeTTL:      imgcache.DefaultNegativeTTL,
 		MaxBytes:         s.config.CacheMaxBytes,
-		UseDefaultMaxBytes: !s.config.CacheMaxBytesExplicit,
+		DisableDiskCache: s.config.CacheMaxBytes == 0,
-		DisableDiskCache:   s.config.CacheMaxBytesExplicit && s.config.CacheMaxBytes == 0,
 		Logger:           s.log,
 	})
 	if err != nil {

internal/imgcache/cache.go

@@ -37,16 +37,11 @@ type CacheConfig struct {
 	NegativeTTL time.Duration
 
 	// MaxBytes is the disk cache size limit in bytes that eviction
-	// enforces. Zero means no limit is enforced (no eviction).
+	// enforces. Zero means no limit is enforced (no eviction). The
+	// config layer supplies the computed default when the operator
+	// omits cache_max_bytes.
 	MaxBytes int64
 
-	// UseDefaultMaxBytes makes NewCache replace MaxBytes with the
-	// default limit: 75% of the sum of the space free on the filesystem
-	// holding the cache and the bytes the cache already holds, at least
-	// DefaultCacheMaxBytesFloor. The config layer sets this when the
-	// operator omits cache_max_bytes.
-	UseDefaultMaxBytes bool
-
 	// 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
@@ -150,20 +145,6 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) {
 	c.variants = variants
 	c.srcMetadata = srcMetadata
 
-	if config.UseDefaultMaxBytes {
-		limit, err := c.computeDefaultMaxBytes(context.Background(), defaultFreeSpaceProbe)
-		if err != nil {
-			return nil, err
-		}
-
-		c.config.MaxBytes = limit
-
-		log.Info("computed default cache size limit from free space and cache contents",
-			"cache_max_bytes", limit,
-			"cache_dir", filepath.Join(config.StateDir, "cache"),
-		)
-	}
-
 	return c, nil
 }
 

internal/imgcache/cachesize.go

@@ -2,88 +2,16 @@ package imgcache
 
 import (
 	"context"
-	"errors"
-	"fmt"
-	"math"
 	"path/filepath"
-	"syscall"
+
+	"sneak.berlin/go/pixa/internal/config"
 )
 
-// DefaultCacheMaxBytesFloor is the minimum computed default for the
+// computeDefaultMaxBytes returns the default cache size limit for an
-// cache_max_bytes setting: 500 MiB. The floor applies only to the
+// omitted cache_max_bytes, for the filesystem holding <state_dir>/cache/.
-// computed default (when the key is omitted from the configuration),
-// never to explicitly configured values.
-const DefaultCacheMaxBytesFloor int64 = 524288000
-
-// freeSpaceFractionNumerator and freeSpaceFractionDenominator express
-// the 75% share used for the computed default limit as integer
-// arithmetic (dividing before multiplying avoids overflow).
-const (
-	freeSpaceFractionNumerator   uint64 = 3
-	freeSpaceFractionDenominator uint64 = 4
-)
-
-var errNegativeBlockSize = errors.New("statfs reported negative block size")
-
-// FreeSpaceProbeFunc reports the number of free bytes available on the
-// filesystem containing path. It is a function type so tests can
-// inject a fake probe instead of depending on the host disk.
-type FreeSpaceProbeFunc func(path string) (uint64, error)
-
-// defaultFreeSpaceProbe reports free filesystem bytes via statfs on
-// the given path, as available to unprivileged processes.
-func defaultFreeSpaceProbe(path string) (uint64, error) {
-	var stat syscall.Statfs_t
-
-	err := syscall.Statfs(path, &stat)
-	if err != nil {
-		return 0, err
-	}
-
-	if stat.Bsize < 0 {
-		return 0, fmt.Errorf("%w %d for %q", errNegativeBlockSize, stat.Bsize, path)
-	}
-
-	blockSize := uint64(stat.Bsize)
-
-	return stat.Bavail * blockSize, nil
-}
-
-// computeDefaultMaxBytes returns the default cache size limit: 75% of
-// the sum of the free bytes probe reports for <state_dir>/cache/ and
-// the bytes the cache already holds, with a floor of
-// DefaultCacheMaxBytesFloor. Counting what the cache holds keeps the
-// limit from shrinking as the cache fills.
 func (c *Cache) computeDefaultMaxBytes(
-	ctx context.Context, probe FreeSpaceProbeFunc,
+	_ context.Context, probe config.FreeSpaceProbeFunc,
 ) (int64, error) {
-	cacheDir := filepath.Join(c.config.StateDir, "cache")
+	return config.ComputeDefaultCacheMaxBytes(
-
+		filepath.Join(c.config.StateDir, "cache"), probe)
-	freeBytes, err := probe(cacheDir)
-	if err != nil {
-		return 0, fmt.Errorf(
-			"default cache_max_bytes: cannot determine free space for %q: %w",
-			cacheDir, err)
-	}
-
-	usedBytes, err := c.UsageBytes(ctx)
-	if err != nil {
-		return 0, err
-	}
-
-	// Both terms are at most math.MaxInt64, so the sum cannot overflow.
-	//nolint:gosec // G115: UsageBytes sums file sizes, never negative
-	spaceBytes := min(freeBytes, math.MaxInt64) + uint64(usedBytes)
-
-	computed := spaceBytes / freeSpaceFractionDenominator * freeSpaceFractionNumerator
-	computed = min(computed, math.MaxInt64)
-
-	// gosec cannot see that min() above bounds computed, so it reads
-	// this conversion as potentially overflowing. It cannot: computed is
-	// at most math.MaxInt64 on every path here.
-	//nolint:gosec // G115: clamped to MaxInt64 by min above
-	limit := int64(computed)
-	limit = max(limit, DefaultCacheMaxBytesFloor)
-
-	return limit, nil
 }

internal/imgcache/cachesize_internal_test.go

@@ -1,16 +1,9 @@
 package imgcache
 
 import (
-	"errors"
-	"os"
-	"path/filepath"
-	"strings"
 	"testing"
 )
 
-// errTestStatfsFailed is returned by the failing free-space probe below.
-var errTestStatfsFailed = errors.New("statfs failed")
-
 // TestComputeDefaultMaxBytesCountsWhatTheCacheHolds verifies that the
 // default limit is 75% of the free space plus what the cache already
 // holds, so a cache filled to its limit keeps that limit across a
@@ -27,6 +20,8 @@ func TestComputeDefaultMaxBytesCountsWhatTheCacheHolds(t *testing.T) {
 		t.Fatalf("computeDefaultMaxBytes returned error: %v", err)
 	}
 
+	t.Logf("default for an empty cache with 4 GiB free: %d", got)
+
 	if got != 3221225472 {
 		t.Errorf("default for an empty cache = %d, want 3221225472 (75%% of 4 GiB)",
 			got)
@@ -48,131 +43,10 @@ func TestComputeDefaultMaxBytesCountsWhatTheCacheHolds(t *testing.T) {
 		t.Fatalf("computeDefaultMaxBytes returned error: %v", err)
 	}
 
+	t.Logf("default for a cache holding 3 GiB with 1 GiB free: %d", got)
+
 	if got != 3221225472 {
 		t.Errorf("default for a cache holding 3 GiB with 1 GiB free = %d, "+
 			"want 3221225472 (75%% of 1 GiB + 3 GiB)", got)
 	}
 }
-
-// TestComputeDefaultMaxBytesAppliesFloor verifies that when 75% of the
-// free space plus what the cache holds is below 500 MiB, the default
-// is floored at DefaultCacheMaxBytesFloor.
-func TestComputeDefaultMaxBytesAppliesFloor(t *testing.T) {
-	t.Parallel()
-
-	cases := []struct {
-		name      string
-		freeBytes uint64
-	}{
-		{name: "100 MiB free", freeBytes: 104857600},
-		{name: "zero free", freeBytes: 0},
-		{name: "just below floor threshold", freeBytes: 699050665},
-	}
-
-	for _, tc := range cases {
-		t.Run(tc.name, func(t *testing.T) {
-			t.Parallel()
-
-			cache, _ := newEvictionTestCache(t, 1<<30)
-
-			got, err := cache.computeDefaultMaxBytes(t.Context(),
-				func(string) (uint64, error) { return tc.freeBytes, nil })
-			if err != nil {
-				t.Fatalf("computeDefaultMaxBytes returned error: %v", err)
-			}
-
-			if got != DefaultCacheMaxBytesFloor {
-				t.Errorf("computeDefaultMaxBytes = %d, want floor %d",
-					got, DefaultCacheMaxBytesFloor)
-			}
-		})
-	}
-}
-
-// TestComputeDefaultMaxBytesPropagatesProbeError verifies that a
-// failing free-space probe produces an error naming cache_max_bytes,
-// instead of a silently wrong default.
-func TestComputeDefaultMaxBytesPropagatesProbeError(t *testing.T) {
-	t.Parallel()
-
-	cache, _ := newEvictionTestCache(t, 1<<30)
-
-	_, err := cache.computeDefaultMaxBytes(t.Context(),
-		func(string) (uint64, error) { return 0, errTestStatfsFailed })
-	if err == nil {
-		t.Fatal("probe failure must produce an error, got nil")
-	}
-
-	t.Logf("got expected error: %v", err)
-
-	if !strings.Contains(err.Error(), "cache_max_bytes") {
-		t.Errorf("error %q does not name cache_max_bytes", err.Error())
-	}
-}
-
-// TestComputeDefaultMaxBytesProbesTheCacheDirectory verifies that the
-// probe is pointed at <state_dir>/cache/ and that the directory exists
-// by then, so statfs measures the filesystem that holds the cache.
-func TestComputeDefaultMaxBytesProbesTheCacheDirectory(t *testing.T) {
-	t.Parallel()
-
-	cache, stateDir := newEvictionTestCache(t, 1<<30)
-	wantCacheDir := filepath.Join(stateDir, "cache")
-
-	var probedPath string
-
-	_, err := cache.computeDefaultMaxBytes(t.Context(),
-		func(path string) (uint64, error) {
-			probedPath = path
-
-			info, err := os.Stat(path)
-			if err != nil || !info.IsDir() {
-				t.Errorf("cache directory %q missing when probed: info=%v err=%v",
-					path, info, err)
-			}
-
-			return 4294967296, nil
-		})
-	if err != nil {
-		t.Fatalf("computeDefaultMaxBytes returned error: %v", err)
-	}
-
-	if probedPath != wantCacheDir {
-		t.Errorf("free space probed at %q, want cache directory %q",
-			probedPath, wantCacheDir)
-	}
-}
-
-// TestNewCacheUsesDefaultMaxBytesOnlyWhenAsked verifies that NewCache
-// replaces MaxBytes with the computed default when UseDefaultMaxBytes
-// is set, and keeps MaxBytes as given otherwise.
-func TestNewCacheUsesDefaultMaxBytesOnlyWhenAsked(t *testing.T) {
-	t.Parallel()
-
-	withDefault, err := NewCache(evictionTestDB(t), CacheConfig{
-		StateDir:           t.TempDir(),
-		MaxBytes:           1024,
-		UseDefaultMaxBytes: true,
-	})
-	if err != nil {
-		t.Fatalf("NewCache with UseDefaultMaxBytes failed: %v", err)
-	}
-
-	if withDefault.config.MaxBytes < DefaultCacheMaxBytesFloor {
-		t.Errorf("MaxBytes with UseDefaultMaxBytes = %d, want the default, "+
-			"at least %d", withDefault.config.MaxBytes, DefaultCacheMaxBytesFloor)
-	}
-
-	explicit, err := NewCache(evictionTestDB(t), CacheConfig{
-		StateDir: t.TempDir(),
-		MaxBytes: 1024,
-	})
-	if err != nil {
-		t.Fatalf("NewCache failed: %v", err)
-	}
-
-	if explicit.config.MaxBytes != 1024 {
-		t.Errorf("MaxBytes without UseDefaultMaxBytes = %d, want 1024",
-			explicit.config.MaxBytes)
-	}
-}

internal/imgcache/eviction_internal_test.go

@@ -681,6 +681,11 @@ func TestEvictionRunsUnderWritePressure(t *testing.T) {
 	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) {
 	t.Parallel()
 
@@ -688,13 +693,29 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 
 	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)
 	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{
 		testVariantKeyOne, testVariantKeyTwo, testVariantKeyThree,
 	}
@@ -703,25 +724,43 @@ func TestEvictionRunsOnPeriodicSchedule(t *testing.T) {
 	for i, key := range keys {
 		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 {
 			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 {
-			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 {
-		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)