Skip to content
Open
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
19 changes: 17 additions & 2 deletions quickwit/quickwit-metastore/src/tests/index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,9 +677,14 @@ pub async fn test_metastore_list_indexes<MetastoreToTest: MetastoreServiceExt +
let index_uri_4 = format!("ram:///indexes/{index_id_4}");
let index_config_4 = IndexConfig::for_test(&index_id_4, &index_uri_4);

let index_id_5 = format!("my-exact-index-{index_id_fragment}-5");
let index_uri_5 = format!("ram:///indexes/{index_id_5}");
let index_config_5 = IndexConfig::for_test(&index_id_5, &index_uri_5);

let index_id_patterns = vec![
format!("prefix-*-{index_id_fragment}-suffix-*"),
format!("prefix*{index_id_fragment}*suffix-*"),
format!("my-exact-index-{index_id_fragment}-5"),
];
let indexes_count = metastore
.list_indexes_metadata(ListIndexesMetadataRequest { index_id_patterns })
Expand Down Expand Up @@ -715,8 +720,17 @@ pub async fn test_metastore_list_indexes<MetastoreToTest: MetastoreServiceExt +
.unwrap()
.index_uid()
.clone();
let index_uid_5 = metastore
.create_index(CreateIndexRequest::try_from_index_config(&index_config_5).unwrap())
.await
.unwrap()
.index_uid()
.clone();

let index_id_patterns = vec![format!("prefix-*-{index_id_fragment}-suffix-*")];
let index_id_patterns = vec![
format!("prefix-*-{index_id_fragment}-suffix-*"),
format!("my-exact-index-{index_id_fragment}-5"),
];
let indexes_count = metastore
.list_indexes_metadata(ListIndexesMetadataRequest { index_id_patterns })
.await
Expand All @@ -725,12 +739,13 @@ pub async fn test_metastore_list_indexes<MetastoreToTest: MetastoreServiceExt +
.await
.unwrap()
.len();
assert_eq!(indexes_count, 2);
assert_eq!(indexes_count, 3);

cleanup_index(&mut metastore, index_uid_1).await;
cleanup_index(&mut metastore, index_uid_2).await;
cleanup_index(&mut metastore, index_uid_3).await;
cleanup_index(&mut metastore, index_uid_4).await;
cleanup_index(&mut metastore, index_uid_5).await;
}

pub async fn test_metastore_delete_index<
Expand Down
4 changes: 4 additions & 0 deletions quickwit/quickwit-proto/protos/quickwit/search.proto
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ message SearchRequest {
optional PartialHit search_after = 16;

CountHits count_hits = 17;

// When an exact index_id is provided (not a pattern), the query fails if that
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// When an exact index_id is provided (not a pattern), the query fails if that
// When an exact index ID is provided (not a pattern), the query fails only if that index is not found and this parameter is set to `false`.

// index is missing and this is false.
bool ignore_missing_indexes = 18;
}

enum CountHits {
Expand Down

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

15 changes: 13 additions & 2 deletions quickwit/quickwit-search/src/root.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,6 +366,7 @@ fn simplify_search_request_for_scroll_api(req: &SearchRequest) -> crate::Result<
// request is simplified after initial query, and we cache the hit count, so we don't need
// to recompute it afterward.
count_hits: quickwit_proto::search::CountHits::Underestimate as i32,
ignore_missing_indexes: req.ignore_missing_indexes,
})
}

Expand Down Expand Up @@ -1156,7 +1157,12 @@ async fn plan_splits_for_root_search(
.deserialize_indexes_metadata()
.await?;

check_all_index_metadata_found(&indexes_metadata[..], &search_request.index_id_patterns[..])?;
if !search_request.ignore_missing_indexes {
check_all_index_metadata_found(
&indexes_metadata[..],
&search_request.index_id_patterns[..],
)?;
}

if indexes_metadata.is_empty() {
return Ok((Vec::new(), HashMap::default()));
Expand Down Expand Up @@ -1243,7 +1249,12 @@ pub async fn search_plan(
.deserialize_indexes_metadata()
.await?;

check_all_index_metadata_found(&indexes_metadata[..], &search_request.index_id_patterns[..])?;
if !search_request.ignore_missing_indexes {
check_all_index_metadata_found(
&indexes_metadata[..],
&search_request.index_id_patterns[..],
)?;
}
Copy link
Member

Choose a reason for hiding this comment

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

Please add tests in the file. Tedious but doable :) You can also push the ignore_missing_indexes parameter to check_all_index_metadata_found and just unit test that function instead. That would make me happy too :)

if indexes_metadata.is_empty() {
return Ok(SearchPlanResponse {
result: serde_json::to_string(&SearchPlanResponseRest {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ use crate::simple_list::{from_simple_list, to_simple_list};

// Multi search doc: https://www.elastic.co/guide/en/elasticsearch/reference/current/search-multi-search.html

#[serde_as]
#[serde_with::skip_serializing_none]
#[derive(Default, Debug, Serialize, Deserialize)]
#[serde(deny_unknown_fields)]
Expand All @@ -50,6 +51,9 @@ pub struct MultiSearchQueryParams {
pub ignore_throttled: Option<bool>,
#[serde(default)]
pub ignore_unavailable: Option<bool>,
#[serde_as(deserialize_as = "OneOrMany<_, PreferMany>")]
#[serde(default)]
pub index: Vec<String>,
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment such as List of indexes to search, rename the variable to indexes and use #[serde(rename = "index")] (assuming it works with serde_as.

#[serde(default)]
pub max_concurrent_searches: Option<u64>,
#[serde(default)]
Expand Down Expand Up @@ -100,6 +104,26 @@ pub struct MultiSearchHeader {
pub routing: Option<Vec<String>>,
}

impl MultiSearchHeader {
pub fn apply_query_param_defaults(&mut self, defaults: &MultiSearchQueryParams) {
if self.allow_no_indices.is_none() {
self.allow_no_indices = defaults.allow_no_indices;
}
if self.expand_wildcards.is_none() {
self.expand_wildcards = defaults.expand_wildcards.clone();
}
if self.ignore_unavailable.is_none() {
self.ignore_unavailable = defaults.ignore_unavailable;
}
if self.index.is_empty() {
self.index = defaults.index.clone();
}
if self.routing.is_none() {
self.routing = defaults.routing.clone();
}
}
}

#[derive(Serialize)]
pub struct MultiSearchResponse {
pub responses: Vec<MultiSearchSingleResponse>,
Expand Down
18 changes: 11 additions & 7 deletions quickwit/quickwit-serve/src/elasticsearch_api/rest_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,6 +358,7 @@ fn build_request_for_es_api(

let max_hits = search_params.size.or(search_body.size).unwrap_or(10);
let start_offset = search_params.from.or(search_body.from).unwrap_or(0);
let ignore_missing_indexes = search_params.ignore_unavailable.unwrap_or(false);
let count_hits = match search_params
.track_total_hits
.or(search_body.track_total_hits)
Expand Down Expand Up @@ -410,6 +411,7 @@ fn build_request_for_es_api(
scroll_ttl_secs,
search_after,
count_hits,
ignore_missing_indexes,
},
has_doc_id_field,
))
Expand Down Expand Up @@ -814,13 +816,15 @@ async fn es_compat_index_multi_search(
let mut payload_lines = str_lines(str_payload);

while let Some(line) = payload_lines.next() {
let request_header = serde_json::from_str::<MultiSearchHeader>(line).map_err(|err| {
SearchError::InvalidArgument(format!(
"failed to parse request header `{}...`: {}",
truncate_str(line, 20),
err
))
})?;
let mut request_header =
serde_json::from_str::<MultiSearchHeader>(line).map_err(|err| {
SearchError::InvalidArgument(format!(
"failed to parse request header `{}...`: {}",
truncate_str(line, 20),
err
))
})?;
request_header.apply_query_param_defaults(&multi_search_params);
if request_header.index.is_empty() {
return Err(ElasticsearchError::from(SearchError::InvalidArgument(
"`_msearch` request header must define at least one index".to_string(),
Expand Down
1 change: 1 addition & 0 deletions quickwit/quickwit-serve/src/search_api/rest_handler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ pub fn search_request_from_api_request(
scroll_ttl_secs: None,
search_after: None,
count_hits: search_request.count_all.into(),
ignore_missing_indexes: false,
};
Ok(search_request)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,3 +104,25 @@ expected:
$expect: "len(val) == 1" # Contains only 'actor'
actor:
id: 5688
---
# test missing index
endpoint: "_msearch"
method: POST
ndjson:
- {"index":"idontexist"}
- {"query" : {"match" : { "type": "PushEvent"}}, "size": 0}
expected:
responses:
- status: 404
---
endpoint: "_msearch"
method: POST
ndjson:
- {"index":"idontexist", "ignore_unavailable": true}
- {"query" : {"match" : { "type": "PushEvent"}}, "size": 0}
expected:
responses:
- hits:
total:
value: 0
status: 200
Copy link
Member

Choose a reason for hiding this comment

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

nit: please add missing new line

Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
endpoint: "idontexist/_search"
params:
q: "*"
status_code: 404
---
endpoint: "idontexist/_search"
params:
q: "*"
ignore_unavailable: "true"
expected:
hits:
total:
value: 0
---
endpoint: "gharchive-*,idontexist/_search"
params:
q: "*"
status_code: 404
---
endpoint: "gharchive-*,idontexist/_search"
params:
q: "*"
ignore_unavailable: "true"
expected:
hits:
total:
value: 4



Copy link
Member

Choose a reason for hiding this comment

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

Please, remove the extra empty lines and insert the missing new line. Your editor can do that automatically.