Categorize split search errors in outcome metrics (#6663)

Add bounded error labels for reader creation, warmup, Tantivy search,
and panic failures. Expose these categories in documentation and Grafana.

Co-authored-by: Pascal Seitz <pascal.seitz@gmail.com>

docs/reference/metrics.md

@@ -79,7 +79,7 @@ PostgreSQL-backed metastores also expose connection pool gauges:
 
 | Namespace | Metric Name | Description | Type |
 | --------- | ----------- | ----------- | ---- |
-| `quickwit_search` | `split_search_outcome` | Number of local leaf split search outcomes by `category` (`success`, operational `error`, cache/pruning, or cancellation phase). Retries are counted separately | `counter` |
+| `quickwit_search` | `split_search_outcome` | Number of local leaf split search outcomes by `category` (`success`, operational `error`, cache/pruning, or cancellation phase). Operational errors are classified by `error` (`create_reader`, `warmup`, `tantivy_search`, or `panic`). Retries are counted separately | `counter` |
 | `quickwit_search` | `leaf_search_split_duration_secs` | Number of seconds required to run a leaf search over a single split. The timer starts after the semaphore is obtained | `histogram` |
 | `quickwit_search` | `active_search_threads_count` | Number of threads in use in the CPU thread pool | `gauge` |
 

monitoring/grafana/dashboards/searchers.json

@@ -112,7 +112,7 @@
           "fullMetaSearch": false,
           "includeNullMetadata": true,
           "instant": false,
-          "legendFormat": "{{instance}} / {{category}}",
+          "legendFormat": "{{instance}} / {{category}} / {{error}}",
           "range": true,
           "refId": "A",
           "useBackend": false

quickwit/quickwit-search/src/leaf.rs

@@ -701,7 +701,10 @@ async fn leaf_search_single_split(
         Some(byte_range_cache.clone()),
     )
     .await
-    .inspect_err(|_| leaf_search_state_guard.set_state(SplitSearchState::Error))?;
+    .inspect_err(|_| {
+        leaf_search_state_guard
+            .set_state(SplitSearchState::Error(SplitSearchErrorKind::CreateReader))
+    })?;
 
     let index_size = compute_index_size(&hot_directory);
     if index_size < search_permit.memory_allocation() {
@@ -711,7 +714,11 @@ async fn leaf_search_single_split(
     let searcher = index
         .reader_builder()
         .reload_policy(ReloadPolicy::Manual)
-        .try_into()?
+        .try_into()
+        .inspect_err(|_| {
+            leaf_search_state_guard
+                .set_state(SplitSearchState::Error(SplitSearchErrorKind::CreateReader))
+        })?
         .searcher();
 
     let agg_context_params = AggContextParams {
@@ -799,7 +806,10 @@ async fn leaf_search_single_split(
         let provably_empty = warmup(&searcher, &warmup_info, &record_absence)
             .instrument(warmup_span.clone())
             .await
-            .inspect_err(|_| leaf_search_state_guard.set_state(SplitSearchState::Error))?;
+            .inspect_err(|_| {
+                leaf_search_state_guard
+                    .set_state(SplitSearchState::Error(SplitSearchErrorKind::Warmup))
+            })?;
         warmup_span.record(
             "downloaded_mb",
             download_counters
@@ -912,12 +922,16 @@ async fn leaf_search_single_split(
                         get_leaf_resp_from_count(searcher.num_docs())
                     } else if collector.is_count_only() {
                         let count = query.count(&searcher).inspect_err(|_| {
-                            leaf_search_state_guard.set_state(SplitSearchState::Error)
+                            leaf_search_state_guard.set_state(SplitSearchState::Error(
+                                SplitSearchErrorKind::TantivySearch,
+                            ))
                         })? as u64;
                         get_leaf_resp_from_count(count)
                     } else {
                         searcher.search(&query, &collector).inspect_err(|_| {
-                            leaf_search_state_guard.set_state(SplitSearchState::Error)
+                            leaf_search_state_guard.set_state(SplitSearchState::Error(
+                                SplitSearchErrorKind::TantivySearch,
+                            ))
                         })?
                     };
                 let (download_num_bytes, download_num_requests) =
@@ -2166,6 +2180,14 @@ fn process_partial_result_cache(
     Ok(uncached_splits)
 }
 
+#[derive(Copy, Clone)]
+enum SplitSearchErrorKind {
+    CreateReader,
+    Warmup,
+    TantivySearch,
+    Panic,
+}
+
 #[derive(Copy, Clone)]
 enum SplitSearchState {
     Start,
@@ -2177,7 +2199,7 @@ enum SplitSearchState {
     CpuQueue,
     Cpu,
     // Reserved for infrastructure and search-execution failures, not invalid user requests.
-    Error,
+    Error(SplitSearchErrorKind),
     Success,
 }
 
@@ -2192,7 +2214,14 @@ impl SplitSearchState {
             SplitSearchState::PrunedAfterWarmup => counters.pruned_after_warmup.inc(),
             SplitSearchState::CpuQueue => counters.cancel_cpu_queue.inc(),
             SplitSearchState::Cpu => counters.cancel_cpu.inc(),
-            SplitSearchState::Error => counters.error.inc(),
+            SplitSearchState::Error(SplitSearchErrorKind::CreateReader) => {
+                counters.error_create_reader.inc()
+            }
+            SplitSearchState::Error(SplitSearchErrorKind::Warmup) => counters.error_warmup.inc(),
+            SplitSearchState::Error(SplitSearchErrorKind::TantivySearch) => {
+                counters.error_tantivy_search.inc()
+            }
+            SplitSearchState::Error(SplitSearchErrorKind::Panic) => counters.error_panic.inc(),
             SplitSearchState::Success => counters.success.inc(),
         }
     }
@@ -2201,7 +2230,7 @@ impl SplitSearchState {
 impl Drop for SplitSearchStateGuard {
     fn drop(&mut self) {
         let state = if std::thread::panicking() {
-            SplitSearchState::Error
+            SplitSearchState::Error(SplitSearchErrorKind::Panic)
         } else {
             self.state
         };
@@ -2310,14 +2339,23 @@ mod tests {
     use crate::LambdaLeafSearchInvoker;
 
     #[test]
-    fn test_split_search_state_guard_records_error() {
+    fn test_split_search_state_guard_records_errors_by_kind() {
         let counters = Arc::new(SplitSearchOutcomeCounters::default());
-        let mut leaf_guard = SplitSearchStateGuard::new(counters.clone());
-        leaf_guard.set_state(SplitSearchState::Error);
+        for error_kind in [
+            SplitSearchErrorKind::CreateReader,
+            SplitSearchErrorKind::Warmup,
+            SplitSearchErrorKind::TantivySearch,
+            SplitSearchErrorKind::Panic,
+        ] {
+            let mut leaf_guard = SplitSearchStateGuard::new(counters.clone());
+            leaf_guard.set_state(SplitSearchState::Error(error_kind));
+            drop(leaf_guard);
+        }
 
-        drop(leaf_guard);
-
-        assert_eq!(counters.error.get(), 1);
+        assert_eq!(counters.error_create_reader.get(), 1);
+        assert_eq!(counters.error_warmup.get(), 1);
+        assert_eq!(counters.error_tantivy_search.get(), 1);
+        assert_eq!(counters.error_panic.get(), 1);
         assert_eq!(counters.cancel_warmup.get(), 0);
     }
 
@@ -2329,7 +2367,10 @@ mod tests {
 
         drop(leaf_guard);
 
-        assert_eq!(counters.error.get(), 0);
+        assert_eq!(counters.error_create_reader.get(), 0);
+        assert_eq!(counters.error_warmup.get(), 0);
+        assert_eq!(counters.error_tantivy_search.get(), 0);
+        assert_eq!(counters.error_panic.get(), 0);
         assert_eq!(counters.cancel_warmup.get(), 1);
     }
 

quickwit/quickwit-search/src/metrics.rs

@@ -47,7 +47,10 @@ pub struct SplitSearchOutcomeCounters {
     pub pruned_after_warmup: Counter,
     pub cancel_cpu_queue: Counter,
     pub cancel_cpu: Counter,
-    pub error: Counter,
+    pub error_create_reader: Counter,
+    pub error_warmup: Counter,
+    pub error_tantivy_search: Counter,
+    pub error_panic: Counter,
     pub success: Counter,
 }
 
@@ -62,7 +65,10 @@ impl Default for SplitSearchOutcomeCounters {
             pruned_after_warmup: Counter::local(),
             cancel_cpu_queue: Counter::local(),
             cancel_cpu: Counter::local(),
-            error: Counter::local(),
+            error_create_reader: Counter::local(),
+            error_warmup: Counter::local(),
+            error_tantivy_search: Counter::local(),
+            error_panic: Counter::local(),
             success: Counter::local(),
         }
     }
@@ -78,7 +84,10 @@ impl fmt::Display for SplitSearchOutcomeCounters {
         print_if_not_null("pruned_after_warmup", &self.pruned_after_warmup, f)?;
         print_if_not_null("cancel_cpu_queue", &self.cancel_cpu_queue, f)?;
         print_if_not_null("cancel_cpu", &self.cancel_cpu, f)?;
-        print_if_not_null("error", &self.error, f)?;
+        print_if_not_null("error_create_reader", &self.error_create_reader, f)?;
+        print_if_not_null("error_warmup", &self.error_warmup, f)?;
+        print_if_not_null("error_tantivy_search", &self.error_tantivy_search, f)?;
+        print_if_not_null("error_panic", &self.error_panic, f)?;
         print_if_not_null("success", &self.success, f)?;
         Ok(())
     }
@@ -93,6 +102,13 @@ impl SplitSearchOutcomeCounters {
                 "category" => category,
             )
         };
+        let error_counter = |error: &'static str| {
+            counter!(
+                parent: &SPLIT_SEARCH_OUTCOME,
+                "category" => "error",
+                "error" => error,
+            )
+        };
         SplitSearchOutcomeCounters {
             cancel_before_warmup: counter("cancel_before_warmup"),
             cache_hit: counter("cache_hit"),
@@ -102,7 +118,10 @@ impl SplitSearchOutcomeCounters {
             pruned_after_warmup: counter("pruned_after_warmup"),
             cancel_cpu_queue: counter("cancel_cpu_queue"),
             cancel_cpu: counter("cancel_cpu"),
-            error: counter("error"),
+            error_create_reader: error_counter("create_reader"),
+            error_warmup: error_counter("warmup"),
+            error_tantivy_search: error_counter("tantivy_search"),
+            error_panic: error_counter("panic"),
             success: counter("success"),
         }
     }
@@ -142,7 +161,7 @@ fn pseudo_exponential_bytes_buckets() -> Vec<f64> {
 
 static SPLIT_SEARCH_OUTCOME: LazyCounter = lazy_counter!(
         name: "split_search_outcome",
-        description: "Count the state in which each leaf search split ended",
+        description: "Count the state in which each leaf search split ended. Errors are classified by the error label",
         subsystem: "search",
 );
 
@@ -260,3 +279,28 @@ pub(crate) static SEARCHER_NODE_LOAD: LazyGauge = lazy_gauge!(
         description: "Current load on this searcher node, expressed as the sum of job costs for all queued and active split search tasks.",
         subsystem: "search",
 );
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn assert_error_label(counter: &Counter, expected_error: &str) {
+        let labels: Vec<(&str, &str)> = counter
+            .key()
+            .labels()
+            .map(|label| (label.key(), label.value()))
+            .collect();
+        assert!(labels.contains(&("category", "error")));
+        assert!(labels.contains(&("error", expected_error)));
+    }
+
+    #[test]
+    fn test_split_search_outcome_error_labels() {
+        let counters = SplitSearchOutcomeCounters::new_global();
+
+        assert_error_label(&counters.error_create_reader, "create_reader");
+        assert_error_label(&counters.error_warmup, "warmup");
+        assert_error_label(&counters.error_tantivy_search, "tantivy_search");
+        assert_error_label(&counters.error_panic, "panic");
+    }
+}