feat(transcode): key the memory cache by content, not by file
The durable tier has been content-keyed since it was introduced —
`content_derived_blobs(source_hash, kind, variant)` — but the moka cache
in front of it was still `{file_id}:{ext}`, so the layer closest to the
request used the wrong axis while the layer behind it used the right
one. That was legacy shape, and I had defended it in a comment as
"deliberate: per-request-path and short-lived", which was a
rationalisation rather than a reason. Ed asked why, and there is no why.
Transcoding is a pure function of the source bytes. Under file keying,
two files with identical content held two RAM entries for identical
bytes, and the second file was a guaranteed miss that fell through to a
DB lookup plus a blob read to fetch what was already in memory under
another key.
Now keyed by content hash when the caller has one, by file id only when
it does not — the same `content` / `external` split `ThumbnailCacheKey`
already makes, and for the same reason: hash-less callers (external
mounts) have no content identity to key on. Prefixed `c:` / `f:` so the
namespaces stay disjoint; a hash and a UUID cannot collide in practice,
but "in practice" is how a file ends up served another file's bytes.
`invalidate` now clears only the file-keyed entry. Dropping content
entries there would be wrong, not merely wasteful: one file's content
changing says nothing about the other files sharing the old bytes, and
evicting theirs would make one user's edit cost everyone else a
re-transcode. Content entries need no eviction — new content is a new
hash, so the old key is never consulted again.
transcode_cache.hurl updated to match, and its header corrected: the
second file is now a RAM hit rather than a derived-tier read, so that
scenario can no longer isolate the durable tier. It says so, and points
at satellites_consistency and a restart as what covers it instead.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -234,6 +234,31 @@ impl ImageTranscodeService {
|
|||||||
/// The `content_derived_blobs.kind` for everything this service writes.
|
/// The `content_derived_blobs.kind` for everything this service writes.
|
||||||
const DERIVED_KIND: &'static str = "transcode";
|
const DERIVED_KIND: &'static str = "transcode";
|
||||||
|
|
||||||
|
/// Memory-cache key: by CONTENT when the caller supplied a hash, by
|
||||||
|
/// file id only when it could not.
|
||||||
|
///
|
||||||
|
/// Transcoding is a pure function of the source bytes, so file keying
|
||||||
|
/// was always the wrong axis for this cache — it just predated the
|
||||||
|
/// content-keyed tier. Two files with identical content held two
|
||||||
|
/// entries for identical bytes, and the second file missed RAM and
|
||||||
|
/// paid a DB lookup plus a blob read to fetch what was already in
|
||||||
|
/// memory under another key.
|
||||||
|
///
|
||||||
|
/// The `c:` / `f:` prefixes keep the two namespaces disjoint. A
|
||||||
|
/// 64-hex hash and a UUID cannot collide in practice, but relying on
|
||||||
|
/// "in practice" for a cache key is how a file ends up served another
|
||||||
|
/// file's bytes.
|
||||||
|
///
|
||||||
|
/// Same shape as `ThumbnailCacheKey`'s `content` / `external` split,
|
||||||
|
/// for the same reason: hash-less callers (external mounts) have no
|
||||||
|
/// content identity to key on, so they keep the per-file entry.
|
||||||
|
fn cache_key(source_hash: Option<&str>, file_id: &str, format: OutputFormat) -> String {
|
||||||
|
match source_hash {
|
||||||
|
Some(hash) => format!("c:{}:{}", hash, format.extension()),
|
||||||
|
None => format!("f:{}:{}", file_id, format.extension()),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/// Initialize the service.
|
/// Initialize the service.
|
||||||
///
|
///
|
||||||
/// Still creates the local cache directories, because this service DOES
|
/// Still creates the local cache directories, because this service DOES
|
||||||
@@ -329,12 +354,7 @@ impl ImageTranscodeService {
|
|||||||
original_mime: &str,
|
original_mime: &str,
|
||||||
target_format: OutputFormat,
|
target_format: OutputFormat,
|
||||||
) -> Result<(Bytes, String, bool), String> {
|
) -> Result<(Bytes, String, bool), String> {
|
||||||
// Keyed by file_id, not content hash. Deliberate: this cache is
|
let cache_key = Self::cache_key(source_hash, file_id, target_format);
|
||||||
// per-request-path and short-lived (10 min TTL), and the file id is
|
|
||||||
// what the caller has in hand on every request. The DURABLE tier
|
|
||||||
// below is content-keyed, which is where dedup across identical
|
|
||||||
// files actually pays.
|
|
||||||
let cache_key = format!("{}:{}", file_id, target_format.extension());
|
|
||||||
|
|
||||||
// ── 1. Fast path: moka memory cache (lock-free read) ──
|
// ── 1. Fast path: moka memory cache (lock-free read) ──
|
||||||
// An empty-Bytes entry is the negative sentinel: "transcoding this
|
// An empty-Bytes entry is the negative sentinel: "transcoding this
|
||||||
@@ -650,7 +670,18 @@ impl ImageTranscodeService {
|
|||||||
|
|
||||||
/// Invalidate cached transcodes for a file
|
/// Invalidate cached transcodes for a file
|
||||||
pub async fn invalidate(&self, file_id: &str) {
|
pub async fn invalidate(&self, file_id: &str) {
|
||||||
let cache_key = format!("{}:{}", file_id, OutputFormat::WebP.extension());
|
// Only the FILE-keyed entry, deliberately.
|
||||||
|
//
|
||||||
|
// Content-keyed entries must not be dropped here: this file's
|
||||||
|
// content changing says nothing about the other files sharing the
|
||||||
|
// old bytes, and evicting theirs would make one user's edit cost
|
||||||
|
// everyone else a re-transcode. They need no eviction anyway —
|
||||||
|
// new content is a new hash, so the old key is simply never
|
||||||
|
// consulted again, and moka's TTL reclaims it.
|
||||||
|
//
|
||||||
|
// What remains here is the fallback entry for hash-less callers,
|
||||||
|
// plus the legacy on-disk pair, which are genuinely per-file.
|
||||||
|
let cache_key = Self::cache_key(None, file_id, OutputFormat::WebP);
|
||||||
self.memory_cache.invalidate(&cache_key).await;
|
self.memory_cache.invalidate(&cache_key).await;
|
||||||
|
|
||||||
let cache_path = self.get_cache_path(file_id, OutputFormat::WebP);
|
let cache_path = self.get_cache_path(file_id, OutputFormat::WebP);
|
||||||
|
|||||||
@@ -20,13 +20,20 @@
|
|||||||
#
|
#
|
||||||
# ## Why each case uploads the same bytes twice
|
# ## Why each case uploads the same bytes twice
|
||||||
#
|
#
|
||||||
# The in-memory cache is keyed `{file_id}:{ext}`, so re-fetching the SAME
|
# Re-fetching the SAME file proves only that a cache exists. Uploading
|
||||||
# file proves only that moka works. Uploading identical content as a
|
# identical content as a SECOND file gives a different file id but the
|
||||||
# SECOND file gives a different file id and therefore a guaranteed memory
|
# same content hash, which is the case that distinguishes content keying
|
||||||
# miss — but the same content hash. If the second fetch still avoids a
|
# from file keying — and both caches here are now content-keyed.
|
||||||
# transcode, only the content-keyed tier can have answered it. That is
|
#
|
||||||
# precisely what the migration bought, and it is unobservable any other
|
# So the assertion is that the second file costs NO transcode, and is
|
||||||
# way.
|
# served from RAM. Under the previous file-keyed memory cache it was a
|
||||||
|
# guaranteed RAM miss: two entries for identical bytes, and a DB lookup
|
||||||
|
# plus a blob read to fetch what was already in memory under another key.
|
||||||
|
#
|
||||||
|
# What this can no longer isolate is the DURABLE tier, because the RAM
|
||||||
|
# cache now answers first for anything within one process lifetime.
|
||||||
|
# That tier is covered instead by `satellites_consistency` (every row
|
||||||
|
# points at a live blob) and by a restart, which hurl cannot perform.
|
||||||
#
|
#
|
||||||
# ## Fixtures
|
# ## Fixtures
|
||||||
#
|
#
|
||||||
@@ -160,6 +167,7 @@ HTTP 200
|
|||||||
[Captures]
|
[Captures]
|
||||||
after_positive: jsonpath "$.transcodes"
|
after_positive: jsonpath "$.transcodes"
|
||||||
after_positive_disk: jsonpath "$.disk_hits"
|
after_positive_disk: jsonpath "$.disk_hits"
|
||||||
|
after_positive_ram: jsonpath "$.cache_hits"
|
||||||
[Asserts]
|
[Asserts]
|
||||||
jsonpath "$.not_beneficial" == {{base_not_beneficial}}
|
jsonpath "$.not_beneficial" == {{base_not_beneficial}}
|
||||||
|
|
||||||
@@ -210,9 +218,17 @@ Authorization: Bearer {{token}}
|
|||||||
|
|
||||||
HTTP 200
|
HTTP 200
|
||||||
[Asserts]
|
[Asserts]
|
||||||
|
# The property that matters, whichever tier answered: identical content is
|
||||||
|
# transcoded ONCE, however many files carry it.
|
||||||
jsonpath "$.transcodes" == {{after_positive}}
|
jsonpath "$.transcodes" == {{after_positive}}
|
||||||
jsonpath "$.not_beneficial" == {{base_not_beneficial}}
|
jsonpath "$.not_beneficial" == {{base_not_beneficial}}
|
||||||
jsonpath "$.disk_hits" != {{after_positive_disk}}
|
# Served from RAM, and that is the point of the memory cache being keyed by
|
||||||
|
# CONTENT rather than by file id. Under file keying this second file was a
|
||||||
|
# guaranteed RAM miss that fell through to a DB lookup plus a blob read to
|
||||||
|
# fetch bytes already in memory under another key — `disk_hits` moved and
|
||||||
|
# `cache_hits` did not. Now it is the other way round.
|
||||||
|
jsonpath "$.cache_hits" != {{after_positive_ram}}
|
||||||
|
jsonpath "$.disk_hits" == {{after_positive_disk}}
|
||||||
|
|
||||||
|
|
||||||
# ═════════════════════════════════════════════════════════════
|
# ═════════════════════════════════════════════════════════════
|
||||||
|
|||||||
Reference in New Issue
Block a user