Skip can match when batched query execution is available #127471
+316
−131
Conversation
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
No need to do the can_match phase for those nodes that support the batched query phase. We can get the equivalent degree of pre-filtering by simply running can_match for sort based queries.
There's no reason whatsoever to run can_match (except the coordinator rewrite part of it) when batched query execution is used. On a per-node level we can still run it to order the shrds but shoul probably remove its use from the query phase completely if there's no sort in the query.
original-brownbear
added
>non-issue
:Search Foundations/Search
…skip-can-match-on-batched
| @@ -562,15 +643,39 @@ static void registerNodeSearchAction( | |||
| final int searchPoolMax = threadPool.info(ThreadPool.Names.SEARCH).getMax(); | |||
| transportService.registerRequestHandler( | |||
| NODE_SEARCH_ACTION_NAME, | |||
| EsExecutors.DIRECT_EXECUTOR_SERVICE, | |||
| threadPool.executor(ThreadPool.Names.SEARCH_COORDINATION), | |||
There was a problem hiding this comment.
Now that we do can_match here as well we absolutely must fork this to search_coordination, at least for the time being.
| shardSearchRequests[i] = r; | ||
| var canMatch = searchService.canMatch(r); | ||
| if (canMatch.canMatch()) { | ||
| r.setRunCanMatchInQueryPhase(false); |
There was a problem hiding this comment.
A bit hacky for now and we should clean this logic up further, but somewhat of a nice speedup to save one round of creating a searcher and whatnot and a win we should take right away IMO.
| final var shardId = shardToQuery.shardId; | ||
| ShardSearchRequest r = state.shardSearchRequests == null ? null : state.shardSearchRequests[translatedIndex]; |
There was a problem hiding this comment.
A bit messy too for the time being, but kind of neat to save building the requst again as a win in this step, we can refine this further to make it look nicer and maybe also dedup some of the lucene internal query object building down the line.
original-brownbear
changed the title
|
Pinging @elastic/es-search-foundations (Team:Search Foundations) |
elasticsearchmachine
added
the
Team:Search Foundations
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.
Admittedly a little dirty, but intentionally kept close to the exitsting logic, we can clean up and optimize in follow-ups, in any case this is a considerable speedup in essentially all possible use cases (we always save a network roundtrip inlcuding serialization and everything).
There's no reason whatsoever to run can_match (except the coordinator rewrite part of it)
when batched query execution is used.
On a per-node level we can still run it to order the shrds but shoul probably remove its
use from the query phase completely if there's no sort in the query.
relates #125788