docs(plan): negative verdicts get a nullable blob_hash

Resolves the .skip-marker question. The table can say "here is the
artifact" but not "there is deliberately no artifact", and absence of a
row is ambiguous — it means both never-attempted and attempted-and-not-
worth-it, which destroys the only thing a negative cache holds.

Two live cases, not one. Transcodes drop a .skip marker when WebP is not
smaller. Thumbnails return empty Bytes for undecodable sources and ones
over MAX_DECODE_PIXELS — RAM-only today, so after moka evicts, a
60-megapixel upload has its full decode attempted again, forever. Same
gap, same table, so do both together.

Nullable rather than a sentinel hash: a sentinel stops blob_hash naming
a real blob and every future reader has to know the lie. Costs are one
`AND blob_hash IS NOT NULL` in ContentDerivedReferenceSource, the same
guard on the dangling-derived check, and dropping NOT NULL.

Permanent vs transient is the load-bearing split, and today's code
cannot tell them apart: generate_and_persist collapses every error into
empty Bytes, timeouts and semaphore closures included. Survivable while
the sentinel lives in moka, which evicts. Persist that same signal and a
thumbnail that timed out once under load is unrenderable forever. So the
renderer must return a typed outcome first, and only
permanently-unrenderable earns a row. The asymmetry sets the default —
a wrongly-cached transient is silent and permanent, a not-cached
permanent only costs repeated work — so unclassified errors are treated
as transient.

No TTL and no renderer-version term. Time is the wrong axis: the verdict
is deterministic in (content, encoder) and does not decay, so a TTL
re-attempts an OOMing decode on a schedule while still leaving staleness
for most of the window after a deploy. What removes the need for a
mechanism is that negative rows are disposable — they hold no data, so
a library upgrade invalidates them with a DELETE ... WHERE blob_hash IS
NULL in the same migration as the dependency bump. Recorded explicitly
so nobody later builds the expiry logic this replaces.
This commit is contained in:
Edouard Vanbelle
2026-08-27 13:46:32 +02:00
parent c04f0c6824
commit 647929ed11
+99 -10
View File
@@ -339,6 +339,97 @@ Same trims as `content_derived_blobs` — no `size`, no `format`, no
Generic naming rather than `file_previews` because the family is Generic naming rather than `file_previews` because the family is
real, and each member would otherwise be a new table plus a new real, and each member would otherwise be a new table plus a new
`BlobReferenceSource` plus a new term in the consistency recompute. `BlobReferenceSource` plus a new term in the consistency recompute.
### Negative verdicts — `blob_hash` must be nullable
*(Resolved 2026-08-27. Supersedes the "`.skip` markers are an open
question" note.)*
The table says "here is the artifact". It cannot say **"there is
deliberately no artifact"**, and absence of a row is ambiguous — it
means both *never attempted* and *attempted, not worth it*. Collapsing
those destroys the only information a negative cache exists to hold.
Two live cases, not one:
* **Transcode not beneficial.** `ImageTranscodeService` can only learn
whether WebP is smaller by doing the full decode + encode. When it is
not, it records a zero-byte `{file_id}.{ext}.skip` marker so the next
GET does not repeat the work.
* **Thumbnail not renderable.** `generate_and_persist` returns empty
`Bytes` — "moka's zero-weight negative-entry convention" — for sources
over `MAX_DECODE_PIXELS` (50 MP) or that fail to decode. This one is
**RAM-only**: after moka evicts, a 60-megapixel upload has its full
decode attempted again, forever.
So this is not a transcode quirk. The rule generalises to every `kind`:
> **Any derivation whose failure is deterministic in the source content
> is worth memoising negatively.**
**The discriminator, or the table fills with noise.** Persist a negative
only when it is *both* expensive to compute *and* deterministic in the
content. `can_transcode(mime)` and "not an image" are cheap metadata
checks — recomputing is free and a row would be pure overhead. It is the
ones that cost a decode that earn a row.
**Permanent vs transient is the load-bearing split.** "Deterministic"
above means *a property of the content*, not merely *a failure that
happened*:
| Permanent — cache it | Transient — never cache it |
|---|---|
| decode failed (corrupt / unsupported) | generation timeout |
| over `MAX_DECODE_PIXELS` | decode semaphore closed |
| transcode result not smaller | blob read I/O error, OOM under load |
**Today's code cannot tell them apart, and that must be fixed before any
of this is persisted.** `generate_and_persist` collapses *every* error
into empty `Bytes` — timeouts and semaphore closures included. That is
survivable while the sentinel lives only in moka, which evicts; write
the same signal to the database and a thumbnail that timed out once
under load is marked unrenderable **forever**. So the renderer must
return a typed outcome — rendered / permanently-unrenderable / transient
failure — and only the middle one earns a row.
The asymmetry sets the default. A wrongly-cached transient is silent and
permanent; a not-cached permanent merely costs repeated work. **When in
doubt, do not cache** — treat unclassified errors as transient.
**Representation: nullable `blob_hash`.** A sentinel hash was considered
and rejected — it stops `blob_hash` naming a real blob, and every future
reader has to know the lie. NULL says what is true. Costs:
* `ContentDerivedReferenceSource` needs `AND blob_hash IS NOT NULL`; a
row holding no blob holds no reference.
* The dangling-derived check (row 9 of the coverage matrix) needs the
same guard, or every negative verdict reports as a broken row.
* The `NOT NULL` constraint is dropped.
**No TTL, and no renderer-version term either.** Both were considered.
Time is the wrong axis: the verdict is deterministic in
`(content, encoder)` and does not decay, so a TTL re-attempts an OOMing
decode on a schedule — reintroducing the exact waste the negative
exists to prevent — while still leaving staleness for most of the window
after a deploy.
What makes the mechanism unnecessary is that **negative rows are
disposable**: they hold no data, so discarding one costs only a
re-derivation. Upgrading the image library invalidates them with a line
in the same migration as the dependency bump —
```sql
DELETE FROM storage.content_derived_blobs WHERE blob_hash IS NULL;
```
— which beats a version term that must be remembered and leaves dead
rows behind when bumped. Write this down, or someone later builds the
expiry logic this paragraph exists to prevent.
Same shape, outside this table and not solved here:
`blob_extracted_text` (no extractable text) and `faces.faces` (no faces
detected) are both deterministic negatives currently indistinguishable
from "never processed".
With `kind` it's a one-line `ALTER … CHECK`: With `kind` it's a one-line `ALTER … CHECK`:
| Kind | Why it lands here | | Kind | Why it lands here |
@@ -1308,13 +1399,10 @@ hardcoded SQL). New sources bolt on independently.
and DI change — mind the construction order, as and DI change — mind the construction order, as
`ThumbnailService` hit the same thing and solved it with a `ThumbnailService` hit the same thing and solved it with a
per-call parameter instead. per-call parameter instead.
* **Decide the `.skip` markers.** `{file_id}.{ext}.skip` records a * **Write the `.skip` markers as negative rows** — resolved: nullable
negative verdict ("result was not smaller — serve the original") `blob_hash`, see *Negative verdicts*. Thumbnails need the same
and has no bytes, so it does not fit a table whose row points at a treatment for undecodable and over-`MAX_DECODE_PIXELS` sources,
blob. Options: leave them as a purely local cache and accept the which are RAM-only today, so do both together rather than twice.
verdict being recomputed per instance, or model it with a sentinel
`blob_hash`. Unresolved; it is the only genuinely open design
question in step 10.
Note the cache is keyed `{file_id}:{ext}` in memory and Note the cache is keyed `{file_id}:{ext}` in memory and
`.transcoded/{ext}/{file_id}.{ext}` on disk, so it also carries the `.transcoded/{ext}/{file_id}.{ext}` on disk, so it also carries the
@@ -1349,9 +1437,10 @@ hardcoded SQL). New sources bolt on independently.
is still needed: `ImageTranscodeService` **already exists** and is still needed: `ImageTranscodeService` **already exists** and
caches `.transcoded/{ext}/{file_id}.{ext}`, so those must be caches `.transcoded/{ext}/{file_id}.{ext}`, so those must be
**re-keyed** file→content on import (legitimate only because a **re-keyed** file→content on import (legitimate only because a
transcode is derivable). Its `.skip` markers — a cached negative transcode is derivable). Its `.skip` markers import as **negative
verdict with no bytes — remain an open question, since they do not rows** with a NULL `blob_hash` — see *Negative verdicts* — so the
fit a table whose point is pointing at a blob. verdict survives the deletion of `.transcoded/`, which it
otherwise would not.
**Not next, and deliberately so.** Two prerequisites, both learned **Not next, and deliberately so.** Two prerequisites, both learned
the hard way on the thumbnail side: the hard way on the thumbnail side: