The reconciliation pass (reconcileVariantRows in internal/imgcache/eviction.go) dropped a variant's accounting row on any error reading its file's size, and logged the file as missing. Now only a missing file loses its row, with the same info line. Any other error keeps the row and logs the warning "kept accounting row for unreadable variant file", with the variant key and the error. The pass then goes on to the next key.
Two commits, for a rebase merge: the first adds the test, which fails on next; the second is the fix, a README paragraph under Storage, and the TODO.md entry.
What the diff does not show:
The test makes the variant file a symbolic link to itself, so os.Stat fails with "too many levels of symbolic links". A directory at the variant path would not do: os.Stat reads its size without error. A permission-based setup would not either, since the test phase runs as root.
The test calls reconcileVariantRows alone. In a whole pass, the walk of the variant directory runs first and removes the link with its row as a damaged file, because the link's own size differs from the row's.
Disclosures:
Judgement call: the check is errors.Is(err, ErrNotFound), not fs.ErrNotExist, because FileStore.Size reports a missing file as ErrNotFound.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/pixa/issues/340.
The reconciliation pass (`reconcileVariantRows` in `internal/imgcache/eviction.go`) dropped a variant's accounting row on any error reading its file's size, and logged the file as missing. Now only a missing file loses its row, with the same info line. Any other error keeps the row and logs the warning "kept accounting row for unreadable variant file", with the variant key and the error. The pass then goes on to the next key.
Two commits, for a rebase merge: the first adds the test, which fails on `next`; the second is the fix, a README paragraph under Storage, and the `TODO.md` entry.
What the diff does not show:
- The test makes the variant file a symbolic link to itself, so `os.Stat` fails with "too many levels of symbolic links". A directory at the variant path would not do: `os.Stat` reads its size without error. A permission-based setup would not either, since the test phase runs as root.
- The test calls `reconcileVariantRows` alone. In a whole pass, the walk of the variant directory runs first and removes the link with its row as a damaged file, because the link's own size differs from the row's.
Disclosures:
- Judgement call: the check is `errors.Is(err, ErrNotFound)`, not `fs.ErrNotExist`, because `FileStore.Size` reports a missing file as `ErrNotFound`.
Model: opus-5-5
reconcileVariantRows is given two tracked variants. The first one's
file is a symbolic link to itself, so reading its size fails with an
error other than a missing file. The test wants its row kept, with a
warning naming its key and the error. The second one has no file, and
the test wants its row dropped with the info line for a missing file.
The test fails on next, which drops both rows and logs both files as
missing. The fix follows in the next commit.
Model: opus-5-5
The reconciliation pass dropped a variant's accounting row on any error
reading its file's size, and logged every such file as missing. A file
still on disk then had no row, so eviction no longer counted it against
cache_max_bytes. Now only a missing file (ErrNotFound, which
FileStore.Size returns for it) loses its row, with the same info line.
Any other error keeps the row and logs a warning naming the variant key
and the error, and the pass goes on to the next key. README.md says so
under Storage.
Model: opus-5-5
PASS: only a missing variant file now loses its accounting row, any other error reading its size keeps the row with a warning naming the key and the error, and the first commit alone fails the new test while the second changes no test.
Model: opus-5-5
PASS: only a missing variant file now loses its accounting row, any other error reading its size keeps the row with a warning naming the key and the error, and the first commit alone fails the new test while the second changes no test.
Model: opus-5-5
clawbot
merged commit 33045a24b5 into next2026-10-27 01:04:23 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #340.
The reconciliation pass (
reconcileVariantRowsininternal/imgcache/eviction.go) dropped a variant's accounting row on any error reading its file's size, and logged the file as missing. Now only a missing file loses its row, with the same info line. Any other error keeps the row and logs the warning "kept accounting row for unreadable variant file", with the variant key and the error. The pass then goes on to the next key.Two commits, for a rebase merge: the first adds the test, which fails on
next; the second is the fix, a README paragraph under Storage, and theTODO.mdentry.What the diff does not show:
os.Statfails with "too many levels of symbolic links". A directory at the variant path would not do:os.Statreads its size without error. A permission-based setup would not either, since the test phase runs as root.reconcileVariantRowsalone. In a whole pass, the walk of the variant directory runs first and removes the link with its row as a damaged file, because the link's own size differs from the row's.Disclosures:
errors.Is(err, ErrNotFound), notfs.ErrNotExist, becauseFileStore.Sizereports a missing file asErrNotFound.Model: opus-5-5
clawbot referenced this pull request2026-10-27 01:00:09 +01:00
PASS: only a missing variant file now loses its accounting row, any other error reading its size keeps the row with a warning naming the key and the error, and the first commit alone fails the new test while the second changes no test.
Model: opus-5-5