fix: eliminate N+1 blob URL fetches and limit concurrency#21
fix: eliminate N+1 blob URL fetches and limit concurrency#21sentry[bot] wants to merge 1 commit into
Conversation
|
Deploying with
|
| Status | Preview URL | Commit | Alias | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! | https://pr-21-sable.justin-tech.workers.dev | 5c44822 | pr-21 |
Tue, 02 Jun 2026 20:35:59 GMT |
Merged changes from #21 (Sentry bot PR): 1. **Eliminate redundant `blob:` URL fetches**: Added check to detect if URL is already a `blob:` URL. If so, bypass network fetch and use directly from memory. Resolves N+1 API call pattern on initial load. 2. **Implement concurrency limiting**: Added module-scoped concurrency limiter (MAX_CONCURRENT_FETCHES = 4) to cap simultaneous remote media fetches. Requests exceeding limit are queued, preventing network contention and N+1 detection for remote resources. 3. **Enhanced observability**: Updated HealthMonitor to report queueDepth as Sentry metric (sable.media.fetch_queue_depth) for monitoring concurrency limiter effectiveness. Changes integrated into existing Cache API implementation in integration branch while preserving auth failure tracking and persistent cache logic. Fixes SABLE-3H
There was a problem hiding this comment.
Pull request overview
This PR optimizes media fetching in the client by avoiding redundant fetches for in-memory blob: object URLs and by adding a concurrency limiter for remote media fetches, with added Sentry metrics to monitor cache and queue behavior.
Changes:
- Short-circuit
useBlobCacheforblob:URLs to prevent redundant re-fetching. - Add a module-scoped concurrency limiter (max 4) to reduce simultaneous remote media fetches.
- Extend
HealthMonitorSentry metrics to report the blob fetch queue depth.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/app/pages/client/ClientNonUIFeatures.tsx | Adds Sentry metric for blob fetch queue depth (and consumes updated cache stats). |
| src/app/hooks/useBlobCache.ts | Avoids fetching blob: URLs and introduces a concurrency-limited fetch queue for remote URLs, expanding cache stats. |
Comments suppressed due to low confidence (1)
src/app/pages/client/ClientNonUIFeatures.tsx:590
- After introducing the fetch queue, the count from
inflightRequests.sizeno longer represents active network requests; it includes queued work too. This makes thesable.media.inflight_requestsgauge and the “High inflight request count” breadcrumb message misleading. Consider reportingactiveFetchCountas inflight, and optionally emitpendingCount(active + queued) / keep the breadcrumb threshold on pending to preserve N+1 observability.
const { cacheSize, inflightCount, queueDepth } = getBlobCacheStats();
Sentry.metrics.gauge('sable.media.blob_cache_size', cacheSize);
if (inflightCount > 0) {
Sentry.metrics.gauge('sable.media.inflight_requests', inflightCount);
if (inflightCount >= 10) {
| export function getBlobCacheStats(): { | ||
| cacheSize: number; | ||
| inflightCount: number; | ||
| queueDepth: number; | ||
| } { | ||
| return { | ||
| cacheSize: imageBlobCache.size, | ||
| inflightCount: inflightRequests.size, | ||
| queueDepth: fetchQueue.length, | ||
| }; | ||
| } |
Description
This PR addresses the N+1 API Call issue (SABLE-3H) observed for
blob:URLs and reduces concurrency for other media fetches.Problem:
On initial page load, when many components (e.g., room avatars/thumbnails) render simultaneously,
useBlobCachewas triggering an N+1 API call pattern. Specifically, the trace showed numerousGET blob:https://sable.cloudhub.social/...requests. Theseblob:URLs are already in-memory object URLs, so re-fetching them was redundant and inefficient. Additionally, for actual remote media URLs, a large number of concurrent fetches could occur, leading to network contention and performance issues.Solution:
blob:URL fetches: A check was added touseBlobCacheto detect if the provided URL is already ablob:URL. If so, it's immediately treated as cached and returned, bypassing any networkfetch()call. This directly resolves the N+1 issue forblob:URLs.MAX_CONCURRENT_FETCHES = 4) was introduced inuseBlobCache. This ensures that only a limited number of remote media fetches can occur simultaneously. Any additional requests are queued, preventing a burst of network activity and reducing the likelihood of N+1 detection for remote resources.HealthMonitorinClientNonUIFeatures.tsxwas updated to report thequeueDepthof the blob fetch queue as a Sentry metric (sable.media.fetch_queue_depth). This allows for monitoring the effectiveness of the concurrency limiter and identifying potential bottlenecks.These changes significantly reduce unnecessary network activity and improve the perceived performance of the application during initial load and when rendering lists of media.
Fixes SABLE-3H
Type of change
Checklist:
AI disclosure:
Fixes SABLE-3H