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

Resolve index names to Index instances early #17048

Merged

Conversation

s1monw
Copy link
Contributor

@s1monw s1monw commented Mar 10, 2016

Today index names are often resolved lazily, only when they are really
needed. This can be problematic especially when it gets to mapping updates
etc. when a node sends a mapping update to the master but while the request
is in-flight the index changes for whatever reason we would still apply the update
since we use the name of the index to identify the index in the clusterstate.
The problem is that index names can be reused which happens in practice and sometimes
even in a automated way rendering this problem as realistic.
In this change we resolve the index including it's UUID as early as possible in places
where changes to the clusterstate are possible. For instance mapping updates on a node use a
concrete index rather than it's name and the master will fail the mapping update iff
the index can't be found by it's <name, uuid> tuple.

@@ -65,6 +68,7 @@
private String source;

private boolean updateAllTypes = false;
private Index concretIndex;
Copy link
Contributor

Choose a reason for hiding this comment

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

misspelling: concreteIndex

@ywelsch
Copy link
Contributor

ywelsch commented Mar 11, 2016

Left minor comments. No second round of reviews needed on my part. Thanks @s1monw

@s1monw s1monw force-pushed the use_concrete_index_on_clusterstate_update branch 4 times, most recently from 36e702b to 7829ef2 Compare March 14, 2016 09:35
@s1monw s1monw force-pushed the use_concrete_index_on_clusterstate_update branch from 7829ef2 to 5ad18b1 Compare March 14, 2016 09:59
Today index names are often resolved lazily, only when they are really
needed. This can be problematic especially when it gets to mapping updates
etc. when a node sends a mapping update to the master but while the request
is in-flight the index changes for whatever reason we would still apply the update
since we use the name of the index to identify the index in the clusterstate.
The problem is that index names can be reused which happens in practice and sometimes
even in a automated way rendering this problem as realistic.
In this change we resolve the index including it's UUID as early as possible in places
where changes to the clusterstate are possible. For instance mapping updates on a node use a
concrete index rather than it's name and the master will fail the mapping update iff
the index can't be found by it's <name, uuid> tuple.

Closes elastic#17048
@s1monw s1monw force-pushed the use_concrete_index_on_clusterstate_update branch from 5ad18b1 to 31740e2 Compare March 14, 2016 10:09
s1monw added a commit that referenced this pull request Mar 14, 2016
…ate_update

Resolve index names to Index instances early

Today index names are often resolved lazily, only when they are really
needed. This can be problematic especially when it gets to mapping updates
etc. when a node sends a mapping update to the master but while the request
is in-flight the index changes for whatever reason we would still apply the update
since we use the name of the index to identify the index in the clusterstate.
The problem is that index names can be reused which happens in practice and sometimes
even in a automated way rendering this problem as realistic.
In this change we resolve the index including it's UUID as early as possible in places
where changes to the clusterstate are possible. For instance mapping updates on a node use a
concrete index rather than it's name and the master will fail the mapping update iff
the index can't be found by it's <name, uuid> tuple.

Closes #17048
@s1monw s1monw merged commit a69d44c into elastic:master Mar 14, 2016
@s1monw s1monw deleted the use_concrete_index_on_clusterstate_update branch March 14, 2016 10:40
@s1monw
Copy link
Contributor Author

s1monw commented Mar 14, 2016

thanks @ywelsch for the review

@clintongormley clintongormley added :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. and removed :Cluster 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 v5.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants