refactor(storage): download manager — collapse the async onion on the search read path#6580
Draft
PSeitz-dd wants to merge 2 commits into
Draft
refactor(storage): download manager — collapse the async onion on the search read path#6580PSeitz-dd wants to merge 2 commits into
PSeitz-dd wants to merge 2 commits into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Collapses the S3 search read path from a deep stack of
#[async_trait] impl Storagedecorators + tantivyDirectorywrappers into a synchronous decision core plus one thin async download layer, and unifies caching inquickwit-storage.Historically, fetching a byte range of a split traversed timeout/retry → fast-field cache → on-disk split cache → request debouncing → bundle-offset translation → ephemeral byte-range cache → static hotcache. Every layer allocated a boxed future and added an
.await, even though only the actual network/disk read performs I/O.How
New
SplitDownloadViewinquickwit-storage/src/download_manager/:resolve()performs every cache lookup, the bundle logical→physical offset translation, and dedup keying without allocating a single future. ReturnsResolution::Hit(OwnedBytes)(zero futures, immediate) orResolution::Miss(DownloadPlan).download(plan)performs the actual read (on-disk split-cache fill or object-store GET), bounded by the backend's existing semaphore, debounced and retried, then writes the result back into the caches synchronously on completion.Caching is unified and keyed by the physical split path joined with the logical file name inside the split, plus the file-relative byte range — keeping the long-term
.fastfield cache and the ephemeral byte-range cache cleanly differentiated per file while remaining globally unique (split files are ULID-named).Removed / folded
timeout_and_retry_storage.rs(−279) — folded into the singledownload()layercache/quickwit_cache.rs(−207) — unified into the download managerdirectories/storage_directory.rs(−142) — replaced bydownload_manager_directory.rsdebouncer.rsslimmed (−118); theStoragetrait is retained unchangedProfiling (prod A/B on
cloudprem-ws-pascal-searcher*)Compared a download-manager build against the previous build under comparable load (~4.1–4.4 cores).
DownloadManagerFileHandle::read_bytes_async → download_manager::download::fetch_slice → S3, versus the old 6-layerHotDirectory → CachingDirectory → StorageDirectory → StorageWithCache → DebouncedStorage → S3chain. Debouncing is preserved (moved inside the manager).Notes
analysis.md(design doc + decisions) is included in this branch for review context; can be dropped before merge if undesired.Storagetrait is intentionally kept — no newObjectDownloadtrait.Checklist
cargo clippy --workspace --all-features --testscargo +nightly fmt --all -- --checkcargo nextest run --all-features🤖 Generated with Claude Code