sneak/pixa
1
0
Fork 0
Code Issues 51 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Cache.Stats reads never-populated output_content/request_cache tables, so TotalItems and TotalSizeBytes are always 0 #56

New Issue
Open
opened 2026-08-07 23:13:03 +02:00 by clawbot · 1 comment
clawbot commented 2026-08-07 23:13:03 +02:00
Collaborator
Copy Link

Discovered while implementing #51 (out of scope there, filing per repo policy).

Cache.Stats in internal/imgcache/cache.go computes TotalItems from request_cache and TotalSizeBytes from output_content:

_ = c.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM request_cache`).Scan(&stats.TotalItems)
_ = c.db.QueryRowContext(ctx, `SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`).Scan(&stats.TotalSizeBytes)

No code path ever inserts into output_content or request_cache (they were created by migration 001 for a design that stored variants content-addressed with a request mapping, but the implementation went with key-addressed VariantStorage instead). Both stats are therefore always 0.

Since #51 (PR #55), real accounting exists: variant_content tracks processed variants and source_content tracks source blobs, and Cache.UsageBytes returns the true total. Suggested fix:

  1. Point TotalItems/TotalSizeBytes at variant_content + source_content (or reuse UsageBytes).
  2. Decide whether to drop the dead output_content/request_cache tables in a future migration or leave them for the originally intended design.

Also worth a look in the same pass: the metaCache field on Cache (in-memory variant meta cache) is initialized but never read or written.

Discovered while implementing #51 (out of scope there, filing per repo policy). `Cache.Stats` in `internal/imgcache/cache.go` computes `TotalItems` from `request_cache` and `TotalSizeBytes` from `output_content`: ``` _ = c.db.QueryRowContext(ctx, `SELECT COUNT(*) FROM request_cache`).Scan(&stats.TotalItems) _ = c.db.QueryRowContext(ctx, `SELECT COALESCE(SUM(size_bytes), 0) FROM output_content`).Scan(&stats.TotalSizeBytes) ``` No code path ever inserts into `output_content` or `request_cache` (they were created by migration 001 for a design that stored variants content-addressed with a request mapping, but the implementation went with key-addressed `VariantStorage` instead). Both stats are therefore always 0. Since #51 (PR #55), real accounting exists: `variant_content` tracks processed variants and `source_content` tracks source blobs, and `Cache.UsageBytes` returns the true total. Suggested fix: 1. Point `TotalItems`/`TotalSizeBytes` at `variant_content` + `source_content` (or reuse `UsageBytes`). 2. Decide whether to drop the dead `output_content`/`request_cache` tables in a future migration or leave them for the originally intended design. Also worth a look in the same pass: the `metaCache` field on `Cache` (in-memory variant meta cache) is initialized but never read or written.
clawbot added this to the 1.0.0 milestone 2026-08-09 03:37:49 +02:00
clawbot commented 2026-08-09 03:51:31 +02:00
Author
Collaborator
Copy Link

Adding a related finding to this issue's scope rather than filing a near-duplicate, found during the 1.0.0 survey and verified against main at 61f42e6.

The stats problem is wider than Cache.Stats reading the wrong tables — the counter columns are never written either:

  • IncrementStats(ctx, hit, fetchBytes) (internal/imgcache/cache.go:324-344) has a fetchBytes > 0 branch that updates upstream_fetch_bytes, but both call sites pass 0 (internal/imgcache/service.go:134 and :148). So upstream_fetch_count and upstream_fetch_bytes never move.
  • transform_count has no writer anywhere in the codebase.

So even after this issue repoints TotalItems/TotalSizeBytes at the real accounting tables (variant_content/source_content, which PR #55 introduces and which do have correct writers), the remaining stats columns will still be permanently zero.

Suggest folding into this issue's definition of done, since it is the same "stats surface reports zeros" defect:

  1. IncrementStats receives the real fetched byte count from both call sites in service.go rather than 0.
  2. transform_count is incremented when a transcode actually happens, or the column is dropped if it is not worth maintaining.
  3. A test asserting each counter actually moves — a stats field with no test is how all of these silently became zero in the first place.

Also still open from this issue's original body: the dead metaCache field. That has grown enough scope (it is the mechanism behind README.md:49's 1-5k r/s claim, and it needs bounding plus eviction-invalidation) that I filed it separately rather than leaving it as a footnote here.

Milestoned 1.0.0. Do this after PR #55 merges — it introduces the accounting tables this should read from.

Adding a related finding to this issue's scope rather than filing a near-duplicate, found during the 1.0.0 survey and verified against `main` at `61f42e6`. The stats problem is wider than `Cache.Stats` reading the wrong tables — **the counter columns are never written either**: - `IncrementStats(ctx, hit, fetchBytes)` (`internal/imgcache/cache.go:324-344`) has a `fetchBytes > 0` branch that updates `upstream_fetch_bytes`, but **both** call sites pass `0` (`internal/imgcache/service.go:134` and `:148`). So `upstream_fetch_count` and `upstream_fetch_bytes` never move. - `transform_count` has no writer anywhere in the codebase. So even after this issue repoints `TotalItems`/`TotalSizeBytes` at the real accounting tables (`variant_content`/`source_content`, which PR #55 introduces and which do have correct writers), the remaining stats columns will still be permanently zero. Suggest folding into this issue's definition of done, since it is the same "stats surface reports zeros" defect: 1. `IncrementStats` receives the real fetched byte count from both call sites in `service.go` rather than `0`. 2. `transform_count` is incremented when a transcode actually happens, or the column is dropped if it is not worth maintaining. 3. A test asserting each counter actually moves — a stats field with no test is how all of these silently became zero in the first place. Also still open from this issue's original body: the dead `metaCache` field. That has grown enough scope (it is the mechanism behind `README.md:49`'s 1-5k r/s claim, and it needs bounding plus eviction-invalidation) that I filed it separately rather than leaving it as a footnote here. Milestoned `1.0.0`. Do this after PR #55 merges — it introduces the accounting tables this should read from.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
merge-ready
needs-checks
needs-rebase
needs-review
needs-rework
No Label
Milestone
No items
No Milestone 1.0.0
Projects
Clear projects
No project
Assignees
Clear assignees
clawbot sneak
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: sneak/pixa#56
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?