-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Include release highlight for query rewrite #97178
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Include release highlight for query rewrite #97178
Conversation
|
Pinging @elastic/es-analytics-geo (Team:Analytics) |
|
Pinging @elastic/es-search (Team:Search) |
|
I wonder if we can make the highlight title more catchy. From a user perspective this will yield both better search performance (because fewer refreshes run as part of search requests) and indexing performative (because fewer refreshes, so less merging). So maybe something like "Better indexing and search performance under concurrent indexing and search" or something along these lines? |
docs/changelog/96161.yaml
Outdated
| - 95541 | ||
| highlight: | ||
| title: Skip shards when querying constant keyword fields | ||
| body: "When a query like a match phrase query or a wildcard query targets a constant keyword field \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think wildcard query in 8.9.0 doesn't overwrite the indexMetadataRewrite(...) method? Only match_phrase and term queries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes you are right...because it uses the SearchExecutionContext.
docs/changelog/96161.yaml
Outdated
| document. This will result in the shard level request to return immediately, before the query is executed on the data\ | ||
| node and, as a result, skipping the shard completely. In a real world scenario it is likely that index patterns or\ | ||
| data streams include tens or hundreds of backing indices each of them with multiple shards involved. Skipping shards\ | ||
| in such scenario might result in better query performance and better cluster resource usage. Note, anyway, that\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This exlpains how skipping shards helps with search performance, which is true, but Elasticsearch has had this for years. Let's focus instead of the new change, which is that the can_match phase no longer needs to refresh search-idle shards in a number of cases?
Quickly mention better indexing performance here as well thanks to less refreshing and merging?
docs/changelog/96161.yaml
Outdated
| in such scenario might result in better query performance and better cluster resource usage. Note, anyway, that\ | ||
| execution of the pre-filter and the corresponding \"can match\" phase where rewriting happens, depends on the overall | ||
| number of shards involved and on whether there is at least one of them returning a non-empty result (see\ | ||
| 'pre_filter_shard_size' setting to understand how to control this behaviour). We do the rewrite operation on the data\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 'pre_filter_shard_size' setting to understand how to control this behaviour). We do the rewrite operation on the data\ | |
| 'pre_filter_shard_size' setting to understand how to control this behaviour). Elasticsearch does the rewrite operation on the data\ |
docs/changelog/96161.yaml
Outdated
| match the value defined in the index mapping, we rewrite the query to match no document. This will result in the\ | ||
| shard level request to return immediately, before the query is executed on the data node and, as a result, skipping\ | ||
| the shard completely. Here we leverage the ability to skip shards whenever possible to avoid unnecessary shard\ | ||
| refreshes and improve query latency. Avoiding such unnecessary shard refreshes improves query latency since the\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe change and improve query latency into and improve query latency (by not doing any search related i/o)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
docs/changelog/96161.yaml
Outdated
| shard level request to return immediately, before the query is executed on the data node and, as a result, skipping\ | ||
| the shard completely. Here we leverage the ability to skip shards whenever possible to avoid unnecessary shard\ | ||
| refreshes and improve query latency. Avoiding such unnecessary shard refreshes improves query latency since the\ | ||
| search thread does not need to wait anymore for unnecessary shard refreshes. Before introducing this change a query\ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe also add after search thread does not need to wait anymore for unnecessary shard refreshes., that searches that don't match the query will remain search idle and therefor the indexing throughput won't be impacted negatively by a refresh.
Update the changelog after making the original PR a `release highlight`. See elastic#96161
💚 Backport successful
|
Update the changelog after making the original PR a
release highlight.See #96161