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 {