feat(transcode): count the decodes that pay nothing
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -2543,6 +2543,10 @@ pub async fn get_transcode_stats(State(state): State<Arc<AppState>>) -> 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()
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user