Fix ilm race condition #109230
Fix ilm race condition #109230
Conversation
There was a problem hiding this comment.
Ensures ILM execution state is copied before execution, improving stability
I am not sure I understand this, can you elaborate? This does not look like it copies execution state, rather it waits for an always true condition AFAICS.
We also need a test demonstrating the issue, will you add that too?
| @@ -30,6 +30,7 @@ | |||
| */ | |||
| public class ClusterStateObserver { | |||
|
|
|||
| public static org.elasticsearch.xpack.core.ml.MlMetadata.Builder ChangePredicate; | |||
There was a problem hiding this comment.
Can we revert this?
| } | ||
| private boolean shrunkenIndexAllocated(ClusterState clusterState, String shrunkenIndexName) { | ||
| IndexMetadata indexMetadata = clusterState.metadata().index(shrunkenIndexName); | ||
| return indexMetadata != null && indexMetadata.getState() == IndexMetadata.State.OPEN; |
There was a problem hiding this comment.
This does not ensure that the index is allocated, only that it exists and is open.
| getClient().admin().indices().resizeIndex(resizeRequest, listener.delegateFailure((l, response) -> { | ||
| ClusterStateObserver.Listener clusterStateListener = new ClusterStateObserver.Listener() { | ||
| @Override | ||
| public void onNewClusterState(ClusterState state) { |
There was a problem hiding this comment.
This is not the right way to use an observer, instead you should pass the condition as a predicate to waitForNextChange.
|
Pinging @elastic/es-data-management (Team:Data Management) |
|
Hi @sscimff, thank you for your interest in Elasticsearch and for taking the time to open a PR! I see you haven't addressed @henningandersen's comments yet. Are you still planning to work on this PR? The reason I'm asking is that we've discussed this issue (#109206) internally, and we're inclined to go with a different approach from the one you've taken here. If you're still interested in fixing this issue, I'm happy to explain our proposed solution to you. Otherwise, I'll close your PR and open one of my own. |
|
Hi @nielsbauman thank you for the message. If you have another approach and decide to implement it. Please go ahead and close this PR. |
|
We've decided internally on a different approach for this issue (see #109206 (comment)). We'll be picking this up internally as well. I'm going to close this PR, thanks nonetheless for your efforts @sscimff! |
Fixes race condition in ILM Shrink action (#109206). Ensures ILM execution state is copied before execution, improving stability.