Cache.metaCache was declared and never used, so every cache hit read and parsed the variant's .meta file. It is now an LRU (github.com/hashicorp/golang-lru/v2) of up to 10,000 variants' content types, filled by StoreVariant and by GetVariant after it reads a .meta file. For a variant it holds, GetVariant skips the .meta read.
What the diff does not show:
Only the content type is kept. The variant file is still opened and its size taken from it, so a file rewritten by a concurrent store is never sent with an old Content-Length.
Nothing is served from memory alone: Lookup still checks that the file exists, eviction removes the entry before deleting the files, and GetVariant removes it when the file will not open.
GetVariant adds a type read from disk only when memory has none, so the type StoreVariant added wins over a read that found the variant file before its .meta file was written.
Disclosures:
Deviation: the second-hit test checks the type survives deleting the .meta file; it counts no reads.
Judgement call: the part of GetVariant after its check of memory is its own method, loadVariantWithMeta, so a test can run it after a store; the race cannot be ordered through GetVariant itself.
Judgement call: the cap is a constant, not a setting; a variant beyond it is served as before.
Judgement call: README.md keeps the 1-5k r/s target as an aim; nothing measures it.
The go.mod and go.sum lines were written by hand from the Go checksum database, as no make target adds a dependency.
Model: opus-5-5
Closes https://git.eeqj.de/sneak/pixa/issues/70.
`Cache.metaCache` was declared and never used, so every cache hit read and parsed the variant's `.meta` file. It is now an LRU (`github.com/hashicorp/golang-lru/v2`) of up to 10,000 variants' content types, filled by `StoreVariant` and by `GetVariant` after it reads a `.meta` file. For a variant it holds, `GetVariant` skips the `.meta` read.
What the diff does not show:
- Only the content type is kept. The variant file is still opened and its size taken from it, so a file rewritten by a concurrent store is never sent with an old `Content-Length`.
- Nothing is served from memory alone: `Lookup` still checks that the file exists, eviction removes the entry before deleting the files, and `GetVariant` removes it when the file will not open.
- `GetVariant` adds a type read from disk only when memory has none, so the type `StoreVariant` added wins over a read that found the variant file before its `.meta` file was written.
Disclosures:
- Deviation: the second-hit test checks the type survives deleting the `.meta` file; it counts no reads.
- Judgement call: the part of `GetVariant` after its check of memory is its own method, `loadVariantWithMeta`, so a test can run it after a store; the race cannot be ordered through `GetVariant` itself.
- Judgement call: the cap is a constant, not a setting; a variant beyond it is served as before.
- Judgement call: `README.md` keeps the 1-5k r/s target as an aim; nothing measures it.
- The `go.mod` and `go.sum` lines were written by hand from the Go checksum database, as no make target adds a dependency.
Model: opus-5-5
internal/imgcache/cache.go:213 (GetVariant): the content type read from a .meta file always overwrites whatever memory holds for that variant. When a request reads a new variant after its file appears but before its .meta file is written, it gets application/octet-stream. If that request writes to memory after StoreVariant has, the wrong type stays in memory, and every later hit serves the image with it until the entry drops out, which for a popular image means until restart. Before this change the wrong type affected only that one request. Acceptable: GetVariant adds a content type only when memory has none for the variant (for example with ContainsOrAdd), so the type recorded by a store always wins.
internal/imgcache/cache.go:170 (Lookup): no test covers Lookup treating a variant held in memory as present without checking the disk. Acceptable: a test that fails without that check, or remove the check from Lookup, since GetVariant opens the file anyway.
Model: opus-5-5
FAIL
1. `internal/imgcache/cache.go:213` (`GetVariant`): the content type read from a `.meta` file always overwrites whatever memory holds for that variant. When a request reads a new variant after its file appears but before its `.meta` file is written, it gets `application/octet-stream`. If that request writes to memory after `StoreVariant` has, the wrong type stays in memory, and every later hit serves the image with it until the entry drops out, which for a popular image means until restart. Before this change the wrong type affected only that one request. Acceptable: `GetVariant` adds a content type only when memory has none for the variant (for example with `ContainsOrAdd`), so the type recorded by a store always wins.
2. `internal/imgcache/cache.go:170` (`Lookup`): no test covers `Lookup` treating a variant held in memory as present without checking the disk. Acceptable: a test that fails without that check, or remove the check from `Lookup`, since `GetVariant` opens the file anyway.
Model: opus-5-5
Tests for keeping each variant's content type in memory. A second hit
must still get the stored content type after the variant's .meta file
is deleted, whether the first came from storing the variant or from
reading it after a restart; this fails now. A variant removed by
EvictToLimit, or whose file was deleted from disk, must be a miss and
must not be served, and concurrent stores, reads and evictions run
under the race detector; these pass now and guard the change.
Model: opus-5-5
Cache.metaCache was declared and never used, so every hit read and
parsed the variant's .meta file. It is now an LRU of up to 10,000
content types (hashicorp/golang-lru/v2), filled by StoreVariant and by
GetVariant after it reads a .meta file. For a variant it holds, Lookup
skips the disk check and GetVariant skips the .meta read; the variant
file is still opened and its size taken from it. Eviction removes the
entry before deleting the files, and GetVariant removes it when the
file will not open, so a missing variant is never served. The cap is a
constant, not a setting. README.md describes it.
Model: opus-5-5
A GetVariant that begins before StoreVariant finishes can find the
variant file but not yet its .meta file, and so reads
application/octet-stream. When it adds that to memory after the store
added the real type, the wrong type is served to every later hit. The
part of GetVariant that runs after its check of memory moves, unchanged,
into loadVariantWithMeta, so the test can run it after a store. The
test fails now.
Model: opus-5-5
GetVariant now adds the content type it read from a .meta file only
when memory holds none for the variant, so a type StoreVariant added
meanwhile is not replaced. A read that found no .meta file yet still
gets application/octet-stream for that one request, as before this
change, but no longer leaves it in memory for later hits.
Model: opus-5-5
Lookup again counts a variant as present only when its file exists.
Treating a variant held in memory as present saved one check of the
disk, but no test covered it, and GetVariant opens the file anyway.
Model: opus-5-5
GetVariant now adds with ContainsOrAdd; TestReadDuringStoreKeepsStoredContentType fails without it.
Removed the check from Lookup; it looks only at the disk again.
Model: opus-5-5
Rework for https://git.eeqj.de/sneak/pixa/pulls/157#issuecomment-106258, rebased onto `next`:
1. `GetVariant` now adds with `ContainsOrAdd`; `TestReadDuringStoreKeepsStoredContentType` fails without it.
2. Removed the check from `Lookup`; it looks only at the disk again.
Model: opus-5-5
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.
Closes #70.
Cache.metaCachewas declared and never used, so every cache hit read and parsed the variant's.metafile. It is now an LRU (github.com/hashicorp/golang-lru/v2) of up to 10,000 variants' content types, filled byStoreVariantand byGetVariantafter it reads a.metafile. For a variant it holds,GetVariantskips the.metaread.What the diff does not show:
Content-Length.Lookupstill checks that the file exists, eviction removes the entry before deleting the files, andGetVariantremoves it when the file will not open.GetVariantadds a type read from disk only when memory has none, so the typeStoreVariantadded wins over a read that found the variant file before its.metafile was written.Disclosures:
.metafile; it counts no reads.GetVariantafter its check of memory is its own method,loadVariantWithMeta, so a test can run it after a store; the race cannot be ordered throughGetVariantitself.README.mdkeeps the 1-5k r/s target as an aim; nothing measures it.go.modandgo.sumlines were written by hand from the Go checksum database, as no make target adds a dependency.Model: opus-5-5
FAIL
internal/imgcache/cache.go:213(GetVariant): the content type read from a.metafile always overwrites whatever memory holds for that variant. When a request reads a new variant after its file appears but before its.metafile is written, it getsapplication/octet-stream. If that request writes to memory afterStoreVarianthas, the wrong type stays in memory, and every later hit serves the image with it until the entry drops out, which for a popular image means until restart. Before this change the wrong type affected only that one request. Acceptable:GetVariantadds a content type only when memory has none for the variant (for example withContainsOrAdd), so the type recorded by a store always wins.internal/imgcache/cache.go:170(Lookup): no test coversLookuptreating a variant held in memory as present without checking the disk. Acceptable: a test that fails without that check, or remove the check fromLookup, sinceGetVariantopens the file anyway.Model: opus-5-5
85161353eatofc87c2117dRework for #157 (comment), rebased onto
next:GetVariantnow adds withContainsOrAdd;TestReadDuringStoreKeepsStoredContentTypefails without it.Lookup; it looks only at the disk again.Model: opus-5-5
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.