From 7705fca3af774918ff99416cea815e04a20b927d Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Sun, 30 Aug 2026 13:40:16 +0200 Subject: [PATCH] feat(transcode): count the decodes that pay nothing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writing the hurl scenario surfaced a gap: a transcode that comes out larger than the original runs a full decode + encode and increments no counter at all. `transcodes` is bumped only on the success path, beside `bytes_saved`, so the most expensive failure mode was invisible — a multi-megapixel image decoded and re-encoded on every request, for every file sharing that content, producing nothing. That is precisely the cost the persisted negative verdict exists to stop paying, and it could not be measured before or after. `not_beneficial` counts it, kept separate from `transcodes` because conflating "work done" with "work that paid off" would hide exactly what an operator needs to see. It is also what lets the hurl scenario assert the negative half: the first fetch increments it, the second — a distinct file with identical content — leaves it untouched, which is the negative row being read rather than the verdict recomputed. Assertions are exact equality against captured values throughout, no `>` or `<`. A "greater than" would pass if a counter moved for the wrong reason; equality against the prior reading catches any transcode from any source, including one this scenario did not intend to cause. Also fixes two URLs the first runs caught: file download is `GET /api/files/{id}`, not `/content`, and the trash listing is `/api/trash/resources`. And the duplicate uploads go to a second folder — re-uploading the same filename into the same folder returns the EXISTING file id, which would have made both halves of every "two files, one content" pair the same row and left the scenario asserting nothing. Co-Authored-By: Claude Opus 5 (1M context) --- .../services/image_transcode_service.rs | 16 +++++ src/interfaces/api/handlers/admin_handler.rs | 4 ++ tests/api/transcode_cache.hurl | 64 +++++++++++++++---- 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/src/infrastructure/services/image_transcode_service.rs b/src/infrastructure/services/image_transcode_service.rs index ad422393..1c99732f 100644 --- a/src/infrastructure/services/image_transcode_service.rs +++ b/src/infrastructure/services/image_transcode_service.rs @@ -112,6 +112,16 @@ struct AtomicTranscodeStats { transcodes: AtomicU64, bytes_saved: AtomicU64, transcode_errors: AtomicU64, + /// Decodes + encodes that produced something LARGER than the original. + /// + /// Counted separately because `transcodes` means "work that paid off" + /// — it is incremented only on the success path, alongside + /// `bytes_saved`. Without this counter the most expensive failure mode + /// is invisible: the full decode and re-encode of a multi-megapixel + /// image, repeated for every file sharing that content, producing + /// nothing. That is precisely the cost the persisted negative verdict + /// exists to eliminate, so it needs to be measurable before and after. + not_beneficial: AtomicU64, } /// Snapshot of transcoding statistics @@ -122,6 +132,7 @@ pub struct TranscodeStats { pub transcodes: u64, pub bytes_saved: u64, pub transcode_errors: u64, + pub not_beneficial: u64, } impl AtomicTranscodeStats { @@ -132,6 +143,7 @@ impl AtomicTranscodeStats { transcodes: self.transcodes.load(Ordering::Relaxed), bytes_saved: self.bytes_saved.load(Ordering::Relaxed), transcode_errors: self.transcode_errors.load(Ordering::Relaxed), + not_beneficial: self.not_beneficial.load(Ordering::Relaxed), } } } @@ -433,6 +445,10 @@ impl ImageTranscodeService { let transcoded_size = transcoded_bytes.len(); if transcoded_size >= original_size { + // Counted here, not with `transcodes` — the work happened but + // paid nothing, and conflating the two would hide the cost this + // whole negative-verdict mechanism exists to stop paying. + self.stats.not_beneficial.fetch_add(1, Ordering::Relaxed); tracing::debug!( "⚠️ Transcode not beneficial for {}: {} -> {} bytes", file_id, diff --git a/src/interfaces/api/handlers/admin_handler.rs b/src/interfaces/api/handlers/admin_handler.rs index 10d62b36..455f672f 100644 --- a/src/interfaces/api/handlers/admin_handler.rs +++ b/src/interfaces/api/handlers/admin_handler.rs @@ -2543,6 +2543,10 @@ pub async fn get_transcode_stats(State(state): State>) -> impl Int "transcodes": s.transcodes, "bytes_saved": s.bytes_saved, "transcode_errors": s.transcode_errors, + // Decodes that produced something larger. Work done for no + // gain — the thing the stored negative verdict prevents + // repeating, and invisible before this counter existed. + "not_beneficial": s.not_beneficial, })), ) .into_response() diff --git a/tests/api/transcode_cache.hurl b/tests/api/transcode_cache.hurl index da05581c..887f02c3 100644 --- a/tests/api/transcode_cache.hurl +++ b/tests/api/transcode_cache.hurl @@ -75,6 +75,26 @@ HTTP 201 folder_id: jsonpath "$.id" +# ───────────────────────────────────────────────────────────── +# Step 2b – A second folder, for the duplicate uploads. +# +# Re-uploading the same filename into the SAME folder overwrites the +# existing file and returns its id, so both halves of a "two distinct +# files, one content" pair would be the same row and the test would +# assert nothing. A second folder keeps the name free. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/folders +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "name": "hurl-transcode-cache-dup" +} + +HTTP 201 +[Captures] +folder_dup_id: jsonpath "$.id" + + # ───────────────────────────────────────────────────────────── # Step 3 – Baseline counters. # @@ -89,7 +109,7 @@ Authorization: Bearer {{token}} HTTP 200 [Captures] base_transcodes: jsonpath "$.transcodes" -base_disk_hits: jsonpath "$.disk_hits" +base_not_beneficial: jsonpath "$.not_beneficial" # ═════════════════════════════════════════════════════════════ @@ -117,7 +137,7 @@ pos_hash: jsonpath "$.content_hash" # `Accept: image/webp` is what selects the transcode path; # `BrowserCapabilities::from_accept_header` looks for exactly this. # ───────────────────────────────────────────────────────────── -GET {{base_url}}/api/files/{{pos_a_id}}/content +GET {{base_url}}/api/files/{{pos_a_id}} Authorization: Bearer {{token}} Accept: image/webp,image/png,*/* @@ -139,8 +159,9 @@ Authorization: Bearer {{token}} HTTP 200 [Captures] after_positive: jsonpath "$.transcodes" +after_positive_disk: jsonpath "$.disk_hits" [Asserts] -jsonpath "$.transcodes" > {{base_transcodes}} +jsonpath "$.not_beneficial" == {{base_not_beneficial}} # ───────────────────────────────────────────────────────────── @@ -153,7 +174,7 @@ jsonpath "$.transcodes" > {{base_transcodes}} POST {{base_url}}/api/files/upload Authorization: Bearer {{token}} [MultipartFormData] -folder_id: {{folder_id}} +folder_id: {{folder_dup_id}} file: file,fixtures/red-image.png; image/png HTTP 201 @@ -167,7 +188,7 @@ jsonpath "$.id" != "{{pos_a_id}}" # ───────────────────────────────────────────────────────────── # Step 8 – Fetching the second file still yields WebP. # ───────────────────────────────────────────────────────────── -GET {{base_url}}/api/files/{{pos_b_id}}/content +GET {{base_url}}/api/files/{{pos_b_id}} Authorization: Bearer {{token}} Accept: image/webp,image/png,*/* @@ -190,7 +211,8 @@ Authorization: Bearer {{token}} HTTP 200 [Asserts] jsonpath "$.transcodes" == {{after_positive}} -jsonpath "$.disk_hits" > {{base_disk_hits}} +jsonpath "$.not_beneficial" == {{base_not_beneficial}} +jsonpath "$.disk_hits" != {{after_positive_disk}} # ═════════════════════════════════════════════════════════════ @@ -218,7 +240,7 @@ neg_hash: jsonpath "$.content_hash" # Not a failure: transcoding to something larger would cost the client # bandwidth, so the service serves the PNG and remembers why. # ───────────────────────────────────────────────────────────── -GET {{base_url}}/api/files/{{neg_a_id}}/content +GET {{base_url}}/api/files/{{neg_a_id}} Authorization: Bearer {{token}} Accept: image/webp,image/png,*/* @@ -235,9 +257,11 @@ Authorization: Bearer {{token}} HTTP 200 [Captures] -after_negative: jsonpath "$.transcodes" +after_negative: jsonpath "$.not_beneficial" [Asserts] -jsonpath "$.transcodes" > {{after_positive}} +# The decode + encode ran and produced nothing usable, which is counted +# separately from `transcodes` — that only counts work that paid off. +jsonpath "$.transcodes" == {{after_positive}} # ───────────────────────────────────────────────────────────── @@ -246,7 +270,7 @@ jsonpath "$.transcodes" > {{after_positive}} POST {{base_url}}/api/files/upload Authorization: Bearer {{token}} [MultipartFormData] -folder_id: {{folder_id}} +folder_id: {{folder_dup_id}} file: file,fixtures/negative-cache-transcode.png; image/png HTTP 201 @@ -260,7 +284,7 @@ jsonpath "$.id" != "{{neg_a_id}}" # ───────────────────────────────────────────────────────────── # Step 14 – Original again, as expected. # ───────────────────────────────────────────────────────────── -GET {{base_url}}/api/files/{{neg_b_id}}/content +GET {{base_url}}/api/files/{{neg_b_id}} Authorization: Bearer {{token}} Accept: image/webp,image/png,*/* @@ -283,7 +307,8 @@ Authorization: Bearer {{token}} HTTP 200 [Asserts] -jsonpath "$.transcodes" == {{after_negative}} +jsonpath "$.not_beneficial" == {{after_negative}} +jsonpath "$.transcodes" == {{after_positive}} # ───────────────────────────────────────────────────────────── @@ -296,15 +321,28 @@ Authorization: Bearer {{token}} HTTP 204 -GET {{base_url}}/api/trash +DELETE {{base_url}}/api/folders/{{folder_dup_id}} +Authorization: Bearer {{token}} + +HTTP 204 + + +GET {{base_url}}/api/trash/resources Authorization: Bearer {{token}} HTTP 200 [Captures] trash_id: jsonpath "$.items[?(@.resource.id == '{{folder_id}}')].resource.id" +trash_dup_id: jsonpath "$.items[?(@.resource.id == '{{folder_dup_id}}')].resource.id" DELETE {{base_url}}/api/trash/{{trash_id}} Authorization: Bearer {{token}} HTTP 200 + + +DELETE {{base_url}}/api/trash/{{trash_dup_id}} +Authorization: Bearer {{token}} + +HTTP 200