split search failures in outcome metrics - #6662
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4cd8f3e555
ℹ️ 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".
| ) | ||
| .await?; | ||
| .await | ||
| .inspect_err(|_| leaf_search_state_guard.set_state(SplitSearchState::Error))?; |
There was a problem hiding this comment.
Count reader initialization failures as errors
When split reader construction fails, the immediately following reader_builder().try_into()? still returns while the guard state is Start, so corrupt/unreadable split metadata at reader reload time is reported as cancel_before_warmup instead of the new error category. Please wrap this setup ? with the same error state transition (or mark the whole infrastructure setup block) so these operational failures do not continue to look like cancellations.
Useful? React with 👍 / 👎.
Classify storage, warmup, Tantivy execution failures, and panics as category="error" rather than cancellation phases.
4cd8f3e to
c4e6d93
Compare
| CpuQueue, | ||
| Cpu, | ||
| // Reserved for infrastructure and search-execution failures, not invalid user requests. | ||
| Error, |
There was a problem hiding this comment.
Should we add an additional error tag to better categorise the error on the emitted metric?
There was a problem hiding this comment.
I added a tag for some categories
Record storage, warmup, Tantivy execution failures, and panics as
category="error"inquickwit_search_split_search_outcomemetric