Skip to content

Commit fc5f101

Browse files
author
Diocrafts
committed
perf(thumbnails): cache-first + ETag 304 eliminates DB queries on GET
Solution A — Cache-first path: - get_thumbnail now tries moka (RAM) → disk BEFORE any DB query - Ownership was verified at creation time; UUIDv4 prevents enumeration - 146-photo timeline: 0 SQL queries instead of 146 per page load Solution C — ETag / If-None-Match short-circuit: - Deterministic ETag 'thumb-{id}-{size}' on every response - If browser sends matching If-None-Match → 304 with zero I/O - Cache-Control: immutable prevents even conditional requests Additional improvements: - Remove moka TTL (was 600s); thumbnails are immutable, weight-only eviction - generate_all_sizes_background populates moka after disk write; first GET after upload served from RAM with zero disk I/O - DB path only taken on cache miss for images needing generation
1 parent f5dd2b9 commit fc5f101

File tree

2 files changed

+70
-26
lines changed

2 files changed

+70
-26
lines changed

src/infrastructure/services/thumbnail_service.rs

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,16 @@ impl ThumbnailService {
105105
// for variable-size thumbnails than entry-count limits.
106106
let _ = max_cache_entries;
107107

108+
// No time_to_live — thumbnails are immutable (content never changes
109+
// for a given file_id). Eviction is purely weight-based: when the
110+
// cache exceeds max_cache_bytes the lightest entries are dropped.
111+
// On eviction the thumbnail is still on disk; the next request
112+
// promotes it back with a single async read (~0.1 ms).
108113
let cache = moka::future::Cache::builder()
109114
.max_capacity(max_cache_bytes as u64)
110115
.weigher(|_key: &ThumbnailCacheKey, value: &Bytes| -> u32 {
111116
value.len().min(u32::MAX as usize) as u32
112117
})
113-
.time_to_live(std::time::Duration::from_secs(600))
114118
.build();
115119

116120
Self {
@@ -512,7 +516,8 @@ impl ThumbnailService {
512516
}
513517
};
514518

515-
// Save each size to disk (async I/O, very fast for small WebP files)
519+
// Save each size to disk AND populate moka so the very first
520+
// GET after upload is served from RAM (zero disk I/O).
516521
for (size, bytes) in thumbnails {
517522
let thumb_path = self.get_thumbnail_path(&file_id, size);
518523
if let Some(parent) = thumb_path.parent() {
@@ -521,6 +526,12 @@ impl ThumbnailService {
521526
if let Err(e) = fs::write(&thumb_path, &bytes).await {
522527
tracing::warn!("Failed to save thumbnail {} {:?}: {}", file_id, size, e);
523528
} else {
529+
// Populate in-memory cache for instant first-hit serving
530+
let cache_key = ThumbnailCacheKey {
531+
file_id: file_id.clone(),
532+
size,
533+
};
534+
self.cache.insert(cache_key, bytes).await;
524535
tracing::debug!("✅ Generated thumbnail: {} {:?}", file_id, size);
525536
}
526537
}

src/interfaces/api/handlers/file_handler.rs

Lines changed: 57 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -291,18 +291,29 @@ impl FileHandler {
291291
// THUMBNAILS
292292
// ═══════════════════════════════════════════════════════════════════════
293293

294-
/// Get a thumbnail for an image file.
294+
/// Get a thumbnail for a file (image or video).
295295
///
296-
/// Thumbnail orchestration (path resolution, generation, caching) stays here
297-
/// because it is tightly coupled to HTTP response headers.
296+
/// **Cache-first**: if the thumbnail already exists in the moka in-memory
297+
/// cache or on disk, serve it immediately — **zero DB queries**. The
298+
/// ownership check was already performed when the thumbnail was first
299+
/// generated (at upload) or uploaded (PUT by the owner). UUIDv4 file IDs
300+
/// have 122 bits of entropy, making enumeration infeasible.
301+
///
302+
/// **ETag / 304**: responses carry an immutable ETag. If the browser
303+
/// sends `If-None-Match` matching the ETag, we return 304 Not Modified
304+
/// without touching cache or DB — pure header round-trip.
305+
///
306+
/// The DB path is only taken on a **cache miss for images** where the
307+
/// thumbnail hasn't been generated yet (first access after upload if
308+
/// background generation hasn't finished).
298309
pub async fn get_thumbnail(
299310
State(state): State<GlobalState>,
300311
auth_user: AuthUser,
312+
headers: HeaderMap,
301313
Path((id, size)): Path<(String, String)>,
302314
) -> impl IntoResponse {
303315
use crate::application::ports::thumbnail_ports::ThumbnailSize;
304316

305-
let file_retrieval_service = &state.applications.file_retrieval_service;
306317
let thumbnail_service = &state.core.thumbnail_service;
307318

308319
let thumb_size = match size.as_str() {
@@ -320,6 +331,46 @@ impl FileHandler {
320331
}
321332
};
322333

334+
// ── ETag short-circuit (Solution C) ──────────────────────────
335+
// Thumbnails are immutable — the ETag never changes for a given
336+
// (file_id, size) pair. If the browser already has it, return 304
337+
// with zero I/O or DB work.
338+
let etag = format!("\"thumb-{}-{:?}\"", id, thumb_size);
339+
if let Some(if_none_match) = headers.get(header::IF_NONE_MATCH) {
340+
if let Ok(val) = if_none_match.to_str() {
341+
if val == etag || val == "*" {
342+
return Response::builder()
343+
.status(StatusCode::NOT_MODIFIED)
344+
.header(header::ETAG, &etag)
345+
.header(header::CACHE_CONTROL, "public, max-age=31536000, immutable")
346+
.body(Body::empty())
347+
.unwrap()
348+
.into_response();
349+
}
350+
}
351+
}
352+
353+
// ── Cache-first path (Solution A) ────────────────────────────
354+
// Try moka (RAM) → disk before touching the database.
355+
// If the thumbnail exists it was authorized at creation time.
356+
if let Some(data) = thumbnail_service
357+
.get_cached_thumbnail(&id, thumb_size.into())
358+
.await
359+
{
360+
return Response::builder()
361+
.status(StatusCode::OK)
362+
.header(header::CONTENT_TYPE, "image/webp")
363+
.header(header::CONTENT_LENGTH, data.len())
364+
.header(header::CACHE_CONTROL, "public, max-age=31536000, immutable")
365+
.header(header::ETAG, &etag)
366+
.body(Body::from(data))
367+
.unwrap()
368+
.into_response();
369+
}
370+
371+
// ── Cache miss — need DB for ownership + blob resolution ─────
372+
let file_retrieval_service = &state.applications.file_retrieval_service;
373+
323374
let file = match file_retrieval_service
324375
.get_file_owned(&id, auth_user.id)
325376
.await
@@ -330,25 +381,8 @@ impl FileHandler {
330381
}
331382
};
332383

384+
// Non-image (video, etc.) with no cached thumbnail → 204
333385
if !thumbnail_service.is_supported_image(&file.mime_type) {
334-
// For non-images (videos, etc.), serve from cache if available;
335-
// otherwise return 204 to signal "not yet generated — please
336-
// generate client-side and upload via PUT".
337-
if let Some(data) = thumbnail_service
338-
.get_cached_thumbnail(&id, thumb_size.into())
339-
.await
340-
{
341-
let etag = format!("\"thumb-{}-{:?}\"", id, thumb_size);
342-
return Response::builder()
343-
.status(StatusCode::OK)
344-
.header(header::CONTENT_TYPE, "image/webp")
345-
.header(header::CONTENT_LENGTH, data.len())
346-
.header(header::CACHE_CONTROL, "public, max-age=31536000, immutable")
347-
.header(header::ETAG, etag)
348-
.body(Body::from(data))
349-
.unwrap()
350-
.into_response();
351-
}
352386
return Response::builder()
353387
.status(StatusCode::NO_CONTENT)
354388
.header(header::CACHE_CONTROL, "no-store")
@@ -376,13 +410,12 @@ impl FileHandler {
376410
.await
377411
{
378412
Ok(data) => {
379-
let etag = format!("\"thumb-{}-{:?}\"", id, thumb_size);
380413
Response::builder()
381414
.status(StatusCode::OK)
382415
.header(header::CONTENT_TYPE, "image/webp")
383416
.header(header::CONTENT_LENGTH, data.len())
384417
.header(header::CACHE_CONTROL, "public, max-age=31536000, immutable")
385-
.header(header::ETAG, etag)
418+
.header(header::ETAG, &etag)
386419
.body(Body::from(data))
387420
.unwrap()
388421
.into_response()

0 commit comments

Comments
 (0)