diff --git a/docs/plan/derived-blobs.md b/docs/plan/derived-blobs.md index 6e460d74..55c201fc 100644 --- a/docs/plan/derived-blobs.md +++ b/docs/plan/derived-blobs.md @@ -1003,9 +1003,19 @@ first render was immediately stale. Caught by `thumbnail_etag_content_keyed.hurl`, where two consecutive GETs of an unchanged file stopped revalidating to 304. -The flip is what removes the hazard: once the derived tier is -authoritative it is populated before it is consulted, so there is no -window in which the row appears between two reads. +**The flip alone does NOT remove the hazard** *(corrected 2026-08-26 — +an earlier revision of this paragraph claimed it did)*. A first render +still creates the row as a side effect of producing the body, whatever +the read order, so two consecutive reads would still straddle its +appearance. + +What actually removes it is resolving the ETag **after** generation on +the 200 path. A 304 can only fire when the client already holds a +validator, which means it has been served before, which means the row +exists — so the *conditional* path can safely consult the derived hash +up front, while the *generating* path computes it from bytes it now +holds. That is a handler restructure, not an ordering change, and it is +the actual prerequisite for the derived-hash ETag. **The disk cache is `CachedBlobBackend`, reused unchanged.** No thumbnail-specific cache, no second root path. Routing derived @@ -1310,10 +1320,23 @@ hardcoded SQL). New sources bolt on independently. **re-keyed** file→content on import (legitimate only because a transcode is derivable). Its `.skip` markers — a cached negative verdict with no bytes — remain an open question. - c. **Flip the read order**, derived first. The HTTP ETag's - source-keyed fallback becomes unreachable here; the attached and - derived halves already landed early, forced by the attachment - case (see *HTTP ETag*). + c. **Flip the read order**, derived first — **done 2026-08-26**. Two + things it is not: a two-line swap, and an ETag fix. + + A derived miss must **fall through** to the sidecar, where the old + code terminated the lookup — while the imports drain, most content + has a sidecar and no row, so terminating would report "no + thumbnail" for nearly everything. + + And it is **WebP-only**. `store_derived_blob` writes `image/webp` + with `variant` keyed on size alone, no format term, so a JPEG + request matches the WebP row and gets the wrong codec — a + regression the old ordering hid, because the `.jpg` sidecar won + first. JPEG clients therefore stay on the sidecar, **and the + sidecar cannot be deleted for them** until `variant` encodes + format. That is a new prerequisite for (e), not a detail: it means + a migration to `(kind, variant, format)` — or a format term inside + `variant` — has to land before the directories can go. d. **Enable deletion** in the import jobs (opt-in, readback-verified). e. **Remove the fallback read path** once the directory no longer *exists* — not merely once it is empty. Two reasons. Empty is a diff --git a/src/infrastructure/services/thumbnail_service.rs b/src/infrastructure/services/thumbnail_service.rs index 85a5a861..6794f794 100644 --- a/src/infrastructure/services/thumbnail_service.rs +++ b/src/infrastructure/services/thumbnail_service.rs @@ -792,11 +792,35 @@ impl ThumbnailService { return Some(bytes); } - // 4. Check disk for blob-hash thumbnails (needs blob_hash to locate) - let thumb_path = self.get_thumbnail_path(hash, size, format); - if let Ok(data) = fs::read(&thumb_path).await { - let bytes = Bytes::from(data); - // Populate in-memory cache for next hit + // 4. Derived blob — the authoritative content tier (step 10c). + // + // Ahead of the sidecar now, rather than last. The sidecar is local + // disk: invisible to other instances, uncarried by a backend + // migration, uncovered by any consistency job. Reading the derived + // tier first is what lets that disk state become deletable, and it is + // not the cost it looks like — `CachedBlobBackend` gives the blob read + // a local disk cache, and moka absorbs the repeats above it. + // + // A miss FALLS THROUGH rather than ending the lookup. That is the + // whole reason this is not a two-line swap: while the imports are + // draining, most content has a sidecar and no row, and terminating + // here would return "no thumbnail" for all of it. + // + // **WebP only.** `store_derived_blob` writes `image/webp` and keys + // `variant` on the size alone, with no format term, so a JPEG request + // would match the WebP row and be served the wrong codec — a + // regression the old ordering hid, because the `.jpg` sidecar won + // first. Until `variant` encodes format, JPEG clients stay on the + // sidecar, and the sidecar therefore cannot be deleted for them. See + // docs/plan/derived-blobs.md. + if format == ThumbnailFormat::Webp + && let Some(dedup) = dedup + && let Some(derived) = dedup + .find_derived_blob(hash, "thumbnail", size.dir_name()) + .await + && let Some(bytes) = + Self::read_blob_to_bytes(dedup, &derived.blob_hash, file_id, size).await + { self.cache .insert( ThumbnailCacheKey::content(hash, size, format), @@ -806,26 +830,21 @@ impl ThumbnailService { return Some(bytes); } - // 4. Tier-3 derived blob. Deliberately LAST while the sidecar still - // exists: for every thumbnail already on disk this branch is never - // reached, so the DB stays off the hot path and a fault here cannot - // break a working gallery. It answers only what disk cannot — another - // instance's render, or a box whose sidecar was never populated. - // - // The order flips (derived blob first, sidecar as fallback) in the - // release that removes the sidecar; see docs/plan/derived-blobs.md. - let dedup = dedup?; - let derived = dedup - .find_derived_blob(hash, "thumbnail", size.dir_name()) - .await?; - let bytes = Self::read_blob_to_bytes(dedup, &derived.blob_hash, file_id, size).await?; - self.cache - .insert( - ThumbnailCacheKey::content(hash, size, format), - bytes.clone(), - ) - .await; - Some(bytes) + // 5. Blob-hash sidecar — fallback for content not yet imported, and + // the only content tier a non-WebP request can reach. + let thumb_path = self.get_thumbnail_path(hash, size, format); + if let Ok(data) = fs::read(&thumb_path).await { + let bytes = Bytes::from(data); + self.cache + .insert( + ThumbnailCacheKey::content(hash, size, format), + bytes.clone(), + ) + .await; + return Some(bytes); + } + + None } /// Store an externally-generated thumbnail (e.g. client-side video frame).