From bf2f0dc2b2f82b64318dc9858b876dcf9c6dad5a Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 30 Aug 2026 19:25:06 +0200 Subject: [PATCH] fix(transcode): store the derived blob before returning, not after MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- .../services/image_transcode_service.rs | 46 ++++++++++++------- 1 file changed, 30 insertions(+), 16 deletions(-) diff --git a/src/infrastructure/services/image_transcode_service.rs b/src/infrastructure/services/image_transcode_service.rs index 12cc6300..b361b973 100644 --- a/src/infrastructure/services/image_transcode_service.rs +++ b/src/infrastructure/services/image_transcode_service.rs @@ -569,22 +569,36 @@ impl ImageTranscodeService { let variant = target_format.extension().to_string(); let mime = target_format.mime_type().to_string(); let bytes = transcoded_bytes.clone(); - // Fire-and-forget, as the disk write was: the bytes are - // already on their way to the client, and a storage hiccup - // should cost a re-derive later rather than this response. - tokio::spawn(async move { - if let Err(e) = dedup - .store_derived_blob(&hash, Self::DERIVED_KIND, &variant, &mime, bytes) - .await - { - tracing::warn!( - target: "oxicloud::transcode", - source_hash = %hash, - error = %e, - "failed to store derived transcode; it will be recomputed" - ); - } - }); + // Awaited, NOT spawned. + // + // Fire-and-forget looked free — the bytes are already on + // 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 + .store_derived_blob(&hash, Self::DERIVED_KIND, &variant, &mime, bytes) + .await + { + tracing::warn!( + target: "oxicloud::transcode", + source_hash = %hash, + error = %e, + "failed to store derived transcode; it will be recomputed" + ); + } } None => { let cache_path_clone = cache_path.clone();