Skip to content

Reuse existing allocation id for primary shard allocation #16530

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2016

Conversation

ywelsch
Copy link
Contributor

@ywelsch ywelsch commented Feb 9, 2016

This solves an issue where shard was activated by master but the cluster crashes before the target node persisted the shard state.

Relates to #14739

@@ -411,13 +411,17 @@ void moveToUnassigned(UnassignedInfo unassignedInfo) {
/**
* Initializes an unassigned shard on a node.
*/
void initialize(String nodeId, long expectedShardSize) {
void initialize(String nodeId, String existingAllocationId, long expectedShardSize) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we document that the existingAllocationId can be null?

@bleskes
Copy link
Contributor

bleskes commented Feb 11, 2016

Left some very minor comments...

@ywelsch ywelsch force-pushed the fix/reuse-alloc-id branch 2 times, most recently from d6ec6cd to ed93ebe Compare February 11, 2016 10:43
@ywelsch
Copy link
Contributor Author

ywelsch commented Feb 11, 2016

@bleskes Thanks for the review! I've updated the PR according to your suggestions and made one additional change: I removed the overloads of the initialize method that I earlier introduced in ShardRouting, RoutingNodes and UnassignedShards as there are not that many call sites for these methods. With each of these methods now being properly documented, I believe that adding null on these very few method calls to be cleaner (similar to -1 that is present for expectedShardSize on most call sites).

@bleskes
Copy link
Contributor

bleskes commented Feb 11, 2016

LGTM. Thanks @ywelsch

ywelsch pushed a commit that referenced this pull request Feb 11, 2016
Reuse existing allocation id for primary shard allocation
@ywelsch ywelsch merged commit 092d381 into elastic:master Feb 11, 2016
@lcawl lcawl added :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. and removed :Allocation labels Feb 13, 2018
@clintongormley clintongormley added :Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) and removed :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. labels Feb 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Allocation All issues relating to the decision making around placing a shard (both master logic & on the nodes) >enhancement v5.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants