diff --git a/TODO.md b/TODO.md index 4897055..09b2d93 100644 --- a/TODO.md +++ b/TODO.md @@ -36,7 +36,9 @@ P2: security: per-origin rate limiting after that, so a client asking for a new missing path each time added rows that stayed for good. The eviction goroutine now calls `CleanExpired` at startup and on each periodic tick, every 5 minutes in pixad, so a row stays at - most about 10 minutes. It runs with the disk cache disabled + most about 10 minutes. `CleanExpired` deletes at most 1000 rows per statement, + so a request waits behind 1000 deletions at most, however many rows have + expired. The goroutine runs with the disk cache disabled (`cache_max_bytes: 0`) too, where it does nothing else, as the negative cache is used either way. - 2026-10-13 `fit=outside` no longer makes an output above 8192 on a side diff --git a/internal/imgcache/cache.go b/internal/imgcache/cache.go index f5e4e47..dc49692 100644 --- a/internal/imgcache/cache.go +++ b/internal/imgcache/cache.go @@ -32,6 +32,12 @@ const httpStatusOK = 200 // variant not among them is served as before, reading its .meta file. const metaCacheSize = 10000 +// cleanExpiredBatchSize is the most expired negative cache rows one +// statement of CleanExpired deletes. Each batch is a statement of its +// own, so a request waits behind one batch at most, however many rows +// have expired. +const cleanExpiredBatchSize = 1000 + // CacheConfig holds cache configuration. type CacheConfig struct { StateDir string @@ -483,20 +489,27 @@ func (c *Cache) GetSourceContent( return c.srcContent.LoadWithSize(contentHash) } -// CleanExpired removes the negative cache rows past their expiry. The -// eviction goroutine calls it at startup and on every periodic tick, so -// a row stays at most NegativeTTL plus one eviction interval, whether or -// not its URL is requested again. +// CleanExpired removes the negative cache rows past their expiry, one +// batch per statement, until none is left. Once ctx ends it starts no +// further batch and returns ctx's error. The eviction goroutine calls it +// at startup and on every periodic tick, so a row stays about +// NegativeTTL plus one eviction interval, whether or not its URL is +// requested again. func (c *Cache) CleanExpired(ctx context.Context) error { - // Clean expired negative cache entries - _, err := c.db.ExecContext(ctx, ` - DELETE FROM negative_cache WHERE expires_at < CURRENT_TIMESTAMP - `) - if err != nil { - return fmt.Errorf("failed to clean negative cache: %w", err) - } + for { + if ctx.Err() != nil { + return ctx.Err() + } - return nil + deleted, err := c.cleanExpiredBatch(ctx) + if err != nil { + return err + } + + if deleted < cleanExpiredBatchSize { + return nil + } + } } // Stats returns cache statistics. @@ -760,3 +773,27 @@ func (c *Cache) checkNegativeCache( return true, nil } + +// cleanExpiredBatch deletes up to cleanExpiredBatchSize negative cache +// rows past their expiry in one statement, and returns how many it +// deleted. DELETE ... LIMIT needs a SQLite build option, so the subquery +// picks the rows. +func (c *Cache) cleanExpiredBatch(ctx context.Context) (int64, error) { + result, err := c.db.ExecContext(ctx, ` + DELETE FROM negative_cache WHERE id IN ( + SELECT id FROM negative_cache + WHERE expires_at < CURRENT_TIMESTAMP + LIMIT ? + ) + `, cleanExpiredBatchSize) + if err != nil { + return 0, fmt.Errorf("failed to clean negative cache: %w", err) + } + + deleted, err := result.RowsAffected() + if err != nil { + return 0, fmt.Errorf("failed to count cleaned negative cache rows: %w", err) + } + + return deleted, nil +}