Remove secondary search queue#75
Merged
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Removes the “secondary” search queue path and its associated configuration/metrics, simplifying search scheduling and timeout behavior to a single queue (SRTF-driven), and removing the “read-only” split-cache touch behavior that previously avoided registering download candidates.
Changes:
- Eliminate secondary/broad-search queue selection (permits + timeouts) and related plumbing across leaf search execution.
- Simplify Prometheus metrics by removing the
queuelabel and secondary-task gauges/counters. - Remove secondary-queue configuration knobs from
SearcherConfig, and simplify split-cache APIs by droppingread_onlyparameters.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| quickwit/quickwit-storage/src/split_cache/split_table.rs | Simplifies touch by removing read_only gating when registering download candidates. |
| quickwit/quickwit-storage/src/split_cache/mod.rs | Updates split-cache wrapper + access path to match the new touch/API shape. |
| quickwit/quickwit-search/src/tests.rs | Updates tests for removed broad/secondary-search boolean parameter. |
| quickwit/quickwit-search/src/service.rs | Removes broad-search timeout/queue selection; always uses primary timeout and single leaf-search path. |
| quickwit/quickwit-search/src/metrics.rs | Removes queue label + secondary metrics; converts split-duration metric from HistogramVec to Histogram. |
| quickwit/quickwit-search/src/metrics_trackers.rs | Updates leaf-search request metric labeling after dropping queue label. |
| quickwit/quickwit-search/src/list_terms.rs | Updates index opening + metrics timing to match simplified split-cache + metrics APIs. |
| quickwit/quickwit-search/src/list_fields.rs | Updates call sites to match removed boolean parameter. |
| quickwit/quickwit-search/src/leaf.rs | Removes is_broad_search branching (permits, split-cache mode, metrics label). |
| quickwit/quickwit-search/src/fetch_docs.rs | Updates call site to match removed boolean parameter. |
| quickwit/quickwit-config/src/node_config/serialize.rs | Updates config parsing/serialization tests after removing secondary config fields. |
| quickwit/quickwit-config/src/node_config/mod.rs | Removes secondary-queue config fields from SearcherConfig. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
289
to
293
| #[serde(default = "SearcherConfig::default_request_timeout_secs")] | ||
| request_timeout_secs: NonZeroU64, | ||
| #[serde(default)] | ||
| #[serde(skip_serializing_if = "Option::is_none")] | ||
| pub storage_timeout_policy: Option<StorageTimeoutPolicy>, |
Collaborator
Author
There was a problem hiding this comment.
we stopped using secondary_* for a while now
rdettai-sk
force-pushed
the
remove-secondary-search-queue
branch
from
b132cb2 to
9aed702
Compare
Darkheir
approved these changes
Jul 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
We don't need this, SRTF has proved more efficient and faster. We prefer to stay close from upstream.
How was this PR tested?
If CI passes we should be good.