feat(storage): cache immutable split byte ranges - #6707
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 359def0140
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d42519e7c5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let object_uri = self | ||
| .inner | ||
| .uri() | ||
| .join(path) | ||
| .map_err(|error| StorageErrorKind::Internal.with_error(error))? | ||
| .into_string(); |
There was a problem hiding this comment.
When an S3-compatible deployment changes its configured endpoint while retaining the disk-cache directory, this key remains only s3://bucket/path plus the byte range. S3CompatibleObjectStorage stores the endpoint solely in its client configuration while uri() returns the original S3 URI, so a different endpoint with the same bucket/key can receive a recovered entry from the previous backend and return incorrect split bytes. Namespace persistent keys by the effective backend endpoint (or invalidate the cache when that identity changes).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 24d1a650f0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
24d1a65 to
d5dac9f
Compare
There was a problem hiding this comment.
💡 Codex Review
https://github.com/quickwit-oss/quickwit/blob/d5dac9f847af8f33dc61e548ac3fcea566daaefc/quickwit-storage/src/split_range_cache/storage.rs#L61
When block_size is below 4 KiB, such as a 2 KiB block with a 1 KiB maximum entry, the configuration passes SplitRangeDiskCacheConfig::validate, but the first cacheable read subtracts the 4 KiB blob index here. This underflows and panics in checked builds; in release builds it wraps to a huge slot size and incorrectly attempts disk admission. Validate that the block exceeds the reserved index or use checked subtraction and bypass disk admission.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Keep footer lookup on the range cache like main, and restore index_storage_with_split_cache for body reads only. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
💡 Codex Review
https://github.com/quickwit-oss/quickwit/blob/4702e6ee616099da3cf6c4b1717670fcab1b2139/quickwit-search/src/leaf.rs#L172
When both split_range_disk_cache and storage_timeout_policy are enabled, open_index_with_caches first creates TimeoutAndRetryStorage at lines 230-236, and this line then places Foyer outside that wrapper. Consequently, a stalled Foyer disk lookup or single-flight wait is not subject to the configured timeout or retries, so a search can block beyond the latency bound that previously covered every get_slice; wrap the completed cache stack with the timeout policy or enforce an equivalent timeout around cache access.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
1 participant
Summary
Storagedecorator that caches exact{object URI, byte range}slices in the process-wide Foyer hybrid cache.get_or_fetch; lower storage errors pass through unchanged; Foyer errors fail open to one direct lower read.max_entry_sizeor the Foyer block slot stay memory-only. Writes are rejected.split_range_disk_cacheis configured.open_split_bundlebefore the footer fetch so RAM footer cache → optional whole-split cache → Foyer → caller storage serves both footer and body reads.Stack
Test plan
cargo test -p quickwit-storage --lib split_rangecargo test -p quickwit-search split_range_cache_layercargo test -p quickwit-serve --all-features setup_searchercargo clippy -p quickwit-search --all-features --tests -- -D warnings