Skip to content
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

Verify shards index UUID when fetching started shards #10200

Merged
merged 1 commit into from
Mar 23, 2015

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Mar 21, 2015

Today we simply fetch the shards metadata without verifying the
index UUID the shard belongs to. We recently added this UUID
to the shard state metadata. This commit adds verification
to the shard metadata fetching to prevent bringing shards
back into an index it doesn't belong to due to name collisions.

@s1monw
Copy link
Contributor Author

s1monw commented Mar 23, 2015

@bleskes wanna take a look

public ActionFuture<NodesGatewayStartedShards> list(ShardId shardId, String[] nodesIds, @Nullable TimeValue timeout) {
return execute(new Request(shardId, nodesIds).timeout(timeout));
public ActionFuture<NodesGatewayStartedShards> list(ShardId shardId, String indexUUID, String[] nodesIds, @Nullable TimeValue timeout) {
return execute(new Request(shardId, indexUUID, Arrays.asList(nodesIds)).timeout(timeout));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the nodes can stay as an array. The constructor just converts to an array again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah so I did this since I didn't want the String, String... signature.. kind of confusing

Copy link
Contributor

Choose a reason for hiding this comment

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

agreed, but what's wrong with Request(ShardId shardId, String[] nodesIds) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true :) will fix

@bleskes
Copy link
Contributor

bleskes commented Mar 23, 2015

Left one minor comment. LGTM otherwise.

@s1monw
Copy link
Contributor Author

s1monw commented Mar 23, 2015

pushed a new commit

Today we simply fetch the shards metadata without verifying the
index UUID the shard belongs to. We recently added this UUID
to the shard state metadata. This commit adds verification
to the shard metadata fetching to prevent bringing shards
back into an index it doesn't belong to due to name collisions.
@bleskes
Copy link
Contributor

bleskes commented Mar 23, 2015

LGTMx2

@s1monw s1monw merged commit cae2707 into elastic:master Mar 23, 2015
@clintongormley clintongormley changed the title [ALLOCATION] Verify shards index UUID when fetching started shards Verify shards index UUID when fetching started shards Jun 8, 2015
@lcawl lcawl added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. and removed :Allocation labels Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. >enhancement resiliency v2.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants