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

Use provided cluster state for indices service validations #10014

Closed
wants to merge 1 commit into from

Conversation

@kimchy
Copy link
Member

commented Mar 6, 2015

Since the method can be called in an #execute event of the cluster service, we need the ability to use the cluster state that will be provided in the ClusterChangedEvent, have the ClusterState be provided as a parameter

Use provided cluster state for indices service validations
Since the method can be called in an #execute event of the cluster service, we need the ability to use the cluster state that will be provided in the ClusterChangedEvent, have the ClusterState be provided as a parameter
@martijnvg

This comment has been minimized.

Copy link
Member

commented Mar 6, 2015

good change, LGTM

@martijnvg martijnvg removed the review label Mar 6, 2015

@kimchy

This comment has been minimized.

Copy link
Member Author

commented Mar 6, 2015

git merge fail, closing manually

@kimchy kimchy closed this Mar 6, 2015

@kimchy kimchy deleted the kimchy:pass_cluster_state branch Mar 6, 2015

@s1monw

This comment has been minimized.

Copy link
Contributor

commented Mar 6, 2015

was there a bug due to that? I wonder if there was one what test failed?

@@ -530,7 +533,7 @@ public void run() {
}
logger.warn("[{}] deleting dangling index", metaData.index());
try {
indicesService.deleteIndexStore("deleting dangling index", metaData);
indicesService.deleteIndexStore("deleting dangling index", metaData, clusterService.state());

This comment has been minimized.

Copy link
@s1monw

s1monw Mar 6, 2015

Contributor

if we pass the state IMO we should document what state it is? I also wonder if we should get the metadata from the state direclty instead of passing it. And then just pass the index name?

This comment has been minimized.

Copy link
@kimchy

kimchy Mar 6, 2015

Author Member

I got to this change when I tried to remove all the options around dangling indices, this code will go away soon :)

This comment has been minimized.

Copy link
@kimchy

kimchy Mar 6, 2015

Author Member

also, in IndicesClusterStateService, we pass a different IndexMetaData, so the flexibility to pass them both is needed?

@@ -467,7 +464,7 @@ public void deleteClosedIndex(String reason, IndexMetaData metaData) {
* Deletes the index store trying to acquire all shards locks for this index.
* This method will delete the metadata for the index even if the actual shards can't be locked.
*/
public void deleteIndexStore(String reason, IndexMetaData metaData) throws IOException {
public void deleteIndexStore(String reason, IndexMetaData metaData, ClusterState clusterState) throws IOException {

This comment has been minimized.

Copy link
@s1monw

s1monw Mar 6, 2015

Contributor

can we please update the signature here to reflect what state this is? I am not sure if we should do this here. The internal state wasn't good enough?

This comment has been minimized.

Copy link
@kimchy

kimchy Mar 6, 2015

Author Member

sure, missed it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.