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