Weaken node-replacement decider during reconciliation #95070
Merged
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
The node-replacement allocation decider requires shard movements to follow a specific route, from source to replacement target. However during the shutdown there may be other changes in the system that make the replacement target unsuitable for the final destination of the shard. Having simulated the move of the shard onto the replacement target we are free to simulate its movement elsewhere, but today this causes the reconciler to get stuck: it cannot move the shard to its desired location because of the ongoing replacement, and it will not move the shard onto the replacement target because that's not its desired location. This commit suppresses this decider during reconciliation which allows the reconcilier to skip the intermediate target node and move the shard straight to its desired location.
DaveCTurner
added
>bug
:Distributed Coordination/Allocation
Collaborator
|
Hi @DaveCTurner, I've created a changelog YAML for you. |
Collaborator
|
Pinging @elastic/es-distributed (Team:Distributed) |
elasticsearchmachine
added
the
Team:Distributed (Obsolete)
DaveCTurner
commented
Apr 6, 2023
| INDEX, | ||
| Settings.builder() | ||
| .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) | ||
| .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, between(1, 5)) |
Contributor
Author
There was a problem hiding this comment.
This is the relevant test change: with more than 1 shard, there are enough other nodes in the cluster for the eventual desired balance to spread them out across multiple nodes even though the node replacement decider doesn't like that.
| .allMatch(s -> s.overallStatus() == SingleNodeShutdownMetadata.Status.COMPLETE) | ||
| ); | ||
| }); | ||
| }, 120, TimeUnit.SECONDS); |
Contributor
Author
There was a problem hiding this comment.
This may be unnecessary but 30s seems like it might be a little short to relocate 5 shards if the CI worker is running egregiously slowly.
Contributor
There was a problem hiding this comment.
NIT: should we make it dependent on the shard count?
Contributor
Author
There was a problem hiding this comment.
not really worth it IMO but I can if you insist :)
Comment on lines
36
to
37
| if (allocation.isReconciling()) { | ||
| return YES__RECONCILING; |
Contributor
Author
There was a problem hiding this comment.
These two lines are the relevant fix here, everything else is just tidying up the use of Decision constants.
Contributor
There was a problem hiding this comment.
This seems fairly permitting condition. I wonder if there any tricky situations when we could permit moving shard to the node that is shutting down? Maybe while the balance is still being computed for the node shutdown?
33 tasks
idegtiarenko
approved these changes
Apr 6, 2023
DaveCTurner
changed the title
DaveCTurner
added
auto-backport-and-merge
auto-merge-without-approval
DaveCTurner
removed
the
auto-merge-without-approval
DaveCTurner
deleted the
2023-04-06-ignore-replace-decider-during-reconciliation
branch
DaveCTurner
added a commit
to DaveCTurner/elasticsearch
that referenced
this pull request
Apr 6, 2023
The node-replacement allocation decider requires shard movements to follow a specific route, from source to replacement target. However during the shutdown there may be other changes in the system that make the replacement target unsuitable for the final destination of the shard. Having simulated the move of the shard onto the replacement target we are free to simulate its movement elsewhere, but today this causes the reconciler to get stuck: it cannot move the shard to its desired location because of the ongoing replacement, and it will not move the shard onto the replacement target because that's not its desired location. This commit weakens this decider during reconciliation which allows the reconcilier to skip the intermediate target node and move the shard straight to its desired location.
Collaborator
💚 Backport successful
|
elasticsearchmachine
pushed a commit
that referenced
this pull request
Apr 6, 2023
The node-replacement allocation decider requires shard movements to follow a specific route, from source to replacement target. However during the shutdown there may be other changes in the system that make the replacement target unsuitable for the final destination of the shard. Having simulated the move of the shard onto the replacement target we are free to simulate its movement elsewhere, but today this causes the reconciler to get stuck: it cannot move the shard to its desired location because of the ongoing replacement, and it will not move the shard onto the replacement target because that's not its desired location. This commit weakens this decider during reconciliation which allows the reconcilier to skip the intermediate target node and move the shard straight to its desired location.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
The node-replacement allocation decider requires shard movements to follow a specific route, from source to replacement target. However during the shutdown there may be other changes in the system that make the replacement target unsuitable for the final destination of the shard. Having simulated the move of the shard onto the replacement target we are free to simulate its movement elsewhere, but today this causes the reconciler to get stuck: it cannot move the shard to its desired location because of the ongoing replacement, and it will not move the shard onto the replacement target because that's not its desired location.
This commit weakens this decider during reconciliation which allows the reconcilier to skip the intermediate target node and move the shard straight to its desired location.