Refactor TransportShardReplicationOperationAction #10749
Closed
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
bleskes
added
>enhancement
v2.0.0-beta1
review
:Core/Infra/Core
|
@martijnvg @brwe can you take a look? |
| // we already marked it as started when we executed it (removed the listener) so pass false | ||
| // to re-add to the cluster listener | ||
| logger.trace("received an error from node the primary was assigned to ({}), scheduling a retry", exp.getMessage()); | ||
| retry(exp); |
There was a problem hiding this comment.
what if retry() throws an exception?
| } | ||
|
|
||
| public void start() { | ||
| this.observer = new ClusterStateObserver(clusterService, internalRequest.request().timeout(), logger); | ||
| doStart(); | ||
| try { |
There was a problem hiding this comment.
make start() package protected?
There was a problem hiding this comment.
I changed it to be AbstractRunnable for other reasons.. the class is package private now
|
@martijnvg @brwe pushed an update with all the feedback + extra testing. |
| public void handleException(TransportException exp) { | ||
| onReplicaFailure(nodeId, exp); | ||
| logger.trace("[{}] transport failure during replica request [{}] ", exp, node, replicaRequest); | ||
| if (!ignoreReplicaException(exp)) { |
|
I have to review the logic agian but the test alone makes me +1 this |
|
LGTM |
bleskes
added a commit
that referenced
this pull request
Apr 24, 2015
Refactor TransportShardReplicationOperationAction state management into clear separate Primary phase and Replication phase. The primary phase is responsible for routing the request to the node holding the primary, validating it and performing the operation on the primary. The Replication phase is responsible for sending the request to the replicas and managing their responses. This also adds unit test infrastructure for this class, and some basic tests. We can extend later as we continue developing. Closes #10749
|
pushed to 1.x as well. |
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.
Refactors TransportShardReplicationOperationAction state management into clear separate Primary phase and Replication phase. The primary phase is responsible for routing the request to the node holding the primary, validating it and performing the operation on the primary. The Replication phase is responsible for sending the request to the replicas and managing their responses.
This refactoring is aimed at simplifying adding operation start and end hooks that are needed for the counter mentioned in #10032 .
This also adds unit test infrastructure for this class, and some basic tests. We can extend later as we continue developing.
For now, this is planned to go to 2.0.0 but we make backport it to 1.6.0, depending how work goes with #10032