From 5fc23346bb7c79f396f9c7c374b58231236b99c4 Mon Sep 17 00:00:00 2001 From: Pascal Seitz Date: Thu, 25 Jun 2026 15:56:44 +0100 Subject: [PATCH 1/2] feat(search): cache never-match queries via the predicate cache Repeated super-selective or non-existent term queries over many splits re-ran warmup on every request. When warmup proves a split empty (a required term has an empty posting list), record a fake empty entry in the existing predicate cache, and consult it before warmup so later requests with the same predicate short-circuit with no storage reads. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../src/query_ast/cache_node.rs | 9 +- quickwit/quickwit-search/src/leaf.rs | 59 ++++++- quickwit/quickwit-search/src/tests.rs | 150 +++++++++++++++++- 3 files changed, 213 insertions(+), 5 deletions(-) diff --git a/quickwit/quickwit-query/src/query_ast/cache_node.rs b/quickwit/quickwit-query/src/query_ast/cache_node.rs index 9c449aef4fb..48eeaf794e4 100644 --- a/quickwit/quickwit-query/src/query_ast/cache_node.rs +++ b/quickwit/quickwit-query/src/query_ast/cache_node.rs @@ -212,11 +212,16 @@ pub struct HitSet { const INCOMPLETE_BLOCK_MARKER: u8 = 0x80; impl HitSet { - #[cfg(test)] - fn empty() -> Self { + /// An empty hit set, matching no document. + pub fn empty() -> Self { Self::from_buffer(OwnedBytes::new(vec![0, 0, 0, 0])) } + /// Returns true if this hit set matches no document. + pub fn is_empty(&self) -> bool { + self.size_hint() == 0 + } + /// Build a HitSet from its serialized form. /// /// The provided buffer must come from `HitSet::into_buffer` diff --git a/quickwit/quickwit-search/src/leaf.rs b/quickwit/quickwit-search/src/leaf.rs index b8a7c6e59a4..b905972e596 100644 --- a/quickwit/quickwit-search/src/leaf.rs +++ b/quickwit/quickwit-search/src/leaf.rs @@ -38,7 +38,8 @@ use quickwit_proto::search::{ SortOrder, SortValue, SplitIdAndFooterOffsets, SplitResourceStats, SplitSearchError, }; use quickwit_query::query_ast::{ - BoolQuery, CacheNode, QueryAst, QueryAstTransformer, RangeQuery, TermQuery, + BoolQuery, CacheNode, HitSet, PredicateCache, QueryAst, QueryAstTransformer, RangeQuery, + TermQuery, }; use quickwit_query::tokenizers::TokenizerManager; use quickwit_storage::{ @@ -621,6 +622,23 @@ fn compute_index_size(hot_directory: &HotDirectory) -> ByteSize { ByteSize(size_bytes) } +/// Cache key for the negative (never-match) cache. +/// +/// We reuse the predicate cache's store, so the key must match the one its +/// [`quickwit_query::query_ast::PredicateCacheInjector`] uses: the JSON of the +/// query AST, unwrapped from any top-level [`CacheNode`] (added for +/// `search_after`). The effective, split-intersected timestamp range is already +/// folded into the query AST by `rewrite_request`, so this key distinguishes time +/// windows; an entry is only ever reused for an identical predicate over the same +/// window on the same split. +fn negative_cache_key(query_ast: &QueryAst) -> Option { + let inner = match query_ast { + QueryAst::Cache(cache_node) => cache_node.inner.as_ref(), + other => other, + }; + serde_json::to_string(inner).ok() +} + /// Apply a leaf search on a single split. async fn leaf_search_single_split( search_request: SearchRequest, @@ -712,7 +730,29 @@ async fn leaf_search_single_split( let warmup_start = Instant::now(); leaf_search_state_guard.set_state(SplitSearchState::WarmUp); - let warmup_outcome = warmup(&searcher, &warmup_info).await?; + // Negative cache: if this exact predicate was previously proven to match no + // document in this split, skip warmup (and the search) entirely. Reusing the + // predicate cache's store means an empty entry recorded by either this path + // or the predicate cache's filler short-circuits the request. An empty result + // is segment- and scoring-agnostic, so this is sound even for scored queries + // (which the `CacheNode` machinery itself does not support). + let negative_key = negative_cache_key(&query_ast); + let cached_known_empty = match &negative_key { + Some(key) => match ctx + .searcher_context + .predicate_cache + .get(split_id.clone(), key.clone()) + { + Some((_segment_id, hits)) => hits.is_empty(), + None => false, + }, + None => false, + }; + let warmup_outcome = if cached_known_empty { + WarmupOutcome::ProvablyEmpty + } else { + warmup(&searcher, &warmup_info).await? + }; let warmup_end = Instant::now(); let warmup_duration: Duration = warmup_end.duration_since(warmup_start); let warmup_size = ByteSize(byte_range_cache.get_num_bytes()); @@ -732,6 +772,21 @@ async fn leaf_search_single_split( search_permit.free_warmup_slot(); if warmup_outcome == WarmupOutcome::ProvablyEmpty { + // Record a fake empty entry in the predicate cache so later requests with + // the same predicate over the same split — even with a different collector + // or aggregation — short-circuit before warmup. We only record when *this* + // request discovered the emptiness through warmup + if !cached_known_empty + && let Some(key) = &negative_key + && let Some(segment_reader) = searcher.segment_readers().first() + { + ctx.searcher_context.predicate_cache.put( + split_id.clone(), + key.clone(), + segment_reader.segment_id(), + HitSet::empty(), + ); + } // A required term's posting list was empty, so the query matches no // document in this split. The remaining warmup downloads were aborted; // skip the search and report an empty (but counted) result. This is the diff --git a/quickwit/quickwit-search/src/tests.rs b/quickwit/quickwit-search/src/tests.rs index c8d851d06cb..9dc4bf0984f 100644 --- a/quickwit/quickwit-search/src/tests.rs +++ b/quickwit/quickwit-search/src/tests.rs @@ -25,7 +25,7 @@ use quickwit_proto::search::{ SortValue, TraceId, }; use quickwit_query::query_ast::{ - QueryAst, qast_helper, qast_json_helper, query_ast_from_user_text, + HitSet, PredicateCache, QueryAst, qast_helper, qast_json_helper, query_ast_from_user_text, }; use serde_json::{Value as JsonValue, json}; use tantivy::Term; @@ -1874,6 +1874,154 @@ async fn test_search_in_text_field_with_custom_tokenizer() -> anyhow::Result<()> Ok(()) } +/// Indexes a single split holding one `body: "hello world"` document and returns +/// the pieces needed to drive `single_doc_mapping_leaf_search` directly, so the +/// test owns the `SearcherContext` (and therefore its predicate cache). +async fn negative_cache_test_setup() -> ( + TestSandbox, + std::sync::Arc, + std::sync::Arc, + Vec, + std::sync::Arc, +) { + let index_id = "negative-cache-index"; + let doc_mapping_yaml = r#" + field_mappings: + - name: body + type: text + "#; + // No timestamp field: `rewrite_request` leaves the query AST untouched, so the + // negative-cache key is simply the JSON of the parsed query AST. + let test_sandbox = TestSandbox::create(index_id, doc_mapping_yaml, "{}", &["body"]) + .await + .unwrap(); + test_sandbox + .add_documents(vec![json!({"body": "hello world"})]) + .await + .unwrap(); + let mut metastore = test_sandbox.metastore(); + let splits_metadata = list_all_splits(vec![test_sandbox.index_uid()], &mut metastore) + .await + .unwrap(); + let splits: Vec = splits_metadata + .iter() + .map(extract_split_and_footer_offsets) + .collect(); + assert_eq!(splits.len(), 1, "test expects a single split"); + let searcher_context = std::sync::Arc::new(SearcherContext::for_test()); + let storage = test_sandbox.storage(); + let doc_mapper = test_sandbox.doc_mapper(); + (test_sandbox, searcher_context, storage, splits, doc_mapper) +} + +/// The negative-cache key the leaf search computes for a request: the JSON of the +/// parsed query AST (no `CacheNode` wrapper, no timestamp rewrite in this setup). +fn negative_cache_key_for(query_ast_json: &str) -> String { + let query_ast: QueryAst = serde_json::from_str(query_ast_json).unwrap(); + serde_json::to_string(&query_ast).unwrap() +} + +#[tokio::test] +async fn test_negative_cache_records_provably_empty_query() { + let (test_sandbox, searcher_context, storage, splits, doc_mapper) = + negative_cache_test_setup().await; + let split_id = splits[0].split_id.clone(); + + let query_ast_json = qast_json_helper("missingtoken", &["body"]); + let request = SearchRequest { + index_id_patterns: vec!["negative-cache-index".to_string()], + query_ast: query_ast_json.clone(), + max_hits: 10, + ..Default::default() + }; + let key = negative_cache_key_for(&query_ast_json); + // Nothing cached yet. + assert!( + searcher_context + .predicate_cache + .get(split_id.clone(), key.clone()) + .is_none() + ); + + let response = single_doc_mapping_leaf_search( + searcher_context.clone(), + std::sync::Arc::new(request), + storage, + splits, + doc_mapper, + ) + .await + .unwrap(); + assert_eq!(response.num_hits, 0); + + // The provably-empty result was recorded as a fake empty entry, so a later + // request with the same predicate can short-circuit before warmup. + let entry = searcher_context.predicate_cache.get(split_id, key); + let (_segment_id, hits) = entry.expect("absent-term query should be recorded"); + assert!(hits.is_empty()); + + test_sandbox.assert_quit().await; +} + +#[tokio::test] +async fn test_negative_cache_short_circuits_known_empty_query() { + let (test_sandbox, searcher_context, storage, splits, doc_mapper) = + negative_cache_test_setup().await; + let split_id = splits[0].split_id.clone(); + + // `hello` genuinely matches the indexed document. + let query_ast_json = qast_json_helper("hello", &["body"]); + // Control: without a cached absence, the query returns its hit. + let control_request = SearchRequest { + index_id_patterns: vec!["negative-cache-index".to_string()], + query_ast: query_ast_json.clone(), + max_hits: 10, + ..Default::default() + }; + let response = single_doc_mapping_leaf_search( + searcher_context.clone(), + std::sync::Arc::new(control_request), + storage.clone(), + splits.clone(), + doc_mapper.clone(), + ) + .await + .unwrap(); + assert_eq!(response.num_hits, 1); + + // Seed a fake empty entry for this exact predicate (the segment id is + // irrelevant: an empty result is segment-agnostic). + let key = negative_cache_key_for(&query_ast_json); + let segment_id = + tantivy::index::SegmentId::from_uuid_string("1686a000d4f7a91939d0e71df1646d7a").unwrap(); + searcher_context + .predicate_cache + .put(split_id, key, segment_id, HitSet::empty()); + + // A different `max_hits` misses the request-keyed partial-result cache but + // shares the query-AST-keyed negative cache, so this exercises the + // cross-collector short-circuit. Despite `hello` being present, the seeded + // absence makes the search report no hit. + let cross_collector_request = SearchRequest { + index_id_patterns: vec!["negative-cache-index".to_string()], + query_ast: query_ast_json, + max_hits: 5, + ..Default::default() + }; + let response = single_doc_mapping_leaf_search( + searcher_context.clone(), + std::sync::Arc::new(cross_collector_request), + storage, + splits, + doc_mapper, + ) + .await + .unwrap(); + assert_eq!(response.num_hits, 0); + + test_sandbox.assert_quit().await; +} + #[test] fn test_global_doc_address_ser_deser() { let doc_address = GlobalDocAddress { From 55c442a8e697fc2f0096a1bc0172d1b6a673f7f1 Mon Sep 17 00:00:00 2001 From: Pascal Seitz Date: Fri, 26 Jun 2026 16:42:46 +0100 Subject: [PATCH 2/2] Cache split emptiness per term instead of per whole query The negative cache keyed each provably-empty split on the whole query AST, so any added/removed filter or different time window produced a new key. In production this gave near-zero reuse: every filter permutation re-opened and re-probed all splits. A required term's absence in a split is immutable and independent of the rest of the query and of the time window. Key the cache on (split, term) instead: warmup reports each required term it proves absent via an `on_absent` callback, and a query short-circuits before warmup when any of its required terms is already known absent. Adding required terms can only make a query emptier, so cached absences keep pruning as filters change. --- quickwit/quickwit-search/src/leaf.rs | 208 +++++++++++++++---------- quickwit/quickwit-search/src/tests.rs | 215 ++++++++++++++++++++++---- 2 files changed, 316 insertions(+), 107 deletions(-) diff --git a/quickwit/quickwit-search/src/leaf.rs b/quickwit/quickwit-search/src/leaf.rs index b905972e596..b0b0e666e29 100644 --- a/quickwit/quickwit-search/src/leaf.rs +++ b/quickwit/quickwit-search/src/leaf.rs @@ -51,6 +51,7 @@ use tantivy::aggregation::agg_req::{AggregationVariants, Aggregations}; use tantivy::collector::Collector; use tantivy::directory::FileSlice; use tantivy::fastfield::FastFieldReaders; +use tantivy::index::SegmentId; use tantivy::schema::Field; use tantivy::{DateTime, Index, ReloadPolicy, Searcher, TantivyError, Term}; use tokio::task::{JoinError, JoinSet}; @@ -257,16 +258,6 @@ pub(crate) async fn open_index_with_caches( Ok((index, hot_directory)) } -/// Outcome of [`warmup`]. -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -pub(crate) enum WarmupOutcome { - /// The warmup completed; the split must be searched. - Completed, - /// A required term has an empty posting list, so the query provably matches - /// nothing in this split. The remaining warmup downloads were cancelled. - ProvablyEmpty, -} - /// Runs `fut`, racing it against `cancel`. If cancellation fires first, the /// (possibly in-flight) future is dropped — aborting its downloads — and /// `Ok(())` is returned. With no token, `fut` simply runs to completion. @@ -299,11 +290,21 @@ async fn run_cancellable( /// * `term_dict_field_names` - A list of fields, where the whole dictionary needs to be loaded. /// This is e.g. required for term aggregation, since we don't know in advance which terms are /// going to be hit. +/// +/// `on_absent` is invoked once for every required term found to have an empty posting list, +/// with the segment it was missing from. Such a term proves the query empty in this split, +/// so the remaining warmup downloads are then cancelled; the callback lets the caller record +/// the (immutable, query-independent) absence — see [`term_absence_cache_key`]. It only ever +/// fires for a single-segment split, where "absent in the split" is sound. +/// +/// Returns whether the query is provably empty in this split (i.e. `on_absent` fired and +/// warmup was short-circuited). #[instrument(skip_all)] pub(crate) async fn warmup( searcher: &Searcher, warmup_info: &WarmupInfo, -) -> anyhow::Result { + on_absent: &(dyn Fn(&Term, SegmentId) + Sync), +) -> anyhow::Result { debug!(warmup_info=?warmup_info); // Early-abort optimization: the split's downloads can be cancelled as soon as @@ -324,6 +325,7 @@ pub(crate) async fn warmup( &warmup_info.terms_grouped_by_field, &warmup_info.required_terms, abort_token.as_ref(), + on_absent, ) .instrument(debug_span!("warm_up_terms")); let warm_up_term_ranges_future = run_cancellable( @@ -372,11 +374,7 @@ pub(crate) async fn warmup( Some(abort_token) => abort_token.is_cancelled(), None => false, }; - if provably_empty { - Ok(WarmupOutcome::ProvablyEmpty) - } else { - Ok(WarmupOutcome::Completed) - } + Ok(provably_empty) } async fn warm_up_term_dict_fields( @@ -454,11 +452,13 @@ async fn warm_up_terms( terms_grouped_by_field: &HashMap>, required_terms: &HashSet, abort_token: Option<&CancellationToken>, + on_absent: &(dyn Fn(&Term, SegmentId) + Sync), ) -> anyhow::Result<()> { let mut warm_up_futures = Vec::new(); for (field, terms) in terms_grouped_by_field { for segment_reader in searcher.segment_readers() { let inv_idx = segment_reader.inverted_index(*field)?; + let segment_id = segment_reader.segment_id(); for (term, position_needed) in terms.iter() { let inv_idx_clone = inv_idx.clone(); // Only a required term can prove the query empty. When such a @@ -471,6 +471,9 @@ async fn warm_up_terms( warm_up_futures.push(async move { let found = inv_idx_clone.warm_postings(term, *position_needed).await?; if !found && let Some(abort_token) = cancel_on_empty { + // Report the absence and fire the abort token. Both are synchronous, so + // they run before any cancellation can drop us. + on_absent(term, segment_id); abort_token.cancel(); } anyhow::Ok(()) @@ -622,21 +625,31 @@ fn compute_index_size(hot_directory: &HotDirectory) -> ByteSize { ByteSize(size_bytes) } -/// Cache key for the negative (never-match) cache. +/// Cache key under which "this term has no posting list in this split" is recorded in +/// the shared predicate cache. /// -/// We reuse the predicate cache's store, so the key must match the one its -/// [`quickwit_query::query_ast::PredicateCacheInjector`] uses: the JSON of the -/// query AST, unwrapped from any top-level [`CacheNode`] (added for -/// `search_after`). The effective, split-intersected timestamp range is already -/// folded into the query AST by `rewrite_request`, so this key distinguishes time -/// windows; an entry is only ever reused for an identical predicate over the same -/// window on the same split. -fn negative_cache_key(query_ast: &QueryAst) -> Option { - let inner = match query_ast { - QueryAst::Cache(cache_node) => cache_node.inner.as_ref(), - other => other, - }; - serde_json::to_string(inner).ok() +/// Absence is a property of the term and the split alone: it is independent of the rest +/// of the query and of any time window, and — because splits are immutable — it never +/// changes once observed. So a single entry per `(split, term)` lets *every* query that +/// carries this term short-circuit before warmup, no matter what other filters or time +/// range ride along, and adding more required terms can only make a query emptier. +/// +/// The key is the field id followed by the hex of the term's serialized value bytes +/// (which for a JSON field already encode the path and type) — together a unique +/// identifier of the term. It never collides with the whole-query keys the [`CacheNode`] +/// positive cache stores in the same instance: those are serialized query ASTs that +/// start with `{`, never a hex field id. +pub(crate) fn term_absence_cache_key(term: &Term) -> String { + use std::fmt::Write; + + let value_bytes = term.serialized_value_bytes(); + let mut key = String::with_capacity(9 + value_bytes.len() * 2); + let _ = write!(key, "{:08x}:", term.field().field_id()); + for byte in value_bytes { + // Hex-encode so the binary value bytes form a valid (printable) String key. + let _ = write!(key, "{byte:02x}"); + } + key } /// Apply a leaf search on a single split. @@ -730,28 +743,39 @@ async fn leaf_search_single_split( let warmup_start = Instant::now(); leaf_search_state_guard.set_state(SplitSearchState::WarmUp); - // Negative cache: if this exact predicate was previously proven to match no - // document in this split, skip warmup (and the search) entirely. Reusing the - // predicate cache's store means an empty entry recorded by either this path - // or the predicate cache's filler short-circuits the request. An empty result - // is segment- and scoring-agnostic, so this is sound even for scored queries - // (which the `CacheNode` machinery itself does not support). - let negative_key = negative_cache_key(&query_ast); - let cached_known_empty = match &negative_key { - Some(key) => match ctx + // Negative cache: a split is provably empty for this query if any required term has + // previously been proven absent here. Absence is an immutable, query- and + // time-window-independent property of the split (see `term_absence_cache_key`), so the + // split short-circuits before warmup no matter which earlier query first proved the + // term absent, nor what other filters or time range this query carries — extra + // required terms can only make it emptier. An empty result is segment- and + // scoring-agnostic, so this holds even for scored queries (which the `CacheNode` + // machinery itself does not support). + let cached_known_empty = warmup_info.required_terms.iter().any(|term| { + match ctx .searcher_context .predicate_cache - .get(split_id.clone(), key.clone()) + .get(split_id.clone(), term_absence_cache_key(term)) { Some((_segment_id, hits)) => hits.is_empty(), None => false, - }, - None => false, - }; - let warmup_outcome = if cached_known_empty { - WarmupOutcome::ProvablyEmpty + } + }); + let provably_empty = if cached_known_empty { + true } else { - warmup(&searcher, &warmup_info).await? + // Record every required term proven absent during warmup, so any future query + // carrying one of them prunes before warmup. Absence is per `(split, term)`, + // immutable, and independent of the rest of the query. + let record_absence = |term: &Term, segment_id: SegmentId| { + ctx.searcher_context.predicate_cache.put( + split_id.clone(), + term_absence_cache_key(term), + segment_id, + HitSet::empty(), + ); + }; + warmup(&searcher, &warmup_info, &record_absence).await? }; let warmup_end = Instant::now(); let warmup_duration: Duration = warmup_end.duration_since(warmup_start); @@ -771,22 +795,11 @@ async fn leaf_search_single_split( search_permit.update_memory_usage(warmup_size); search_permit.free_warmup_slot(); - if warmup_outcome == WarmupOutcome::ProvablyEmpty { - // Record a fake empty entry in the predicate cache so later requests with - // the same predicate over the same split — even with a different collector - // or aggregation — short-circuit before warmup. We only record when *this* - // request discovered the emptiness through warmup - if !cached_known_empty - && let Some(key) = &negative_key - && let Some(segment_reader) = searcher.segment_readers().first() - { - ctx.searcher_context.predicate_cache.put( - split_id.clone(), - key.clone(), - segment_reader.segment_id(), - HitSet::empty(), - ); - } + if provably_empty { + // The absences discovered this run were already recorded per-term above (or the + // short-circuit came from a prior run's per-term entry), so there is nothing to + // write here. + // // A required term's posting list was empty, so the query matches no // document in this split. The remaining warmup downloads were aborted; // skip the search and report an empty (but counted) result. This is the @@ -2803,6 +2816,8 @@ mod tests { (searcher, field) } + /// Builds a `WarmupInfo` warming `terms`, with `required` as the set of required + /// terms (each must be present for the query to match). fn warmup_info_with_required(terms: &[&Term], required: &[&Term]) -> WarmupInfo { let mut terms_grouped_by_field: HashMap> = HashMap::new(); for term in terms { @@ -2819,33 +2834,70 @@ mod tests { } #[tokio::test] - async fn test_warmup_aborts_when_required_term_is_missing() { + async fn test_warmup_reports_absent_required_terms() { let (searcher, body) = ram_searcher_with_text("body", &["hello world"]); - // Single segment: the early-abort optimization is armed. + // Single segment: the early-abort optimization is armed, so absence is recorded. assert_eq!(searcher.segment_readers().len(), 1); let present = Term::from_field_text(body, "hello"); let missing = Term::from_field_text(body, "missing"); - // A required term with an empty posting list proves the query empty. - let warmup_info = warmup_info_with_required(&[&present, &missing], &[&missing]); - assert_eq!( - warmup(&searcher, &warmup_info).await.unwrap(), - WarmupOutcome::ProvablyEmpty - ); + // Runs warmup, returning whether the split is provably empty and the terms that + // `on_absent` was invoked with. + async fn run(searcher: &Searcher, warmup_info: &WarmupInfo) -> (bool, Vec) { + let reported = std::sync::Mutex::new(Vec::new()); + let provably_empty = warmup(searcher, warmup_info, &|term: &Term, _segment_id| { + reported.lock().unwrap().push(term.clone()); + }) + .await + .unwrap(); + (provably_empty, reported.into_inner().unwrap()) + } + + // An absent required term is reported (so the caller can cache it) and proves the + // split empty; the present required term is not reported. + let warmup_info = warmup_info_with_required(&[&present, &missing], &[&present, &missing]); + let (provably_empty, reported) = run(&searcher, &warmup_info).await; + assert!(provably_empty); + assert_eq!(reported, vec![missing.clone()]); - // The required term is present: warmup completes normally. + // All required terms present: nothing reported, so the split must be searched. let warmup_info = warmup_info_with_required(&[&present], &[&present]); - assert_eq!( - warmup(&searcher, &warmup_info).await.unwrap(), - WarmupOutcome::Completed - ); + let (provably_empty, reported) = run(&searcher, &warmup_info).await; + assert!(!provably_empty); + assert!(reported.is_empty()); - // A missing term that is not required must not abort. + // A missing term that is not required is never reported (recording would be unsound + // without a required-term proof), so the split must be searched. let warmup_info = warmup_info_with_required(&[&present, &missing], &[]); + let (provably_empty, reported) = run(&searcher, &warmup_info).await; + assert!(!provably_empty); + assert!(reported.is_empty()); + } + + #[test] + fn test_term_absence_cache_key() { + let mut schema_builder = Schema::builder(); + let body = schema_builder.add_text_field("body", tantivy::schema::TEXT); + let title = schema_builder.add_text_field("title", tantivy::schema::TEXT); + let _schema = schema_builder.build(); + + let key = term_absence_cache_key(&Term::from_field_text(body, "hello")); + // Stable for the same term, and disjoint from the JSON query keys the `CacheNode` + // positive cache stores (which start with '{', never a hex field id). + assert!(!key.starts_with('{')); assert_eq!( - warmup(&searcher, &warmup_info).await.unwrap(), - WarmupOutcome::Completed + key, + term_absence_cache_key(&Term::from_field_text(body, "hello")) + ); + // A different value or a different field yields a different key. + assert_ne!( + key, + term_absence_cache_key(&Term::from_field_text(body, "world")) + ); + assert_ne!( + key, + term_absence_cache_key(&Term::from_field_text(title, "hello")) ); } diff --git a/quickwit/quickwit-search/src/tests.rs b/quickwit/quickwit-search/src/tests.rs index 9dc4bf0984f..cba72ec3d34 100644 --- a/quickwit/quickwit-search/src/tests.rs +++ b/quickwit/quickwit-search/src/tests.rs @@ -1890,8 +1890,8 @@ async fn negative_cache_test_setup() -> ( - name: body type: text "#; - // No timestamp field: `rewrite_request` leaves the query AST untouched, so the - // negative-cache key is simply the JSON of the parsed query AST. + // A plain text field; queries on it build single-token terms whose absence the + // per-term negative cache keys on. let test_sandbox = TestSandbox::create(index_id, doc_mapping_yaml, "{}", &["body"]) .await .unwrap(); @@ -1914,27 +1914,27 @@ async fn negative_cache_test_setup() -> ( (test_sandbox, searcher_context, storage, splits, doc_mapper) } -/// The negative-cache key the leaf search computes for a request: the JSON of the -/// parsed query AST (no `CacheNode` wrapper, no timestamp rewrite in this setup). -fn negative_cache_key_for(query_ast_json: &str) -> String { - let query_ast: QueryAst = serde_json::from_str(query_ast_json).unwrap(); - serde_json::to_string(&query_ast).unwrap() +/// The per-term absence cache key the leaf search uses for `field:value` on a text field +/// with the default tokenizer (a single-token value builds exactly this term). +fn term_absence_key_for(doc_mapper: &DocMapper, field_name: &str, value: &str) -> String { + let field = doc_mapper.schema().get_field(field_name).unwrap(); + let term = tantivy::Term::from_field_text(field, value); + leaf::term_absence_cache_key(&term) } #[tokio::test] -async fn test_negative_cache_records_provably_empty_query() { +async fn test_negative_cache_records_term_absence() { let (test_sandbox, searcher_context, storage, splits, doc_mapper) = negative_cache_test_setup().await; let split_id = splits[0].split_id.clone(); - let query_ast_json = qast_json_helper("missingtoken", &["body"]); let request = SearchRequest { index_id_patterns: vec!["negative-cache-index".to_string()], - query_ast: query_ast_json.clone(), + query_ast: qast_json_helper("missingtoken", &["body"]), max_hits: 10, ..Default::default() }; - let key = negative_cache_key_for(&query_ast_json); + let key = term_absence_key_for(&doc_mapper, "body", "missingtoken"); // Nothing cached yet. assert!( searcher_context @@ -1954,27 +1954,26 @@ async fn test_negative_cache_records_provably_empty_query() { .unwrap(); assert_eq!(response.num_hits, 0); - // The provably-empty result was recorded as a fake empty entry, so a later - // request with the same predicate can short-circuit before warmup. + // The absent term was recorded per-term, so any later query carrying it — with any + // other filters or time window — can short-circuit before warmup. let entry = searcher_context.predicate_cache.get(split_id, key); - let (_segment_id, hits) = entry.expect("absent-term query should be recorded"); + let (_segment_id, hits) = entry.expect("absent term should be recorded"); assert!(hits.is_empty()); test_sandbox.assert_quit().await; } #[tokio::test] -async fn test_negative_cache_short_circuits_known_empty_query() { +async fn test_negative_cache_short_circuits_query_with_extra_terms() { let (test_sandbox, searcher_context, storage, splits, doc_mapper) = negative_cache_test_setup().await; let split_id = splits[0].split_id.clone(); - // `hello` genuinely matches the indexed document. - let query_ast_json = qast_json_helper("hello", &["body"]); - // Control: without a cached absence, the query returns its hit. + // `hello` and `world` both genuinely match the indexed document. Control: with no + // cached absence, their conjunction returns the hit. let control_request = SearchRequest { index_id_patterns: vec!["negative-cache-index".to_string()], - query_ast: query_ast_json.clone(), + query_ast: qast_json_helper("hello world", &["body"]), max_hits: 10, ..Default::default() }; @@ -1989,28 +1988,186 @@ async fn test_negative_cache_short_circuits_known_empty_query() { .unwrap(); assert_eq!(response.num_hits, 1); - // Seed a fake empty entry for this exact predicate (the segment id is - // irrelevant: an empty result is segment-agnostic). - let key = negative_cache_key_for(&query_ast_json); + // Seed a per-term absence for `hello` alone, as if an *earlier, different* query had + // proven it absent (the segment id is irrelevant for an empty result). + let key = term_absence_key_for(&doc_mapper, "body", "hello"); let segment_id = tantivy::index::SegmentId::from_uuid_string("1686a000d4f7a91939d0e71df1646d7a").unwrap(); searcher_context .predicate_cache .put(split_id, key, segment_id, HitSet::empty()); - // A different `max_hits` misses the request-keyed partial-result cache but - // shares the query-AST-keyed negative cache, so this exercises the - // cross-collector short-circuit. Despite `hello` being present, the seeded - // absence makes the search report no hit. - let cross_collector_request = SearchRequest { + // A *larger* query that merely contains `hello` as a required term: despite `hello` + // (and `world`) being present, the cached absence of `hello` proves the whole + // conjunction empty, so it short-circuits to no hits. This is the cross-query reuse + // the per-term cache provides — adding required terms can only keep it empty. The + // different `max_hits` dodges the request-keyed partial-result cache so this really + // exercises the predicate cache. + let extra_terms_request = SearchRequest { index_id_patterns: vec!["negative-cache-index".to_string()], - query_ast: query_ast_json, + query_ast: qast_json_helper("hello world", &["body"]), max_hits: 5, ..Default::default() }; let response = single_doc_mapping_leaf_search( searcher_context.clone(), - std::sync::Arc::new(cross_collector_request), + std::sync::Arc::new(extra_terms_request), + storage, + splits, + doc_mapper, + ) + .await + .unwrap(); + assert_eq!(response.num_hits, 0); + + test_sandbox.assert_quit().await; +} + +/// End-to-end version of the above with no manual seeding: a first real query proves a term +/// absent (the warmup path records it), then a second query that *adds* a present term still +/// prunes — exactly "first `body:nonexistent`, then `body:nonexistent body:hello`". +#[tokio::test] +async fn test_negative_cache_records_then_prunes_query_with_added_term() { + let (test_sandbox, searcher_context, storage, splits, doc_mapper) = + negative_cache_test_setup().await; + let split_id = splits[0].split_id.clone(); + + // Query 1: a term absent from the only document. The real warmup proves it absent and + // records it per-term — no seeding. + let absent_request = SearchRequest { + index_id_patterns: vec!["negative-cache-index".to_string()], + query_ast: qast_json_helper("nonexistent", &["body"]), + max_hits: 10, + ..Default::default() + }; + let response = single_doc_mapping_leaf_search( + searcher_context.clone(), + std::sync::Arc::new(absent_request), + storage.clone(), + splits.clone(), + doc_mapper.clone(), + ) + .await + .unwrap(); + assert_eq!(response.num_hits, 0); + + // The first query recorded the absence through the real leaf-search path. + let key = term_absence_key_for(&doc_mapper, "body", "nonexistent"); + let (_segment_id, hits) = searcher_context + .predicate_cache + .get(split_id, key) + .expect("the first query should have recorded the absent term"); + assert!(hits.is_empty()); + + // Query 2 adds a present term: `nonexistent hello` parses (default AND) to + // `nonexistent AND hello`. `hello` matches the document, but the cached absence of + // `nonexistent` proves the conjunction empty, so it prunes before warmup. A different + // `max_hits` dodges the request-keyed partial-result cache so this exercises the + // predicate cache. + let added_term_request = SearchRequest { + index_id_patterns: vec!["negative-cache-index".to_string()], + query_ast: qast_json_helper("nonexistent hello", &["body"]), + max_hits: 5, + ..Default::default() + }; + let response = single_doc_mapping_leaf_search( + searcher_context.clone(), + std::sync::Arc::new(added_term_request), + storage, + splits, + doc_mapper, + ) + .await + .unwrap(); + assert_eq!(response.num_hits, 0); + + test_sandbox.assert_quit().await; +} + +/// Like [`negative_cache_test_setup`], but the index has a `ts` timestamp field, so a +/// request's time range is folded into the query AST by `rewrite_request` — letting us +/// check that a per-term absence still short-circuits regardless of the window. Returns +/// the first indexed timestamp as well. +async fn negative_cache_ts_test_setup() -> ( + TestSandbox, + std::sync::Arc, + std::sync::Arc, + Vec, + std::sync::Arc, + i64, +) { + let index_id = "negative-cache-ts-index"; + let doc_mapping_yaml = r#" + field_mappings: + - name: body + type: text + - name: ts + type: datetime + fast: true + timestamp_field: ts + "#; + let test_sandbox = TestSandbox::create(index_id, doc_mapping_yaml, "{}", &["body"]) + .await + .unwrap(); + let start_timestamp = 1_700_000_000i64; + let mut docs = Vec::with_capacity(10); + for i in 0..10 { + docs.push(json!({"body": format!("info {i}"), "ts": start_timestamp + i})); + } + test_sandbox.add_documents(docs).await.unwrap(); + let mut metastore = test_sandbox.metastore(); + let splits_metadata = list_all_splits(vec![test_sandbox.index_uid()], &mut metastore) + .await + .unwrap(); + let splits: Vec = splits_metadata + .iter() + .map(extract_split_and_footer_offsets) + .collect(); + assert_eq!(splits.len(), 1, "test expects a single split"); + let searcher_context = std::sync::Arc::new(SearcherContext::for_test()); + let storage = test_sandbox.storage(); + let doc_mapper = test_sandbox.doc_mapper(); + ( + test_sandbox, + searcher_context, + storage, + splits, + doc_mapper, + start_timestamp, + ) +} + +#[tokio::test] +async fn test_negative_cache_short_circuits_across_time_windows() { + // A per-term absence does not depend on the time window: the required term + // `body:info` is unaffected by the timestamp range folded into the query AST, so an + // absence proven for any window short-circuits the same term over every other window. + let (test_sandbox, searcher_context, storage, splits, doc_mapper, start_timestamp) = + negative_cache_ts_test_setup().await; + let split_id = splits[0].split_id.clone(); + + // `info` genuinely matches every indexed document. Seed its absence as if proven for + // some other window (the segment id is irrelevant for an empty result). + let key = term_absence_key_for(&doc_mapper, "body", "info"); + let segment_id = + tantivy::index::SegmentId::from_uuid_string("1686a000d4f7a91939d0e71df1646d7a").unwrap(); + searcher_context + .predicate_cache + .put(split_id, key, segment_id, HitSet::empty()); + + // Query a specific, different window: despite `info` matching, the seeded absence + // short-circuits the search. + let query_window = SearchRequest { + index_id_patterns: vec!["negative-cache-ts-index".to_string()], + query_ast: qast_json_helper("info", &["body"]), + start_timestamp: Some(start_timestamp + 4), + end_timestamp: Some(start_timestamp + 9), + max_hits: 10, + ..Default::default() + }; + let response = single_doc_mapping_leaf_search( + searcher_context.clone(), + std::sync::Arc::new(query_window), storage, splits, doc_mapper,