Move to Error step if ClusterState* steps throw #35069
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously, if ClusterStateActionSteps or ClusterStateWaitSteps threw an exception executing, the exception would only be caught and logged by the generic ClusterStateUpdateTask machinery and the index would become stuck on that step. Now, exceptions thrown in these steps will be caught and the index will be moved to the Error step.
gwbrown
added
the
:Data Management/ILM+SLM
Collaborator
|
Pinging @elastic/es-core-infra |
talevy
approved these changes
Oct 29, 2018
Contributor
talevy
left a comment
talevy
left a comment
There was a problem hiding this comment.
left some comments I think should be addressed
| "policy [" + policy + "] for index [" + index.getName() + "] failed on step [" + startStep.getKey() + "].", e); | ||
| } | ||
|
|
||
| private ClusterState moveToErrorStep(final ClusterState state, Step.StepKey currentStepKey, Exception cause) throws IOException { |
Contributor
There was a problem hiding this comment.
this should probably be RuntimeException here?
...in/ilm/src/test/java/org/elasticsearch/xpack/indexlifecycle/ExecuteStepsUpdateTaskTests.java
Show resolved
Hide resolved
dakrone
requested changes
Oct 30, 2018
| state = ((ClusterStateActionStep) currentStep).performAction(index, state); | ||
| try { | ||
| state = ((ClusterStateActionStep) currentStep).performAction(index, state); | ||
| } catch (RuntimeException exception) { |
Member
There was a problem hiding this comment.
Is there a reason we can't catch Exception here?
Contributor
Author
There was a problem hiding this comment.
This was left over from an earlier draft of this that had more in the try block, and I wasn't sure if we wanted to include IOException in this catch. I've changed it.
.../plugin/ilm/src/main/java/org/elasticsearch/xpack/indexlifecycle/ExecuteStepsUpdateTask.java
Show resolved
Hide resolved
| } | ||
|
|
||
| private ClusterState moveToErrorStep(final ClusterState state, Step.StepKey currentStepKey, RuntimeException cause) throws IOException { | ||
| logger.error("policy [{}] for index [{}] failed on step [{}]. Moving to ERROR step", policy, index.getName(), currentStepKey); |
dakrone
approved these changes
Oct 30, 2018
gwbrown
added a commit
that referenced
this pull request
Oct 30, 2018
Previously, if ClusterStateActionSteps or ClusterStateWaitSteps threw an exception executing, the exception would only be caught and logged by the generic ClusterStateUpdateTask machinery and the index would become stuck on that step. Now, exceptions thrown in these steps will be caught and the index will be moved to the Error step.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously, if ClusterStateActionSteps or ClusterStateWaitSteps threw an
exception while executing, the exception would only be caught and logged by
the generic ClusterStateUpdateTask machinery and the index would become
stuck on that step.
Now, exceptions thrown in these steps will be caught and the index will
be moved to the Error step.
Credit to @talevy for the test cases, turns out we both worked on this separately and he got to them first.