Cut over from SearchContext to ReaderContext #51282
Merged
Conversation
This file contains 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
dnhatn
added
>feature
:Search/Search
|
Pinging @elastic/es-search (:Search/Search) |
jimczi
approved these changes
Jan 27, 2020
| /** | ||
| * This life time is for objects that need to live until the end of the current search phase. | ||
| */ | ||
| PHASE, |
There was a problem hiding this comment.
I think we can remove Lifetime entirely. We don't use COLLECTION so it would be easier to just call addReleasable(Releasable).
There was a problem hiding this comment.
++. I removed it in 34ed39b.
| private final ShardSearchRequest request; | ||
| private final ScrollContext scrollContext; | ||
| private AggregatedDfs aggregatedDfs; | ||
| private List<RescoreContext> rescore; |
There was a problem hiding this comment.
Why do you need to keep the rescorers ? Can't we build them on each phase like we do for the Query ?
There was a problem hiding this comment.
We set rescoredDocIds in the query phase and use them in the explain of the fetch phase. If we pass the rescoredDocIds around, then we can remove this.
|
@jimczi Thank you for your review :). |
dnhatn
added a commit
that referenced
this pull request
Feb 22, 2020
With this change, we partially move the state of SearchContext to ReaderContext. This is another step allowing us to move the state of search to the coordinating node. We will need several follow-ups to move the entire search state to the coordinating node. Relates #46523
dnhatn
added a commit
that referenced
this pull request
Jun 4, 2020
Previously, we resolve IndexService and IndexShard once when creating a search context. However, since #51282, we resolve them in every phase as we recreate a search context in each phase. This change can cause the dfs_query or fetch phase to fail with ShardNotFoundException while previously did not. This behavior is quite subtle, but I think we should make it in line with the master branch.
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.
With this change, we partially move the state of SearchContext to ReaderContext. This is another step allowing us to move the state of search to the coordinating node.
We will need several follow-ups to move the entire search state to the coordinating node.
Relates #46523
Note: this pull request targets the
reader-contextbranch.