Use Feuer Disk-Cache by PSeitz · Pull Request #6962 · quickwit-oss/quickwit

PSeitz · 2026-10-29T16:36:52Z

Description

Describe the proposed changes made in this PR.

How was this PR tested?

Describe how you tested this PR.

Port memory and disk cache integration from Pomsky while preserving upstream search behavior. Update configuration, metrics, documentation, and tests. Validation: 495 config/storage/search library tests passed on macOS; search, serve, and Lambda checks passed. Linux io_uring disk tests remain unverified. Generated with OpenAI gpt-6-astra.

Inline small test suites and consolidate overlapping cases and fixtures without changing production behavior. Validation: make fmt, git diff --check, and 484 config/storage/search library tests passed on macOS. Linux io_uring execution remains unverified. Generated with OpenAI gpt-6-astra.

Pin Feuer's portable zero-disk constructors and remove the direct feuer-memory dependency. Replace the separate fast-field cache with deprecated configuration aliases for memory-only split-range caching. Preserve shared-download cancellation across Feuer's initializer takeover and wait for metadata writes when testing disk recovery. Validation against the Git pin: 475 macOS and 500 Linux config/storage/search library tests passed. Search, serve, and Lambda compile checks, formatting, license generation, and diff checks passed. Generated with OpenAI gpt-6-astra.

PSeitz · 2026-10-29T16:37:06Z

Replaces #6958 to use the source branch PSeitz/quickwit:use_feuer. Both branches point to the same commit (0e0e4933f6c7605912116f9ead69c29fa163bd34), so this replacement changes the branch name, not the code.

Extract the download coalescer from quickwit-oss#6962 so split readers can batch nearby reads without Feuer. Copy proper subranges to avoid retaining larger merged allocations, and preserve cancellation and error propagation. AI-assisted extraction using openai/gpt-6-astra.

trinity-1686a

i still have to go through quickwit-search

trinity-1686a · 2026-11-02T10:21:52Z

+#   # Feuer fast-field RAM cache. Default 1G. Omitted while disk_cache
+#   # is set disables this cache; set a capacity explicitly to keep both.
 #   fast_field_cache_capacity: 1G


i wonder what we want to do about deprecated config options like that, i think we want to remove them from the sample documentation (but keep them in the configuration docs until actually removed)

my plan is to use their values for feuer

fast_field_cache_capacity + split_footer_capacity = feuer_capacity

I commented to as deprecated

trinity-1686a · 2026-11-02T11:09:21Z

+#   disk_cache:
+#     disk_capacity: 1500GiB
+#     memory_capacity: 15GiB
+#   # Alternatively, replace the scalar result-cache settings above with:


a disk cache with a memory capacity is a rare sight

renamed to storage_cache

storage_cache_directory is outside, because it is shared by the (holy) 3 caches

trinity-1686a · 2026-11-04T11:22:59Z

    /// Search split cache. `None` if no split cache is configured.
    pub split_cache_opt: Option<Arc<SearchSplitCache>>,
+    /// Process-wide cache for split byte ranges, with optional disk storage.
+    pub disk_cache_opt: Option<Arc<FeuerSplitRangeCache>>,


disk cache isn't a great name if the disk part is even optional

Called storage_cache now

trinity-1686a · 2026-11-04T15:09:51Z

            .predicate_cache
-            .get(split_id.clone(), term_absence_cache_key(term))
+            .get_async(split_id.clone(), term_absence_cache_key(term))
+            .await


should this get parallelized? (awaiting futures in a loop instead of concurrently)

it should, p95 accesstime can peak to a few seconds ... although I don't have it per requested size in the metrics. I'll update that

although we usually only have one hit here currently, so there's no real parallelism yet

trinity-1686a · 2026-11-04T15:10:01Z

+            ctx.searcher_context
+                .predicate_cache
+                .put_async(split_id.clone(), key, segment_id, HitSet::empty())
+                .await;


should this get parallelized? (awaiting futures in a loop instead of concurrently)

trinity-1686a · 2026-11-04T15:35:18Z


 /// A (half-open) range bounded inclusively below and exclusively above [start..end).
-#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Serialize)]


oh, that's interesting. I wonder if feuer should take a key that's Hash, and have a normal looking Hasher with a finish_u128() function so that it can be compatible with what people are used too. that might have limitations when used with type for which the Hash impl can easily cause collisions independantly of hashers though (i recall reading something about PathBuf)
going totally the other way, feuer could accept any Serialize + Eq, and internally use some sort of HashingSerializer so there's no need for json encoding in cache access
(these are ideas, not change requests)

hmm yes Serialize on the keys + hashing to u128 would make sense

Hash is actually less of a hassle than Serialize

trinity-1686a · 2026-11-04T15:40:10Z

+        assert!(
+            encoded_result.len() >= 32,
+            "invalid cached predicate header"
+        );


i'm a bit scared by these assertion. In the realm of a possibly persistent cache, that can mean a bad entry keeps a pod from recovering until a PV gets manually deleted, i'd prefer if we ignored the error, or cleared the faulty entry from cache (same for LeafSearchCache::get_async actually))

Yes, that's the biggest risk. In normal operation we should be safe by two mechanisms.

Every disk payload is checksummed

You can give the cache a payload version, so it will ignore the whole disk cache if a version changes. (so we don't need to version partial response cache etc.)

I changed it so it will ignore entries that can't be deserialized

(i'm not too worried about corruption, i haven't checked feuer disk yet, but i know about the checksumming, i'm more concerned about logic error inserting invalid values somehow; possibly through code that doesn't exist today but will exist tomorrow, which up to now was problematic but not that much, and with the assertion that was, would become very problematic. in general this would fit clearly in the "new failure mode" bucket, which is a bucket i'd rather keep small if not empty)

Without disk cache that's the same issue we have today, no?

trinity-1686a · 2026-11-04T15:43:48Z

+        }
+    }
+
+    pub fn disk(cache: FeuerCache) -> Self {


in memory cache gets metrics, but disk doesn't?

also i'm in favour of making async functions that require a runtime handle, even if they don't actually await

in memory cache gets metrics, but disk doesn't?

I enabled it for both. Long-term we may drop them since they are covered by Feuer

also i'm in favour of making async functions that require a runtime handle, even if they don't actually await

replaced it with sync put (which I added to Feuer)

trinity-1686a · 2026-11-04T15:51:55Z

+
+    /// Bridge Tantivy's synchronous predicate API. Disk-backed calls MUST run on a CPU or
+    /// blocking thread, never a Tokio worker. The saved runtime drives Feuer's I/O tasks.
+    pub fn block_on<F: Future>(&self, future: F) -> F::Output {


i don't like this function for reasons already stated. it's not clear to me why we need to call feueur from blocking context. if we do need that, having a block_on on ResultCache feels very strange. having something like a get_many_blocking could make sense so that at least we don't do multiple rtt to storage when we could do concurrent reads

Migrate to Feuer's Cache builder API at revision b5af890. Copy coalesced subranges into independent allocations so each entry retains and charges only its payload. Move exclusively owned whole buffers into the cache without copying. Update ownership tests and trace metadata for the pinned revision.

Combine legacy fast-field and footer budgets when disk_cache is omitted, preserving the default total of 1.5 GB. Let explicit disk_cache settings override those budgets while validating legacy options and rejecting overflow when summing capacities. Initialize memory-only range caching for standalone and Lambda searchers. Update configuration tests, cache coverage, and docs.

Parallelize required-term cache lookups and simplify cache documentation.

chatgpt-codex-connector · 2026-11-10T14:37:44Z

Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-11-10T14:45:53.244956Z	`41d35f3`	Draft marked ready

ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 41d35f3dfc

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-11-10T14:45:48Z

+                    ) || error.is::<io::Error>()) =>
+            {
+                let mut memory_config = config.clone();
+                memory_config.disk_capacity = bytesize::ByteSize::b(0);


Fail startup instead of discarding the disk tier

When a production node cannot open the configured cache because of a permission, filesystem, direct-I/O, or io_uring error, this branch rewrites the disk capacity to zero and lets startup succeed. The node then runs until restart without the configured disk capacity, potentially sending a large volume of unexpected misses to object storage; a warning and gauge do not make the deployment valid. Return the initialization error instead of hiding it behind a memory-only cache, as required for production-path failures.

AGENTS.md reference: AGENTS.md:L19-L19

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-11-10T14:45:49Z

+            return Ok(());
+        }
+        let _in_flight_guards = object_storage_get_slice_in_flight_guards(buffer.len());
+        default_get_slice_into(self, path, range, buffer).await?;


Retry Azure reads through the full response body

When an Azure download fails after its first chunk, this path calls default_get_slice_into, which consumes the reader after get_slice_stream's retry(...) closure has already returned (that closure only peeks the first chunk). Before this change, get_slice kept download_all inside the retry closure; with the default storage cache, every cache miss now uses this method, so a retryable later page/body error fails the split immediately. Keep the full body read under the Azure retry policy.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-11-10T14:45:49Z

            // TODO remove the clone here.
-            .get(split.clone(), search_request.clone())
+            .get_async(split.clone(), search_request.clone())
+            .await


Run per-split disk cache lookups concurrently

When partial_request_cache has a disk tier and a leaf request contains many splits, each lookup here can perform a disk read, but the surrounding loop does not start the next lookup until this .await completes. This puts the sum of all disk-hit latencies on the request's critical path before any search tasks are scheduled, which is especially costly for requests assigned hundreds or thousands of splits. Drive these lookups concurrently with a sensible bound, then merge the results.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-11-10T14:45:49Z

+        range: Range<usize>,
+        buffer: &mut [u8],
+    ) -> StorageResult<()> {
+        crate::default_get_slice_into(self, path, range, buffer).await


Instrument GCS get_slice_into reads

When a storage-cache miss reads a GCS-backed split, Feuer calls this new get_slice_into path. It delegates directly to get_slice_stream, bypassing the in-flight guards, OBJECT_STORAGE_GET_TOTAL increment, and duration timer used by get_slice, so those production GCS search GETs disappear from the existing object-storage telemetry. Add equivalent instrumentation around the streamed read.

Useful? React with 👍 / 👎.

PSeitz added 3 commits October 27, 2026 17:07

PSeitz added area:storage S3, GCS, Azure and local storage type:enhancement An improvement to something that exists, or its deprecation or removal area:search Root and leaf search, caches, sorting, pagination, Lambda offload labels Oct 29, 2026

This was referenced Oct 29, 2026

Use Feuer Disk-Cache #6958

Closed

Fix OOM - Streaming Coalescing in Quickwit #6985

Draft

trinity-1686a reviewed Nov 4, 2026

View reviewed changes

PSeitz added 12 commits November 5, 2026 15:39

Rename disk_cache to storage_cache and remove download coalescing

4bd194f

Describe caches without implementation names in user documentation

ee35c70

Remove FeuerSplitRangeCache wrapper

295d2f7

Trim cache documentation and restore existing cache settings

09a18a4

Register all Feuer metrics through Quickwit declarations

92b7075

Clarify cache names and remove redundant qualifiers

234b414

Trim cache integration diff and preserve configuration names

0d9a416

Address cache review feedback and simplify documentation

c650469

Simplify cache integration and mark legacy configuration fields

f791c61

Resolve predicate cache lookups asynchronously before query construction

f1c34fb

PSeitz added 25 commits November 9, 2026 07:53

Simplify Feuer cache integration and tests

580750c

Rename split range cache to storage cache

a0d83cc

Treat malformed cached search results as cache misses

7024ed5

Require explicit get_slice_into implementations

eab506f

Use one shared directory for Feuer caches

c09eaa5

Document disk cache requirements

e79ec7f

Simplify cache lookup, deserialization, and test helpers

0a3d5de

Simplify Feuer cache ownership and initialization

fec246a

Use synchronous Feuer cache insertion

4a52731

Reuse split fixtures in storage cache tests

a2cba3c

Use typed Hash keys with Feuer

cd0c089

Simplify warmup caching and Lambda collector locking

c2402f6

Simplify cache config resolution and test fixtures

68de86a

Simplify cache error mapping and restore derive order

bfe4bbc

Simplify memory capacity documentation

447351e

Fall back to memory on disk cache startup failures

7edeeec

Parallelize required-term cache lookups and simplify cache documentation.

Use imports for cache types and helpers

fcde1cd

Retain Quickwit lookup metrics for disk-backed result caches

c96873e

Update Feuer for synchronous cache opening

24cd68d

Document deprecated cache budgets in sample configuration

6b205dc

Allow zero capacities to disable storage caches

8f2763e

Validate legacy cache settings during deserialization

131a602

Simplify legacy cache budget comments

fbdc913

Link to Feuer metrics on main

ed8d4f7

Fix storage timeout success accounting and clarify retry tests

41d35f3

PSeitz marked this pull request as ready for review November 10, 2026 14:37

PSeitz requested a review from a team as a code owner November 10, 2026 14:37

datadog-official Bot added the type:feature A new capability, such as an endpoint, query type, source, setting or command label Nov 10, 2026

chatgpt-codex-connector Bot reviewed Nov 10, 2026

View reviewed changes

Conversation

PSeitz commented Oct 29, 2026

Description

How was this PR tested?

Uh oh!

PSeitz commented Oct 29, 2026

Uh oh!

trinity-1686a left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PSeitz Nov 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

PSeitz Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot commented Nov 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Nov 10, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Nov 10, 2026

PSeitz Nov 7, 2026 •

edited

Loading

PSeitz Nov 10, 2026 •

edited

Loading

chatgpt-codex-connector Bot commented Nov 10, 2026 •

edited

Loading