From 11e3acc76c21d075c971774b0a58abef96a89609 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Tue, 27 Oct 2026 00:16:54 +0000 Subject: [PATCH] Go on past cache entries the reconciliation pass cannot read (closes #343) The walk of each storage directory returned the error of an entry it could not read. The pass then stopped before its later checks, and the same entry stopped every later pass. Now reconcileFiles skips a directory it cannot list and a file whose info it cannot read, temp files included. It logs a warning naming the path and the error, and the walk goes on. It reads each file's info once, before the temp-file check, and hands it to sweepStaleTempFile, reconcileVariantFile or reconcileSourceFile. A cancelled context still stops the walk with its error. README.md says so under Storage. Model: opus-5-5 --- README.md | 4 +++ TODO.md | 9 +++++ internal/imgcache/eviction.go | 68 ++++++++++++++++++----------------- 3 files changed, 48 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 49cafcb..55fa104 100644 --- a/README.md +++ b/README.md @@ -256,6 +256,10 @@ Each look also drops the rows of variants whose files are missing. When pixa cannot read a variant file's size for another reason, it keeps the variant's row. It logs a warning naming the variant key and the error. +During a look, pixa skips any file or directory in the cache that it cannot +read. It logs a warning naming the path and the error, and goes on with the rest +of the look. + pixa's target is 1-5k r/s, which has not been measured at that rate (see Load Test). Toward it, a cache hit opens the variant's file and nothing else, and makes no database call. The content type comes from the file's first bytes, diff --git a/TODO.md b/TODO.md index 4b6920f..11bda0e 100644 --- a/TODO.md +++ b/TODO.md @@ -30,6 +30,15 @@ P2: security: per-origin rate limiting # Completed Steps +- 2026-10-27 the reconciliation pass skips a directory it cannot list and a file + whose info it cannot read, and goes on (closes #343). It returned the error, + so one such entry stopped every pass before its later checks. + - Each skipped entry gets the warning "skipped unreadable entry in cache + directory", with its path and the error. + - `reconcileFiles` reads each file's info once, before the temp-file check, + and hands it to `sweepStaleTempFile`, `reconcileVariantFile` or + `reconcileSourceFile`. + - A cancelled context still stops the walk with its error. - 2026-10-26 the reconciliation pass drops a variant's accounting row only when its file is missing (closes #340). Any other error reading the file's size keeps the row. It dropped the row on any error and logged the file as missing. diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go index 2312a14..aa12384 100644 --- a/internal/imgcache/eviction.go +++ b/internal/imgcache/eviction.go @@ -515,12 +515,14 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error { } // reconcileFiles walks baseDir, one of the cache's storage directories, -// sweeping stale temp files and passing each other file, with its name, to -// reconcileFile. Once ctx is cancelled it stops at the next file and -// returns ctx's error. +// sweeping stale temp files and passing each other file, with its info and +// name, to reconcileFile. It skips a directory it cannot list and a file +// whose info it cannot read. Each gets a warning naming its path and the +// error, and the walk goes on, so one such entry does not stop the pass. +// Once ctx is cancelled it stops at the next file and returns ctx's error. func (c *Cache) reconcileFiles( ctx context.Context, baseDir string, - reconcileFile func(path string, entry fs.DirEntry, name string) error, + reconcileFile func(path string, info fs.FileInfo, name string) error, ) error { return filepath.WalkDir( baseDir, @@ -529,19 +531,34 @@ func (c *Cache) reconcileFiles( return ctx.Err() } - if err != nil || entry.IsDir() { - return err + if err != nil { + c.log.Warn("skipped unreadable entry in cache directory", + "path", path, "error", err) + + return nil + } + + if entry.IsDir() { + return nil + } + + info, err := entry.Info() + if err != nil { + c.log.Warn("skipped unreadable entry in cache directory", + "path", path, "error", err) + + return nil } name := entry.Name() if strings.HasPrefix(name, tempFilePrefix) { - c.sweepStaleTempFile(path, entry) + c.sweepStaleTempFile(path, info) return nil } - return reconcileFile(path, entry, name) + return reconcileFile(path, info, name) }, ) } @@ -551,8 +568,8 @@ func (c *Cache) reconcileFiles( // and sweeping stale temp files. func (c *Cache) reconcileVariantFiles(ctx context.Context) error { return c.reconcileFiles(ctx, c.variants.baseDir, - func(path string, entry fs.DirEntry, name string) error { - return c.reconcileVariantFile(ctx, path, entry, imagerequest.VariantKey(name)) + func(path string, info fs.FileInfo, name string) error { + return c.reconcileVariantFile(ctx, path, info, imagerequest.VariantKey(name)) }) } @@ -562,16 +579,11 @@ func (c *Cache) reconcileVariantFiles(ctx context.Context) error { // crash or a restored snapshot can leave it, is removed with its row and // a warning naming it, so the variant is converted again. func (c *Cache) reconcileVariantFile( - ctx context.Context, path string, entry fs.DirEntry, cacheKey imagerequest.VariantKey, + ctx context.Context, path string, info fs.FileInfo, cacheKey imagerequest.VariantKey, ) error { - info, err := entry.Info() - if err != nil { - return fmt.Errorf("failed to stat variant file: %w", err) - } - var recordedSize int64 - err = c.db.QueryRowContext(ctx, + err := c.db.QueryRowContext(ctx, `SELECT size_bytes FROM variant_content WHERE cache_key = ?`, string(cacheKey), ).Scan(&recordedSize) @@ -714,8 +726,8 @@ func queryStringColumn[T ~string]( // and sweeping stale temp files. func (c *Cache) reconcileSourceFiles(ctx context.Context) error { return c.reconcileFiles(ctx, c.srcContent.baseDir, - func(path string, entry fs.DirEntry, name string) error { - return c.reconcileSourceFile(ctx, path, entry, imagerequest.SourceKey(name)) + func(path string, info fs.FileInfo, name string) error { + return c.reconcileSourceFile(ctx, path, info, imagerequest.SourceKey(name)) }) } @@ -726,16 +738,11 @@ func (c *Cache) reconcileSourceFiles(ctx context.Context) error { // is removed with its row and a warning naming it, so the source is // fetched again. func (c *Cache) reconcileSourceFile( - ctx context.Context, path string, entry fs.DirEntry, key imagerequest.SourceKey, + ctx context.Context, path string, info fs.FileInfo, key imagerequest.SourceKey, ) error { - info, err := entry.Info() - if err != nil { - return fmt.Errorf("failed to stat source content file: %w", err) - } - var recordedSize int64 - err = c.db.QueryRowContext(ctx, + err := c.db.QueryRowContext(ctx, `SELECT size_bytes FROM source_content WHERE source_key = ?`, string(key), ).Scan(&recordedSize) @@ -900,17 +907,12 @@ func (c *Cache) correctUsageTotal( // sweepStaleTempFile removes a temp file left behind by a crashed // write once it is old enough that no in-flight store can own it. -func (c *Cache) sweepStaleTempFile(path string, entry fs.DirEntry) { - info, err := entry.Info() - if err != nil { - return - } - +func (c *Cache) sweepStaleTempFile(path string, info fs.FileInfo) { if time.Since(info.ModTime()) < staleTempFileAge { return } - err = os.Remove(path) + err := os.Remove(path) if err != nil && !os.IsNotExist(err) { c.log.Warn("failed to remove stale temp file", "path", path, "error", err)