fix(transcode): store the derived blob before returning, not after
Fire-and-forget raced its own purpose. A second request for the SAME content arriving before the spawned write landed found no row, re-ran the full decode + encode, and stored the identical blob again. Keying derivations by content exists so identical content is derived once — a write that has not landed yet cannot deliver that, and the window is milliseconds wide exactly when it matters most, a page loading many images at once. Caught by transcode_cache.hurl, which asserts a second distinct file with identical bytes does not re-transcode: `transcodes: 2` where 1 was expected, `disk_hits: 0` where the derived tier should have answered. It had been passing on timing luck. The cost of awaiting is bounded. This path has just spent a full decode and re-encode, so one blob write beside it is marginal, and it only runs on a genuine miss — every subsequent request for that content is served from the row. The negative verdict was already awaited, which is why only the positive half of the scenario failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -569,10 +569,25 @@ impl ImageTranscodeService {
|
|||||||
let variant = target_format.extension().to_string();
|
let variant = target_format.extension().to_string();
|
||||||
let mime = target_format.mime_type().to_string();
|
let mime = target_format.mime_type().to_string();
|
||||||
let bytes = transcoded_bytes.clone();
|
let bytes = transcoded_bytes.clone();
|
||||||
// Fire-and-forget, as the disk write was: the bytes are
|
// Awaited, NOT spawned.
|
||||||
// already on their way to the client, and a storage hiccup
|
//
|
||||||
// should cost a re-derive later rather than this response.
|
// Fire-and-forget looked free — the bytes are already on
|
||||||
tokio::spawn(async move {
|
// their way to the client — but it raced its own purpose. A
|
||||||
|
// second request for the SAME content arriving before the
|
||||||
|
// spawn lands finds no row, re-runs the whole decode +
|
||||||
|
// encode, and stores the identical blob again. The point of
|
||||||
|
// keying by content is that identical content is derived
|
||||||
|
// once; a write that has not landed yet cannot deliver
|
||||||
|
// that, and the window is milliseconds wide precisely when
|
||||||
|
// it matters most (a page loading many images at once).
|
||||||
|
//
|
||||||
|
// Caught by `transcode_cache.hurl`, which asserts the second
|
||||||
|
// distinct file with identical bytes does not re-transcode
|
||||||
|
// — it had been passing on timing luck.
|
||||||
|
//
|
||||||
|
// The cost is bounded: this path has just spent a full
|
||||||
|
// decode and re-encode, so one blob write is marginal
|
||||||
|
// beside it, and it only runs on a genuine miss.
|
||||||
if let Err(e) = dedup
|
if let Err(e) = dedup
|
||||||
.store_derived_blob(&hash, Self::DERIVED_KIND, &variant, &mime, bytes)
|
.store_derived_blob(&hash, Self::DERIVED_KIND, &variant, &mime, bytes)
|
||||||
.await
|
.await
|
||||||
@@ -584,7 +599,6 @@ impl ImageTranscodeService {
|
|||||||
"failed to store derived transcode; it will be recomputed"
|
"failed to store derived transcode; it will be recomputed"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
});
|
|
||||||
}
|
}
|
||||||
None => {
|
None => {
|
||||||
let cache_path_clone = cache_path.clone();
|
let cache_path_clone = cache_path.clone();
|
||||||
|
|||||||
Reference in New Issue
Block a user