Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions quickwit/quickwit-proto/protos/quickwit/search.proto
Original file line number Diff line number Diff line change
Expand Up @@ -141,8 +141,10 @@ message ListFieldsRequest {
// When provided, only fields from documents matching this query are returned.
optional string query_ast = 5;

// Control if the request will fail if split_ids contains a split that does not exist.
// optional bool fail_on_missing_index = 6;
// Maximum number of fields to return. Overrides QW_FIELD_LIST_SIZE_LIMIT.
optional uint32 limit = 6;

reserved 7;
}

message LeafListFieldsRequest {
Expand All @@ -157,6 +159,9 @@ message LeafListFieldsRequest {
// Optional limit query to a list of fields
// Wildcard expressions are supported.
repeated string field_patterns = 4;

// Maximum number of fields to return. Overrides QW_FIELD_LIST_SIZE_LIMIT.
optional uint32 limit = 5;
}

/// Message returned by leaf and root list fields requests.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 7 additions & 3 deletions quickwit/quickwit-search/src/list_fields/leaf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ use tracing::{Span, instrument};

use crate::leaf::open_split_bundle;
use crate::list_fields::patterns::FieldPatterns;
use crate::list_fields::{merge_entries, sort_and_dedup};
use crate::list_fields::{merge_entries_with_limit_override, sort_and_dedup};
use crate::search_thread_pool;
use crate::service::SearcherContext;

Expand All @@ -45,6 +45,7 @@ pub async fn leaf_list_fields(
index_id: IndexId,
field_patterns_strs: &[String],
split_footers: Vec<SplitIdAndFooterOffsets>,
limit: Option<u32>,
searcher_ctx: Arc<SearcherContext>,
storage: Arc<dyn Storage>,
) -> crate::Result<ListFieldsResponse> {
Expand All @@ -62,7 +63,7 @@ pub async fn leaf_list_fields(
)
.await?;

let merged_entries: Vec<ListFieldsEntry> = merge_fields_metadata(all_entries).await?;
let merged_entries: Vec<ListFieldsEntry> = merge_fields_metadata(all_entries, limit).await?;

let response = ListFieldsResponse {
entries: merged_entries,
Expand Down Expand Up @@ -229,10 +230,13 @@ fn filter_fields_metadata(
#[instrument(skip_all, fields(num_splits = all_entries.len()))]
async fn merge_fields_metadata(
all_entries: Vec<Vec<ListFieldsEntry>>,
limit: Option<u32>,
) -> crate::Result<Vec<ListFieldsEntry>> {
let parent_span = Span::current();
search_thread_pool()
.run_cpu_intensive(move || parent_span.in_scope(|| merge_entries(all_entries)))
.run_cpu_intensive(move || {
parent_span.in_scope(|| merge_entries_with_limit_override(all_entries, limit))
})
.await
.context("failed to merge single split list fields")?
}
Expand Down
30 changes: 23 additions & 7 deletions quickwit/quickwit-search/src/list_fields/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,18 @@ use tracing::instrument;
pub use crate::list_fields::leaf::leaf_list_fields;
pub use crate::list_fields::root::root_list_fields;

/// QW_FIELD_LIST_SIZE_LIMIT defines a hard limit on the number of fields that
/// can be returned. When the limit is exceeded, the fields present in the most
/// splits are retained.
/// QW_FIELD_LIST_SIZE_LIMIT defines the default limit on the number of fields
/// that can be returned. A request-specific limit takes precedence. When the
/// limit is exceeded, the fields present in the most splits are retained.
///
/// Having many fields can happen when a user is creating fields dynamically in
/// a JSON type with random field names. Retaining the most common fields bounds
/// response memory while pruning the long tail of rare fields.
fn field_list_size_limit() -> usize {
quickwit_common::get_from_env_cached!(usize, "QW_FIELD_LIST_SIZE_LIMIT", 100_000, false)
/// response memory while pruning the long tail of rare fields. The default is
/// 10,000 because responses with 100,000 fields may exceed gRPC message size limits.
fn field_list_size_limit(limit: Option<u32>) -> usize {
limit.map(|limit| limit as usize).unwrap_or_else(|| {
quickwit_common::get_from_env_cached!(usize, "QW_FIELD_LIST_SIZE_LIMIT", 10_000, false)
})
}

// Sorts and deduplicates the list of fields.
Expand All @@ -63,10 +66,18 @@ fn sort_and_dedup(entries: &mut Vec<ListFieldsEntry>) {
});
}

#[cfg(test)]
fn merge_entries(entry_groups: Vec<Vec<ListFieldsEntry>>) -> crate::Result<Vec<ListFieldsEntry>> {
merge_entries_with_limit_override(entry_groups, None)
}

fn merge_entries_with_limit_override(
entry_groups: Vec<Vec<ListFieldsEntry>>,
limit: Option<u32>,
) -> crate::Result<Vec<ListFieldsEntry>> {
Ok(merge_entries_with_limit(
entry_groups,
field_list_size_limit(),
field_list_size_limit(limit),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Count field names rather than type entries

When one field has multiple observed types—for example, id appearing as both long and double—and the caller sends ?fields=id&limit=1, merge_entries_with_limittruncates the individual(field_name, field_type)entries to one._field_caps` therefore silently omits one capability type even though the response contains only one field name and has not exceeded the requested maximum; select field names first and retain all type entries belonging to each selected name.

Useful? React with 👍 / 👎.

))
}

Expand Down Expand Up @@ -204,6 +215,11 @@ mod tests {

use super::*;

#[test]
fn request_limit_overrides_configured_limit() {
assert_eq!(field_list_size_limit(Some(123)), 123);
}

#[test]
fn merge_leaf_list_fields_identical_test() {
let entry1 = ListFieldsEntry {
Expand Down
8 changes: 5 additions & 3 deletions quickwit/quickwit-search/src/list_fields/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ use quickwit_proto::types::{IndexId, IndexUid};
use quickwit_query::query_ast::QueryAst;
use tracing::{Span, instrument};

use crate::list_fields::{merge_entries, sort_and_dedup};
use crate::list_fields::{merge_entries_with_limit_override, sort_and_dedup};
use crate::search_job_placer::group_jobs_by_index_id;
use crate::{
ClusterClient, SearchError, SearchJob, list_relevant_splits, resolve_index_patterns,
Expand Down Expand Up @@ -141,7 +141,7 @@ pub async fn root_list_fields(
.into_iter()
.map(|response| response.entries)
.collect();
let merged_entries = merge_fields_metadata(leaf_entries).await?;
let merged_entries = merge_fields_metadata(leaf_entries, list_fields_req.limit).await?;
let response = ListFieldsResponse {
entries: merged_entries,
};
Expand Down Expand Up @@ -170,6 +170,7 @@ fn jobs_to_leaf_requests(
index_uri: index_meta.index_uri.to_string(),
field_patterns: search_request_for_leaf.field_patterns.clone(),
split_offsets: job_group.into_iter().map(|job| job.offsets).collect(),
limit: search_request_for_leaf.limit,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply the request limit only after the global merge

When a request spans multiple leaf requests, forwarding the exact limit causes every leaf to discard candidates before their global num_splits totals are known. For example, with limit=1, if each of two leaves has a different local field in 100 splits and the same common field in 99 splits, both leaves discard the common field even though its global count of 198 should make it the retained field. Use a leaf cap of at least max(request_limit, configured_default) and apply the requested limit only at the root so the documented most-common-field selection remains globally correct.

Useful? React with 👍 / 👎.

};
leaf_search_requests.push(leaf_search_request);
Ok(())
Expand All @@ -181,6 +182,7 @@ fn jobs_to_leaf_requests(
#[instrument(skip_all, fields(num_leaves = entry_groups.len()))]
async fn merge_fields_metadata(
mut entry_groups: Vec<Vec<ListFieldsEntry>>,
limit: Option<u32>,
) -> crate::Result<Vec<ListFieldsEntry>> {
let parent_span = Span::current();
search_thread_pool()
Expand All @@ -200,7 +202,7 @@ async fn merge_fields_metadata(
sort_and_dedup(entry_group);
}
}
merge_entries(entry_groups)
merge_entries_with_limit_override(entry_groups, limit)
})
})
.await
Expand Down
1 change: 1 addition & 0 deletions quickwit/quickwit-search/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ impl SearchService for SearchServiceImpl {
index_id,
&list_fields_req.field_patterns,
split_ids,
list_fields_req.limit,
self.searcher_context.clone(),
storage,
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ pub fn build_list_field_request_for_es_api(
start_timestamp: search_params.start_timestamp,
end_timestamp: search_params.end_timestamp,
query_ast: query_ast_json,
limit: None,
})
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ pub(crate) async fn es_compat_index_mapping(
start_timestamp: params.start_timestamp,
end_timestamp: params.end_timestamp,
query_ast: None,
limit: None,
};
let list_fields_response = match search_service.root_list_fields(list_fields_request).await {
Ok(response) => Some(response),
Expand Down
Loading