cjw/Oxicloud
1
0
Fork 0
Code Issues Pull Requests Actions 4 Packages Projects Releases Wiki Activity

perf(thumbnail): cache attached-blob lookups on the request path

Browse Source 
Every thumbnail request paid an uncached storage.file_attached_blobs
point query before it could answer — including 304 revalidations and
RAM thumbnail hits, where the ETag path (thumbnail_content_id) probes
the row every time and tier 2b probes it again with the same key. A
photos grid revalidating 60 thumbnails per visit meant 60+ point
queries per browse, repeated on every visit.

find_attached_blob now reads through a process-local moka cache in
DedupService, keyed by the row's (file_id, kind, variant) PK, holding
positive and negative entries (most files have no attached preview, so
the negative side carries the win). Two rules keep it honest:

- DB faults are surfaced as Err and never cached — a transient outage
  cannot freeze "no attached blob" into a negative entry (a read
  failure is never proof that data is absent). The public signature is
  unchanged; the SQL body moved to find_attached_blob_uncached.
- Writes invalidate eagerly: store_attached_blob and the Inserted arm
  of store_attached_blob_if_absent on success, and deletions via
  ThumbnailRefreshHook::on_file_deleted, which all three production
  delete paths (single file, folder cascade, trash clear) fire after
  the DELETE commits. The 60s TTL bounds only what the process cannot
  see (bare SQL, copy_file_satellites races).

The Nextcloud preview endpoint rides the same lookup and benefits
identically. Five in-memory contract tests pin the cache behaviour,
including the fault-not-cached rule.

Co-Authored-By: Claude Code <noreply@anthropic.com>
This commit is contained in:
SZCJW
2026-09-20 00:28:00 +08:00
parent 68e21f4bef
commit d33d1932b6
4 changed files with 340 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/infrastructure/services/dedup_service.rs
+296 -7
View File
@@ -58,7 +58,7 @@ use crate::application::ports::blob_lifecycle::BlobLifecycleHook;
use crate::application::ports::blob_reference_ports::{BlobReferenceRegistry, RefLevel};
use crate::application::ports::blob_storage_ports::BlobStorageBackend;
use crate::application::ports::dedup_ports::{
BlobMetadataDto, DedupPort, DedupResultDto, DedupStatsDto,
BlobMetadataDto, DedupPort, DedupResultDto, DedupStatsDto, DerivedBlobRef,
};
use crate::application::services::blob_lifecycle_service::BlobLifecycleService;
use crate::domain::errors::{DomainError, ErrorKind};
@@ -646,6 +646,49 @@ fn manifest_reap_sql(registry: &BlobReferenceRegistry) -> String {
)
}
// ── Attached-blob lookup cache ───────────────────────────────────────────────
/// Cache size cap for [`DedupService::attached_blob_cache`] — plain entry
/// count (no weigher): an entry is three short strings + two short strings,
/// tens of bytes; 50k entries ≈ a few MB, noise next to the manifest cache.
pub(crate) const ATTACHED_BLOB_CACHE_MAX_ENTRIES: u64 = 50_000;
/// Hard staleness bound for [`DedupService::attached_blob_cache`].
///
/// Deliberately [`moka::future::Cache::builder().time_to_live`] and NOT
/// `time_to_idle`: a hot negative entry under TTI never expires, and TTL must
/// be the last-resort bound for writes this process never saw (bare SQL, a
/// future second instance, the `copy_file_satellites` race window).
pub(crate) const ATTACHED_BLOB_CACHE_TTL_SECS: u64 = 60;
/// Cache key for [`DedupService::attached_blob_cache`] — the
/// `storage.file_attached_blobs` primary key. A struct, not a
/// `(String, String, String)` tuple: three same-typed fields read by position
/// would force every construction site (and the `invalidate_for_file` scan)
/// to guess semantics; self-documenting beats positional here.
#[derive(Clone, PartialEq, Eq, Hash, Debug)]
struct AttachedBlobKey {
file_id: String,
kind: String,
variant: String,
}
impl AttachedBlobKey {
fn new(file_id: &str, kind: &str, variant: &str) -> Self {
Self {
file_id: file_id.to_string(),
kind: kind.to_string(),
variant: variant.to_string(),
}
}
}
/// Loader-error sentinel for the `try_get_with` cache wrapper on
/// [`Self::find_attached_blob`]. The SQL lookup treats a DB fault the same as
/// "no row" only at the very last moment — the cache must never see it, or a
/// transient outage would freeze "no attached blob" into place for a full
/// TTL while rows exist (a read failure is never proof that data is absent).
struct AttachedLookupFault;
pub struct DedupService {
/// Pluggable blob storage backend (local FS, S3, …).
backend: Arc<dyn BlobStorageBackend>,
@@ -664,6 +707,28 @@ pub struct DedupService {
/// seen immediately), weight-bounded (a manifest is ~72 B per chunk),
/// short TTL so GC'd manifests age out fast (benches/MANIFEST-CACHE.md).
manifest_cache: moka::future::Cache<String, Arc<ChunkManifest>>,
/// `file_id → attached blob` lookup cache (`storage.file_attached_blobs`
/// rows) for the thumbnail hot path — `ThumbnailService::
/// thumbnail_content_id` hits it on EVERY request (including 304
/// revalidations and RAM thumbnail hits, `thumbnail_service.rs` ~:744)
/// and `get_cached_thumbnail` tier 2b hits it again with the same key
/// (~:851); the Nextcloud preview endpoint rides the same lookup.
///
/// Positive AND negative (`Option<DerivedBlobRef>` — most files have no
/// attached preview row, so the negative side is where the win is). The
/// loader NEVER caches a DB error: `find_attached_blob_uncached` returns
/// `Err` and `try_get_with` drops it, so a transient outage cannot freeze
/// "no attached blob" into the cache for a full TTL (a read failure is
/// never proof that data is absent).
///
/// Writes invalidate through the same type: `store_attached_blob` /
/// `store_attached_blob_if_absent` on success, file deletions via the
/// `ThumbnailRefreshHook::on_file_deleted` piggyback. The TTL above
/// remains the bound for anything this process cannot see (bare SQL,
/// `copy_file_satellites` races); invalidate-vs-inflight-REFILL races are
/// narrowed by `try_get_with` but not eliminated, and the residual window
/// is ≤ one TTL.
attached_blob_cache: moka::future::Cache<AttachedBlobKey, Option<DerivedBlobRef>>,
/// Every table that holds blob references, so GC agrees with the
/// consistency jobs on what "referenced" means. Defaults to the two
/// built-in sources; DI replaces it once more tables exist. Never
@@ -698,6 +763,7 @@ impl DedupService {
maintenance_pool,
blob_lifecycle: None,
manifest_cache: Self::build_manifest_cache(),
attached_blob_cache: Self::build_attached_blob_cache(),
reference_registry: registry.clone(),
manifest_reap_sql: manifest_reap_sql(&registry),
blob_reap_sql: blob_reap_sql(&registry),
@@ -729,6 +795,19 @@ impl DedupService {
.build()
}
/// See the `attached_blob_cache` field docs. Plain entry-count cap (no
/// weigher — an entry is a handful of short strings), TTL as the hard
/// staleness bound; same hard-coded-const treatment as the manifest
/// cache rather than config: an internal accelerator with strict
/// write-side invalidation, where a misconfiguration costs performance,
/// never correctness.
fn build_attached_blob_cache() -> moka::future::Cache<AttachedBlobKey, Option<DerivedBlobRef>> {
moka::future::Cache::builder()
.max_capacity(ATTACHED_BLOB_CACHE_MAX_ENTRIES)
.time_to_live(std::time::Duration::from_secs(ATTACHED_BLOB_CACHE_TTL_SECS))
.build()
}
/// Registers the blob-reference registry used by the manifest reap
/// predicate. Without it `garbage_collect` skips manifest collection
/// entirely — see `docs/plan/derived-blobs.md`.
@@ -831,6 +910,14 @@ impl DedupService {
.await
.map_err(|e| DomainError::internal_error("Dedup", format!("record attached blob: {e}")))?;
// The row is replaced — drop any cached (possibly negative) entry so
// the next lookup refills from the new truth. Only on the success
// path: if the execute had failed, the row is unchanged and the
// cache is still accurate, so invalidating would just cost a refill.
self.attached_blob_cache
.invalidate(&AttachedBlobKey::new(file_id, kind, variant))
.await;
// Two shapes to balance depending on whether the UPSERT was a
// real content replacement or a same-content re-store:
//
@@ -960,12 +1047,27 @@ impl DedupService {
// if the row gets updated in it, the sidecar delete
// path fails its verify and keeps the sidecar — the
// conservative fallback.
//
// This readback now flows through the `attached_blob_cache`.
// Safe in-process: any write this process made already
// invalidated the key. The only degraded case is a negative
// entry cached before some OTHER process inserted the row —
// nonexistent in a single-instance deployment, and even then
// the consequence is `existing_hash: ""` → the import keeps
// its sidecar, the documented conservative fallback.
let existing = self.find_attached_blob(file_id, kind, variant).await;
return Ok(AttachedBlobInsertOutcome::AlreadyPresent {
existing_hash: existing.map(|r| r.blob_hash).unwrap_or_default(),
});
}
// We wrote a row for a key the cache may hold a negative entry for
// (the common "import backfill" case) — drop it so the new row is
// immediately visible to the thumbnail path.
self.attached_blob_cache
.invalidate(&AttachedBlobKey::new(file_id, kind, variant))
.await;
Ok(AttachedBlobInsertOutcome::Inserted {
hash: attached_hash,
})
@@ -973,12 +1075,51 @@ impl DedupService {
/// Look up bytes attached to a file. File-keyed counterpart of
/// [`Self::find_derived_blob`].
///
/// Cached read-through of [`Self::attached_blob_cache`] (positive AND
/// negative); see the field docs for why. The public signature is
/// unchanged — including the historical "DB fault reads as no row"
/// behaviour — but the fault now dies BEFORE the cache instead of being
/// indistinguishable from an absent row.
pub async fn find_attached_blob(
&self,
file_id: &str,
kind: &str,
variant: &str,
) -> Option<crate::application::ports::dedup_ports::DerivedBlobRef> {
) -> Option<DerivedBlobRef> {
match self
.attached_blob_cache
.try_get_with(AttachedBlobKey::new(file_id, kind, variant), async {
self.find_attached_blob_uncached(file_id, kind, variant)
.await
.map_err(|_| AttachedLookupFault) // Err ⇒ never cached
})
.await
{
Ok(attached) => attached,
Err(_) => {
tracing::debug!(
target: "oxicloud::dedup",
"attached-blob lookup failed (not cached): file={} kind={} variant={}",
file_id,
kind,
variant
);
None
}
}
}
/// The uncached lookup — one indexed point query on the
/// `file_attached_blobs` primary key. Unlike the historical inlined
/// body, a DB fault surfaces as `Err` so the cache wrapper can refuse to
/// store it; only a genuine `Ok(None)` means "no row".
async fn find_attached_blob_uncached(
&self,
file_id: &str,
kind: &str,
variant: &str,
) -> sqlx::Result<Option<DerivedBlobRef>> {
sqlx::query_as::<_, (String, String)>(
"SELECT blob_hash, content_type FROM storage.file_attached_blobs
WHERE file_id = $1::uuid AND kind = $2 AND variant = $3",
@@ -988,16 +1129,31 @@ impl DedupService {
.bind(variant)
.fetch_optional(self.pool.as_ref())
.await
.ok()
.flatten()
.map(|(blob_hash, content_type)| {
crate::application::ports::dedup_ports::DerivedBlobRef {
.map(|row| {
row.map(|(blob_hash, content_type)| DerivedBlobRef {
blob_hash,
content_type,
}
})
})
}
/// Invalidate every `(kind, variant)` entry cached for one file.
///
/// Fired from `ThumbnailRefreshHook::on_file_deleted` so all three
/// production delete paths (single file, folder cascade, trash clear)
/// drop their cached rows after the DELETE commits. A linear scan over
/// the keys is fine here: deletions are rare and the cache is capped at
/// [`ATTACHED_BLOB_CACHE_MAX_ENTRIES`].
pub async fn invalidate_attached_blobs_for_file(&self, file_id: &str) {
// moka's `Iter` yields `(Arc<K>, V)` synchronously — the await lives
// in `invalidate`, not in the scan itself.
for (key, _) in self.attached_blob_cache.iter() {
if key.file_id == file_id {
self.attached_blob_cache.invalidate(&*key).await;
}
}
}
pub async fn store_derived_blob(
&self,
source_hash: &str,
@@ -1307,6 +1463,7 @@ impl DedupService {
maintenance_pool: stub_pool.clone(),
blob_lifecycle: None,
manifest_cache: Self::build_manifest_cache(),
attached_blob_cache: Self::build_attached_blob_cache(),
reference_registry: stub_registry.clone(),
manifest_reap_sql: manifest_reap_sql(&stub_registry),
blob_reap_sql: blob_reap_sql(&stub_registry),
@@ -4184,6 +4341,138 @@ impl crate::infrastructure::scheduler::JobHandler for DedupService {
mod tests {
use super::*;
// ── attached_blob_cache — find_attached_blob read-through ───────────────
//
// Pure in-memory contract tests: `new_stub()` connects lazily to an
// unreachable pool, so anything that reaches the "DB" fails loudly. That
// is exactly what makes these work — a served `Some` proves the cache was
// consulted, and a missing entry after a fault proves the fault was not
// cached. Same no-SQL style as the hash_cache tests in
// `file_blob_read_repository.rs`.
fn attached_key(file_id: &str, kind: &str, variant: &str) -> AttachedBlobKey {
AttachedBlobKey::new(file_id, kind, variant)
}
fn sample_ref(hash: &str) -> DerivedBlobRef {
DerivedBlobRef {
blob_hash: hash.to_string(),
content_type: "image/jpeg".to_string(),
}
}
/// A seeded entry is served without touching the (unreachable) stub pool
/// — returning `Some` at all proves the read-through hit the cache.
#[tokio::test]
async fn attached_lookup_serves_a_seeded_entry() {
let svc = DedupService::new_stub();
let k = attached_key("0189d1b3-8f2a-7cde-b1ad-000000000001", "preview", "icon");
svc.attached_blob_cache
.insert(k.clone(), Some(sample_ref("abc")))
.await;
assert_eq!(
svc.find_attached_blob(&k.file_id, "preview", "icon").await,
Some(sample_ref("abc"))
);
}
/// Negative entries are where most of the win is (most files have no
/// attached preview). A cached `None` must be served as `None` AND
/// survive the call — not be evicted by the miss path.
#[tokio::test]
async fn attached_lookup_serves_and_keeps_a_negative_entry() {
let svc = DedupService::new_stub();
let k = attached_key("0189d1b3-8f2a-7cde-b1ad-000000000002", "preview", "icon");
svc.attached_blob_cache.insert(k.clone(), None).await;
assert_eq!(
svc.find_attached_blob(&k.file_id, "preview", "icon").await,
None
);
assert!(
svc.attached_blob_cache.get(&k).await.is_some(),
"negative entry was dropped by the lookup"
);
}
/// THE contract this change exists for: a DB fault must not be cached.
/// The stub pool cannot connect, so the uncached lookup errors; the
/// wrapper returns `None` (historical behaviour) and leaves the cache
/// empty — a row that appears after a transient outage must be visible
/// on the very next call, not hidden behind a frozen negative entry.
#[tokio::test]
async fn attached_lookup_does_not_cache_a_db_fault() {
let svc = DedupService::new_stub();
let k = attached_key("0189d1b3-8f2a-7cde-b1ad-000000000003", "preview", "icon");
assert_eq!(
svc.find_attached_blob(&k.file_id, "preview", "icon").await,
None
);
assert!(
svc.attached_blob_cache.get(&k).await.is_none(),
"DB fault was cached as a negative entry"
);
}
/// Per-file invalidation drops every `(kind, variant)` of that file and
/// leaves other files' entries alone.
#[tokio::test]
async fn invalidate_attached_blobs_for_file_is_scoped_to_the_file() {
let svc = DedupService::new_stub();
let k1 = attached_key("0189d1b3-8f2a-7cde-b1ad-000000000004", "preview", "icon");
let k2 = attached_key("0189d1b3-8f2a-7cde-b1ad-000000000004", "preview", "large");
let k3 = attached_key("0189d1b3-8f2a-7cde-b1ad-000000000005", "preview", "icon");
for (k, v) in [
(k1.clone(), Some(sample_ref("a"))),
(k2.clone(), None),
(k3.clone(), Some(sample_ref("c"))),
] {
svc.attached_blob_cache.insert(k, v).await;
}
svc.invalidate_attached_blobs_for_file(&k1.file_id).await;
assert!(svc.attached_blob_cache.get(&k1).await.is_none());
assert!(svc.attached_blob_cache.get(&k2).await.is_none());
assert!(
svc.attached_blob_cache.get(&k3).await.is_some(),
"another file's entry must survive"
);
}
/// Invalidation happens only after a SUCCESSFUL write: the store path
/// fails (unreachable pool) before any row is touched, so the previously
/// cached entry must still be there. Invalidating on failure would be
/// harmless but pointless — the row is unchanged and the cache accurate.
#[tokio::test]
async fn failed_attached_store_leaves_the_cache_alone() {
let svc = DedupService::new_stub();
let k = attached_key("0189d1b3-8f2a-7cde-b1ad-000000000006", "preview", "icon");
svc.attached_blob_cache
.insert(k.clone(), Some(sample_ref("xyz")))
.await;
let result = svc
.store_attached_blob(
&k.file_id,
"preview",
"icon",
"image/png",
Bytes::from_static(b"nope"),
uuid::Uuid::nil(),
)
.await;
assert!(
result.is_err(),
"stub pool is unreachable — store must fail"
);
assert_eq!(
svc.attached_blob_cache.get(&k).await,
Some(Some(sample_ref("xyz"))),
"failed store must not disturb the cache"
);
}
/// Golden test for the statement `garbage_collect` runs against production
/// data. It is assembled from the registered reference sources rather than
/// written as a literal, so this pins the whole thing byte-for-byte — the
src/infrastructure/services/thumbnail_service.rs
+6
View File
@@ -1874,10 +1874,16 @@ impl crate::application::ports::file_lifecycle::FileLifecycleHook for ThumbnailR
fn on_file_deleted(&self, file_id: &str) {
let thumbnail = self.thumbnail.clone();
let file_id = file_id.to_string();
// The row is gone (CASCADE cleared file_attached_blobs) — drop any
// cached attached-blob lookup for this file too. TTL would bound the
// staleness anyway, but deletes are rare and the cache lookup after a
// delete is pure waste.
let dedup = self.dedup.clone();
tokio::spawn(async move {
if let Err(e) = thumbnail.delete_thumbnails(&file_id).await {
tracing::warn!("Failed to delete thumbnails for file {}: {}", file_id, e);
}
dedup.invalidate_attached_blobs_for_file(&file_id).await;
});
}
}