Fold InitialSearchPhase into AbstractSearchAsyncAction #47182
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
Historically, we have two base classes for search actions that generally need to fan out to multiple shards and then move on to the following phase: InitialSearchPhase and AbstractSearchAsyncAction that extends it. Practically, every search action extends the latter, and there are no direct subclasses of InitialSearchPhase in our codebase. This commit folds InitialSearchPhase into AbstractSearchAsyncAction in the attempt of simplifying things and making the search code running on the coordinating node easier to reason about.
javanna
added
>enhancement
:Search/Search
|
Pinging @elastic/es-search |
javanna
commented
Sep 26, 2019
| latch.countDown(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I am curious to hear if I am over-simplifying things here. This is how the next phase used to be before #27069 . I wonder if it's necessary for the next phase to be an AbstractSearchAsyncAction subclass at this point, which is not so easy to recreate.
There was a problem hiding this comment.
I think it's important to keep since this test checks random failures too ? We can maybe just override executePhaseOnShard but I am not sure if onShardFailure is also needed ?
|
run elasticsearch-ci/2 |
jimczi
approved these changes
Sep 27, 2019
| latch.countDown(); | ||
| } | ||
|
|
There was a problem hiding this comment.
I think it's important to keep since this test checks random failures too ? We can maybe just override executePhaseOnShard but I am not sure if onShardFailure is also needed ?
|
@elasticmachine update branch |
javanna
added a commit
to javanna/elasticsearch
that referenced
this pull request
Oct 4, 2019
Historically, we have two base classes for search actions that generally need to fan out to multiple shards and then move on to the following phase: InitialSearchPhase and AbstractSearchAsyncAction that extends it. Practically, every search action extends the latter, and there are no direct subclasses of InitialSearchPhase in our codebase. This commit folds InitialSearchPhase into AbstractSearchAsyncAction in the attempt of simplifying things and making the search code running on the coordinating node easier to reason about.
javanna
added a commit
that referenced
this pull request
Oct 7, 2019
Historically, we have two base classes for search actions that generally need to fan out to multiple shards and then move on to the following phase: InitialSearchPhase and AbstractSearchAsyncAction that extends it. Practically, every search action extends the latter, and there are no direct subclasses of InitialSearchPhase in our codebase. This commit folds InitialSearchPhase into AbstractSearchAsyncAction in the attempt of simplifying things and making the search code running on the coordinating node easier to reason about.
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.
Historically, we have two base classes for search actions that generally need to fan out to multiple shards and then move on to the following phase: InitialSearchPhase and AbstractSearchAsyncAction that extends it. Practically, every search action extends the latter, and there are no direct subclasses of InitialSearchPhase in our codebase.
This commit folds InitialSearchPhase into AbstractSearchAsyncAction in the attempt of simplifying things and making the search code running on the coordinating node easier to reason about.