diff --git a/README.md b/README.md index fe05438..75c0d1d 100644 --- a/README.md +++ b/README.md @@ -74,9 +74,10 @@ 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 is 75% of the - space free when pixa starts, which the cache's own files reduce, so a fuller - cache gives a smaller limit after a restart. +- Set `cache_max_bytes` for a lasting deployment. Its default, worked out each + time pixa starts, is 75% of the sum of the space free on the volume and the + space the cached images already take, so a restart keeps the limit the cache + 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 @@ -108,7 +109,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 + default 75% of (free space + what the cache holds) - 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 +426,7 @@ tell. With no file, pixa uses the environment 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 space | +| `PIXA_CACHE_MAX_BYTES` | `cache_max_bytes` | Disk cache limit in bytes; `0` disables it; default 75% of (free + cached) | | `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,8 +491,10 @@ 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 free space on - the filesystem containing `/cache/` (minimum 500 MiB) + disk cache entirely; omitted defaults to 75% of the sum of the free space on + the filesystem containing `/cache/` and the bytes of source and + 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 diff --git a/TODO.md b/TODO.md index 464cf22..3c1d04b 100644 --- a/TODO.md +++ b/TODO.md @@ -50,6 +50,14 @@ P2: security: referer blacklist share an identical line can end up one inside the other, which a rebase can do to an entry already on `next`. The Workflow above says to read the merged entries after every merge or rebase. +- 2026-10-04 the default `cache_max_bytes` no longer shrinks as the cache fills + (closes #184): for an omitted key, the cache works out the limit when it + opens, after the database is open, as 75% of the sum of the free space on the + filesystem containing `/cache/` and what the cache already holds by + its own size accounting, at least 500 MiB, so a cache filled to its limit + keeps that limit across a restart. The computation and its tests moved from + `internal/config` to `internal/imgcache`; the config only records whether the + key was set. - 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 diff --git a/config.example.yml b/config.example.yml index 2133dd9..be42aa4 100644 --- a/config.example.yml +++ b/config.example.yml @@ -128,8 +128,9 @@ 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 free -# space on the filesystem containing /cache/ at startup, +# processes uncached). When omitted, the default is 75% of the sum of +# the free space on the filesystem containing /cache/ and +# the bytes of images the cache already holds, worked out at startup, # with a minimum of 500 MiB. # cache_max_bytes: 10737418240 diff --git a/internal/config/cache_max_bytes_internal_test.go b/internal/config/cache_max_bytes_internal_test.go index 50933c2..6af1554 100644 --- a/internal/config/cache_max_bytes_internal_test.go +++ b/internal/config/cache_max_bytes_internal_test.go @@ -1,26 +1,10 @@ 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 @@ -151,155 +135,30 @@ func TestCacheMaxBytesInvalidValuesAbortStartup(t *testing.T) { } } -// TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace verifies the -// computed default is 75% of the probed free space when that exceeds -// the floor. -func TestComputeDefaultCacheMaxBytesUses75PercentOfFreeSpace(t *testing.T) { +// TestCacheMaxBytesExplicitIsRecorded verifies that an omitted +// cache_max_bytes is recorded as not explicit, so the cache works out +// the default when it opens, and that an explicit zero is recorded as +// explicit, so it disables the disk cache instead. +func TestCacheMaxBytesExplicitIsRecorded(t *testing.T) { t.Parallel() - // 4 GiB free -> 3 GiB default. - probe := func(string) (uint64, error) { return 4294967296, nil } + signingKeyLine := "signing_key: " + validTestSigningKey + "\n" - 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 /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") + omitted, err := configFromYAML(t, signingKeyLine) if err != nil { t.Fatalf("minimal config should be valid, got error: %v", err) } - c.StateDir = t.TempDir() - 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 + if omitted.CacheMaxBytesExplicit { + t.Error("omitted cache_max_bytes recorded as explicit") } - err = c.resolveCacheMaxBytes(discardLogger(), probe) + zero, err := configFromYAML(t, signingKeyLine+"cache_max_bytes: 0\n") if err != nil { - t.Fatalf("resolveCacheMaxBytes returned error: %v", err) + t.Fatalf("cache_max_bytes: 0 must be accepted, got error: %v", err) } - if c.CacheMaxBytes != 3221225472 { - 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) + if !zero.CacheMaxBytesExplicit { + t.Error("cache_max_bytes: 0 not recorded as explicit") } } diff --git a/internal/config/cachesize.go b/internal/config/cachesize.go deleted file mode 100644 index 8f7a014..0000000 --- a/internal/config/cachesize.go +++ /dev/null @@ -1,116 +0,0 @@ -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 /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 -} diff --git a/internal/config/config.go b/internal/config/config.go index a9e583f..a9f7b72 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -93,9 +93,7 @@ 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( + errValueNull = errors.New( "value is null; omit the key entirely to use the default") errValuesNull = errors.New( "value is null; omit a key entirely to use its default") @@ -171,18 +169,19 @@ type Config struct { // address, and an explicit list replaces the default. TrustedProxies []netip.Prefix - // 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 holds the computed default - // (75% of free space on the filesystem containing - // /cache/, floored at DefaultCacheMaxBytesFloor). + // CacheMaxBytes is the disk cache size limit in bytes. Only an + // explicit zero (CacheMaxBytesExplicit true) disables the disk + // cache. Zero with CacheMaxBytesExplicit false means + // cache_max_bytes was omitted, and the cache works out the default + // limit when it opens. 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; only an omitted key - // gets the computed default (and its floor) in resolveCacheMaxBytes. - cacheMaxBytesExplicit bool + // Explicit values are used exactly as given; for an omitted key the + // cache works out the default limit when it opens (see + // imgcache.CacheConfig.UseDefaultMaxBytes). + CacheMaxBytesExplicit bool } // New creates a new Config instance from the environment and the @@ -219,9 +218,13 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) { return nil, err } - err = c.resolveCacheMaxBytes(log, defaultFreeSpaceProbe) - if err != nil { - return nil, err + // An omitted cache_max_bytes is worked out and logged when the + // cache opens. + if c.CacheMaxBytesExplicit { + log.Info("effective cache size limit", + "cache_max_bytes", c.CacheMaxBytes, + "cache_disabled", c.CacheMaxBytes == 0, + ) } if c.Debug { @@ -300,11 +303,11 @@ func newFromSmartConfig(sc *smartconfig.Config) (*Config, error) { TrustedProxies: trustedProxies, } - // The computed default for cache_max_bytes needs a validated - // state_dir, so it is resolved later (resolveCacheMaxBytes); here - // we only record whether the operator set the key explicitly. + // The default for an omitted cache_max_bytes is worked out when + // the cache opens; here 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 diff --git a/internal/config/env_internal_test.go b/internal/config/env_internal_test.go index e61ffbf..dd90dfa 100644 --- a/internal/config/env_internal_test.go +++ b/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", diff --git a/internal/handlers/cache_max_bytes_internal_test.go b/internal/handlers/cache_max_bytes_internal_test.go new file mode 100644 index 0000000..af0a8ab --- /dev/null +++ b/internal/handlers/cache_max_bytes_internal_test.go @@ -0,0 +1,147 @@ +package handlers + +import ( + "os" + "path/filepath" + "testing" + + "go.uber.org/fx/fxtest" + + "sneak.berlin/go/pixa/internal/config" + "sneak.berlin/go/pixa/internal/database" + "sneak.berlin/go/pixa/internal/globals" + "sneak.berlin/go/pixa/internal/logger" +) + +// TestNewCacheConfigFromCacheMaxBytes checks the cache configuration +// built from cache_max_bytes: omitted, the cache works out the default +// limit; 0 turns the disk cache off; a positive value is the limit, +// unchanged. +func TestNewCacheConfigFromCacheMaxBytes(t *testing.T) { + t.Parallel() + + const oneGiB = 1 << 30 + + cases := []struct { + name string + cacheMaxBytes int64 + cacheMaxBytesExplicit bool + wantMaxBytes int64 + wantUseDefaultMaxBytes bool + wantDisableDiskCache bool + }{ + { + name: "cache_max_bytes omitted", + cacheMaxBytes: 0, + cacheMaxBytesExplicit: false, + wantMaxBytes: 0, + wantUseDefaultMaxBytes: true, + wantDisableDiskCache: false, + }, + { + name: "cache_max_bytes: 0", + cacheMaxBytes: 0, + cacheMaxBytesExplicit: true, + wantMaxBytes: 0, + wantUseDefaultMaxBytes: false, + wantDisableDiskCache: true, + }, + { + name: "cache_max_bytes: 1 GiB", + cacheMaxBytes: oneGiB, + cacheMaxBytesExplicit: true, + wantMaxBytes: oneGiB, + wantUseDefaultMaxBytes: false, + wantDisableDiskCache: false, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + cfg := &config.Config{ + CacheMaxBytes: tc.cacheMaxBytes, + CacheMaxBytesExplicit: tc.cacheMaxBytesExplicit, + } + + got := newCacheConfig(cfg, nil) + t.Logf("MaxBytes = %d, UseDefaultMaxBytes = %v, DisableDiskCache = %v", + got.MaxBytes, got.UseDefaultMaxBytes, got.DisableDiskCache) + + if got.MaxBytes != tc.wantMaxBytes { + t.Errorf("MaxBytes = %d, want %d", got.MaxBytes, tc.wantMaxBytes) + } + + if got.UseDefaultMaxBytes != tc.wantUseDefaultMaxBytes { + t.Errorf("UseDefaultMaxBytes = %v, want %v", + got.UseDefaultMaxBytes, tc.wantUseDefaultMaxBytes) + } + + if got.DisableDiskCache != tc.wantDisableDiskCache { + t.Errorf("DisableDiskCache = %v, want %v", + got.DisableDiskCache, tc.wantDisableDiskCache) + } + }) + } +} + +// TestDiskCacheOffOnlyForExplicitZeroCacheMaxBytes starts the handlers +// once with cache_max_bytes omitted and once with cache_max_bytes: 0, +// and checks by whether the cache directories were created that the +// disk cache is on in the first case and off in the second. +func TestDiskCacheOffOnlyForExplicitZeroCacheMaxBytes(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + cacheMaxBytesExplicit bool + wantDiskCache bool + }{ + {name: "cache_max_bytes omitted", cacheMaxBytesExplicit: false, wantDiskCache: true}, + {name: "cache_max_bytes: 0", cacheMaxBytesExplicit: true, wantDiskCache: false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + stateDir := t.TempDir() + cfg := &config.Config{ + SigningKey: testSigningKey, + StateDir: stateDir, + DBURL: "file:" + filepath.Join(stateDir, "state.sqlite3"), + CacheMaxBytes: 0, + CacheMaxBytesExplicit: tc.cacheMaxBytesExplicit, + } + + lc := fxtest.NewLifecycle(t) + + log, err := logger.New(lc, logger.Params{Globals: &globals.Globals{}}) + if err != nil { + t.Fatalf("logger.New() error = %v", err) + } + + db, err := database.New(lc, database.Params{Logger: log, Config: cfg}) + if err != nil { + t.Fatalf("database.New() error = %v", err) + } + + _, err = New(lc, Params{Logger: log, Database: db, Config: cfg}) + if err != nil { + t.Fatalf("New() error = %v", err) + } + + lc.RequireStart() + t.Cleanup(lc.RequireStop) + + _, err = os.Stat(filepath.Join(stateDir, "cache", "variants")) + gotDiskCache := err == nil + + if gotDiskCache != tc.wantDiskCache { + t.Errorf("cache directories created = %v, want %v", + gotDiskCache, tc.wantDiskCache) + } + }) + } +} diff --git a/internal/handlers/handlers.go b/internal/handlers/handlers.go index cecb5d7..489e3c3 100644 --- a/internal/handlers/handlers.go +++ b/internal/handlers/handlers.go @@ -80,18 +80,25 @@ func (s *Handlers) WaitForProcessing(ctx context.Context) int { return s.imgSvc.WaitForProcessing(ctx) } +// newCacheConfig builds the image cache's configuration from cfg. +// cache_max_bytes: 0 disables the disk cache entirely; any other value +// is the eviction limit in bytes; when it is omitted, the cache works +// out the default limit itself. +func newCacheConfig(cfg *config.Config, log *slog.Logger) imgcache.CacheConfig { + return imgcache.CacheConfig{ + StateDir: cfg.StateDir, + CacheTTL: imgcache.DefaultCacheTTL, + NegativeTTL: imgcache.DefaultNegativeTTL, + MaxBytes: cfg.CacheMaxBytes, + UseDefaultMaxBytes: !cfg.CacheMaxBytesExplicit, + DisableDiskCache: cfg.CacheMaxBytesExplicit && cfg.CacheMaxBytes == 0, + Logger: log, + } +} + // 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. - cache, err := imgcache.NewCache(s.db.DB(), imgcache.CacheConfig{ - StateDir: s.config.StateDir, - CacheTTL: imgcache.DefaultCacheTTL, - NegativeTTL: imgcache.DefaultNegativeTTL, - MaxBytes: s.config.CacheMaxBytes, - DisableDiskCache: s.config.CacheMaxBytes == 0, - Logger: s.log, - }) + cache, err := imgcache.NewCache(s.db.DB(), newCacheConfig(s.config, s.log)) if err != nil { return err } diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index 5cd47ae..3c31956 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -37,11 +37,16 @@ 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). The - // config layer supplies the computed default when the operator - // omits cache_max_bytes. + // enforces. Zero means no limit is enforced (no eviction). 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 @@ -95,6 +100,14 @@ type Cache struct { // NewCache creates a new cache instance. func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) { + return newCache(db, config, defaultFreeSpaceProbe) +} + +// newCache is NewCache with the free-space probe passed in, so tests +// can fake the free space the default limit is worked out from. +func newCache( + db *sql.DB, config CacheConfig, probe FreeSpaceProbeFunc, +) (*Cache, error) { log := config.Logger if log == nil { log = slog.Default() @@ -145,6 +158,20 @@ func NewCache(db *sql.DB, config CacheConfig) (*Cache, error) { c.variants = variants c.srcMetadata = srcMetadata + if config.UseDefaultMaxBytes { + limit, err := c.computeDefaultMaxBytes(context.Background(), probe) + 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 } diff --git a/internal/imgcache/cachesize.go b/internal/imgcache/cachesize.go new file mode 100644 index 0000000..437687e --- /dev/null +++ b/internal/imgcache/cachesize.go @@ -0,0 +1,89 @@ +package imgcache + +import ( + "context" + "errors" + "fmt" + "math" + "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 + +// 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 /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, +) (int64, error) { + cacheDir := filepath.Join(c.config.StateDir, "cache") + + 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 +} diff --git a/internal/imgcache/cachesize_internal_test.go b/internal/imgcache/cachesize_internal_test.go new file mode 100644 index 0000000..2416366 --- /dev/null +++ b/internal/imgcache/cachesize_internal_test.go @@ -0,0 +1,188 @@ +package imgcache + +import ( + "errors" + "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") +) + +// 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 +// restart instead of shrinking to 75% of the space left free. +func TestComputeDefaultMaxBytesCountsWhatTheCacheHolds(t *testing.T) { + t.Parallel() + + cache, _ := newEvictionTestCache(t, 1<<30) + + // Empty cache, 4 GiB free -> 3 GiB default. + got, err := cache.computeDefaultMaxBytes(t.Context(), + func(string) (uint64, error) { return 4294967296, nil }) + if err != nil { + 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) + } + + // The cache now holds those 3 GiB, which leaves 1 GiB free. + _, err = cache.db.ExecContext(t.Context(), + `INSERT INTO variant_content (cache_key, size_bytes, content_type) + VALUES (?, ?, ?)`, + string(testVariantKeyOne), 3221225472, testContentTypeWebP, + ) + if err != nil { + t.Fatalf("failed to insert variant accounting row: %v", err) + } + + got, err = cache.computeDefaultMaxBytes(t.Context(), + func(string) (uint64, error) { return 1073741824, nil }) + if err != nil { + 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()) + } +} + +// TestNewCacheComputesDefaultMaxBytesWhenAsked verifies that with +// UseDefaultMaxBytes set, the cache's limit becomes the computed +// default, and that the probe is pointed at /cache/, which +// must be created first so statfs measures the right filesystem. +func TestNewCacheComputesDefaultMaxBytesWhenAsked(t *testing.T) { + t.Parallel() + + stateDir := t.TempDir() + wantCacheDir := filepath.Join(stateDir, "cache") + + var probedPath string + + // 4 GiB free -> 3 GiB default. + probe := func(path string) (uint64, error) { + probedPath = path + + info, err := os.Stat(path) + if err != nil || !info.IsDir() { + t.Errorf("cache directory %q was not created before probing: info=%v err=%v", + path, info, err) + } + + return 4294967296, nil + } + + cache, err := newCache(evictionTestDB(t), CacheConfig{ + StateDir: stateDir, + UseDefaultMaxBytes: true, + }, probe) + if err != nil { + t.Fatalf("newCache returned error: %v", err) + } + + if cache.config.MaxBytes != 3221225472 { + t.Errorf("MaxBytes = %d, want computed default 3221225472", + cache.config.MaxBytes) + } + + if probedPath != wantCacheDir { + t.Errorf("free space probed at %q, want cache directory %q", + probedPath, wantCacheDir) + } +} + +// TestNewCacheKeepsExplicitMaxBytes verifies that without +// UseDefaultMaxBytes the cache keeps MaxBytes exactly as given and +// never consults the free-space probe. +func TestNewCacheKeepsExplicitMaxBytes(t *testing.T) { + t.Parallel() + + probe := func(string) (uint64, error) { + t.Error("free-space probe must not be consulted for explicit values") + + return 0, errTestProbeNotExpected + } + + cache, err := newCache(evictionTestDB(t), CacheConfig{ + StateDir: t.TempDir(), + MaxBytes: 1024, + }, probe) + if err != nil { + t.Fatalf("newCache returned error: %v", err) + } + + if cache.config.MaxBytes != 1024 { + t.Errorf("MaxBytes = %d, want explicit 1024 (no floor, no recompute)", + cache.config.MaxBytes) + } +}