fix(thumbnails): drop derived-hash ETag until the read-order flip
The ETag is computed BEFORE the body. On a cache miss no content_derived_blobs row exists, so thumbnail_content_id returned the source-keyed form — then rendering created that row, and the next request resolved to the derived hash instead. The validator changed as a side effect of producing the body, making every first render immediately stale. Latent until aaf08532: before the consolidation, the on-demand render path never wrote a derived row, so the flip had nothing to trigger it. Fixing one gap exposed the other. Caught by thumbnail_etag_content_keyed.hurl — two consecutive GETs of an unchanged file stopped revalidating to 304. The plan already said derived-hash keying must land WITH the read-order flip and not before; I brought it forward anyway when the attachment case forced the attached half. This is the evidence for the constraint, so the plan now records the attempt and why it failed rather than leaving the note as untested caution. The attached lookup stays — it has no such window, since an upload writes its row synchronously before any read can observe it, and it fixes a real collision: a copy inherits the source hash, so an original and a copy carrying different uploaded previews would otherwise share one validator while serving different bytes. The flip removes the hazard for the derived half too: once that tier is authoritative it is populated before it is consulted, so no row can appear between two reads.
This commit is contained in:
@@ -991,6 +991,22 @@ yet generated and for formats the derived tier does not hold. The
|
|||||||
`LEFT JOIN` in step 2 above already returns the derived hash in the
|
`LEFT JOIN` in step 2 above already returns the derived hash in the
|
||||||
same query, so the ETag costs no extra round-trip.
|
same query, so the ETag costs no extra round-trip.
|
||||||
|
|
||||||
|
**Attempted early (2026-08-26) and reverted — the constraint above is
|
||||||
|
load-bearing.** The attached half shipped and is correct, because an
|
||||||
|
upload writes its row synchronously before any read can observe it. The
|
||||||
|
*derived* half was brought forward at the same time and had to be backed
|
||||||
|
out: the ETag is computed **before** the body, so on a cache miss no row
|
||||||
|
exists and the handler emits the source-keyed form — then rendering
|
||||||
|
creates the row, and the very next request resolves to the derived hash.
|
||||||
|
The validator changed as a side effect of producing the body, so every
|
||||||
|
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 disk cache is `CachedBlobBackend`, reused unchanged.** No
|
**The disk cache is `CachedBlobBackend`, reused unchanged.** No
|
||||||
thumbnail-specific cache, no second root path. Routing derived
|
thumbnail-specific cache, no second root path. Routing derived
|
||||||
blobs through the same stack gets, for free:
|
blobs through the same stack gets, for free:
|
||||||
|
|||||||
@@ -607,14 +607,26 @@ impl ThumbnailService {
|
|||||||
/// `immutable` set, clients would never revalidate. Worse, a copy
|
/// `immutable` set, clients would never revalidate. Worse, a copy
|
||||||
/// inherits the source hash, so an original and a copy carrying
|
/// inherits the source hash, so an original and a copy carrying
|
||||||
/// *different* uploaded previews would collide on one ETag.
|
/// *different* uploaded previews would collide on one ETag.
|
||||||
/// * Otherwise a **derived** blob's own hash. Strictly better than
|
/// * Otherwise the **source-keyed** form, which identifies a render of
|
||||||
/// source-keying, never worse: the pair is written from the same bytes,
|
|
||||||
/// and where they can diverge — a sidecar re-rendered while the derived
|
|
||||||
/// row stays pinned by `ON CONFLICT DO NOTHING` — a source-keyed ETag
|
|
||||||
/// is wrong too, because the renderer is not part of the key.
|
|
||||||
/// * Otherwise the source-keyed form, which still identifies a render of
|
|
||||||
/// known content at a known size and format.
|
/// known content at a known size and format.
|
||||||
///
|
///
|
||||||
|
/// # Why the derived blob's own hash is NOT used yet
|
||||||
|
///
|
||||||
|
/// It would be a better key — the hash *is* the bytes, so any change in
|
||||||
|
/// output invalidates by construction. But it cannot be resolved here
|
||||||
|
/// without flipping on the first render: the ETag is computed *before*
|
||||||
|
/// the body, so on a cache miss no `content_derived_blobs` row exists yet
|
||||||
|
/// and this returns the source-keyed form — then rendering *creates* that
|
||||||
|
/// row, and the next request resolves to the derived hash instead. The
|
||||||
|
/// validator would change as a side effect of producing the body, making
|
||||||
|
/// every first render immediately stale.
|
||||||
|
///
|
||||||
|
/// It belongs with the read-order flip, when the derived tier becomes
|
||||||
|
/// authoritative and is populated before it is consulted. See
|
||||||
|
/// `docs/plan/derived-blobs.md`. The attached lookup above has no such
|
||||||
|
/// problem: an upload writes its row synchronously, before any read that
|
||||||
|
/// could observe it.
|
||||||
|
///
|
||||||
/// Known gap: a legacy `ext-{file_id}.jpg` with no `file_attached_blobs`
|
/// Known gap: a legacy `ext-{file_id}.jpg` with no `file_attached_blobs`
|
||||||
/// row yet falls through to the source-keyed form, so those bytes keep
|
/// row yet falls through to the source-keyed form, so those bytes keep
|
||||||
/// today's coarse validator until the import backfills the row. No worse
|
/// today's coarse validator until the import backfills the row. No worse
|
||||||
@@ -627,19 +639,12 @@ impl ThumbnailService {
|
|||||||
format: ThumbnailFormat,
|
format: ThumbnailFormat,
|
||||||
dedup: Option<&DedupService>,
|
dedup: Option<&DedupService>,
|
||||||
) -> String {
|
) -> String {
|
||||||
if let Some(dedup) = dedup {
|
if let Some(dedup) = dedup
|
||||||
if let Some(attached) = dedup
|
&& let Some(attached) = dedup
|
||||||
.find_attached_blob(file_id, "preview", size.dir_name())
|
.find_attached_blob(file_id, "preview", size.dir_name())
|
||||||
.await
|
.await
|
||||||
{
|
{
|
||||||
return attached.blob_hash;
|
return attached.blob_hash;
|
||||||
}
|
|
||||||
if let Some(derived) = dedup
|
|
||||||
.find_derived_blob(blob_hash, "thumbnail", size.dir_name())
|
|
||||||
.await
|
|
||||||
{
|
|
||||||
return derived.blob_hash;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
format!(
|
format!(
|
||||||
"thumb-{}-{}-{}",
|
"thumb-{}-{}-{}",
|
||||||
|
|||||||
Reference in New Issue
Block a user