Stop policy on last PhaseCompleteStep instead of TerminalPolic… #51631
Conversation
This file contains 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
Currently when an ILM policy finishes its execution, the index moves into the `TerminalPolicyStep`, denoted by a completed/completed/completed phase/action/step lifecycle execution state. This commit changes the behavior so that the index lifecycle execution state halts at the last configured phase's `PhaseCompleteStep`, so for instance, if an index were configured with a policy containing a `hot` and `cold` phase, the index would stop at the `cold/complete/complete` `PhaseCompleteStep`. This allows an ILM user to update the policy to add any later phases and have indices configured to use that policy pick up execution at the newly added "later" phase. For example, if a `delete` phase were added to the policy specified about, the index would then move from `cold/complete/complete` into the `delete` phase. Relates to elastic#48431
dakrone
added
>enhancement
:Data Management/ILM+SLM
|
Pinging @elastic/es-core-features (:Core/Features/ILM+SLM) |
andreidan
reviewed
Jan 30, 2020
x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/ExecuteStepsUpdateTask.java
Show resolved
Hide resolved
...ugin/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java
Show resolved
Hide resolved
x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java
Show resolved
Hide resolved
| }); | ||
| } | ||
|
|
||
| public void testMasterFailover() throws Exception { |
There was a problem hiding this comment.
was the deletion of this test intentional? if so shall we do it in a different commit so we have some semantics as to why it was removed?
There was a problem hiding this comment.
This was a test that didn't really do much outside of what we already have tested elsewhere, in addition, it was using a fake step that was near impossible to make work with the way that phases now operate, so I removed it.
I don't think a separate commit is that useful, since this ends up squashed into a single commit anyway?
There was a problem hiding this comment.
Ah, yeah, you're right (not sure how should we signal the reasoning behind these changes for future reference - commit message?)
|
LGTM, thanks @dakrone! |
probakowski
approved these changes
Jan 30, 2020
andreidan
approved these changes
Jan 31, 2020
dakrone
changed the title
dakrone
added a commit
to dakrone/elasticsearch
that referenced
this pull request
Jan 31, 2020
…tic#51631) Currently when an ILM policy finishes its execution, the index moves into the `TerminalPolicyStep`, denoted by a completed/completed/completed phase/action/step lifecycle execution state. This commit changes the behavior so that the index lifecycle execution state halts at the last configured phase's `PhaseCompleteStep`, so for instance, if an index were configured with a policy containing a `hot` and `cold` phase, the index would stop at the `cold/complete/complete` `PhaseCompleteStep`. This allows an ILM user to update the policy to add any later phases and have indices configured to use that policy pick up execution at the newly added "later" phase. For example, if a `delete` phase were added to the policy specified about, the index would then move from `cold/complete/complete` into the `delete` phase. Relates to elastic#48431
This was referenced Feb 11, 2020
dakrone
added a commit
to dakrone/elasticsearch
that referenced
this pull request
Apr 14, 2020
Prior to the change in elastic#51631 indices were moved to the `TerminalPolicyStep` when their ILM actions had completed. Once we switched ILM to stop in the last policy configured, these steps because inaccessible from the policy's perspective. This meant that indices upgraded from ES prior to 7.7.0 could see the following error spammed in their logs every 10 minutes (by default) for every index in this state: ``` [2020-04-14T15:52:23,764][ERROR][o.e.x.i.IndexLifecycleRunner] [midgar] current step [{"phase":"completed","action":"completed","name":"completed"}] for index [foo] with policy [full] is not recognized ``` This changes the runner to ignore these steps, which is what is desired anyway since the index is already in the terminal phase.
dakrone
added a commit
that referenced
this pull request
Apr 14, 2020
Prior to the change in #51631 indices were moved to the `TerminalPolicyStep` when their ILM actions had completed. Once we switched ILM to stop in the last policy configured, these steps because inaccessible from the policy's perspective. This meant that indices upgraded from ES prior to 7.7.0 could see the following error spammed in their logs every 10 minutes (by default) for every index in this state: ``` [2020-04-14T15:52:23,764][ERROR][o.e.x.i.IndexLifecycleRunner] [midgar] current step [{"phase":"completed","action":"completed","name":"completed"}] for index [foo] with policy [full] is not recognized ``` This changes the runner to ignore these steps, which is what is desired anyway since the index is already in the terminal phase.
dakrone
added a commit
that referenced
this pull request
Apr 14, 2020
Prior to the change in #51631 indices were moved to the `TerminalPolicyStep` when their ILM actions had completed. Once we switched ILM to stop in the last policy configured, these steps because inaccessible from the policy's perspective. This meant that indices upgraded from ES prior to 7.7.0 could see the following error spammed in their logs every 10 minutes (by default) for every index in this state: ``` [2020-04-14T15:52:23,764][ERROR][o.e.x.i.IndexLifecycleRunner] [midgar] current step [{"phase":"completed","action":"completed","name":"completed"}] for index [foo] with policy [full] is not recognized ``` This changes the runner to ignore these steps, which is what is desired anyway since the index is already in the terminal phase.
dakrone
added a commit
that referenced
this pull request
Apr 14, 2020
Prior to the change in #51631 indices were moved to the `TerminalPolicyStep` when their ILM actions had completed. Once we switched ILM to stop in the last policy configured, these steps because inaccessible from the policy's perspective. This meant that indices upgraded from ES prior to 7.7.0 could see the following error spammed in their logs every 10 minutes (by default) for every index in this state: ``` [2020-04-14T15:52:23,764][ERROR][o.e.x.i.IndexLifecycleRunner] [midgar] current step [{"phase":"completed","action":"completed","name":"completed"}] for index [foo] with policy [full] is not recognized ``` This changes the runner to ignore these steps, which is what is desired anyway since the index is already in the terminal phase.
yyff
pushed a commit
to yyff/elasticsearch
that referenced
this pull request
Apr 17, 2020
Prior to the change in elastic#51631 indices were moved to the `TerminalPolicyStep` when their ILM actions had completed. Once we switched ILM to stop in the last policy configured, these steps because inaccessible from the policy's perspective. This meant that indices upgraded from ES prior to 7.7.0 could see the following error spammed in their logs every 10 minutes (by default) for every index in this state: ``` [2020-04-14T15:52:23,764][ERROR][o.e.x.i.IndexLifecycleRunner] [midgar] current step [{"phase":"completed","action":"completed","name":"completed"}] for index [foo] with policy [full] is not recognized ``` This changes the runner to ignore these steps, which is what is desired anyway since the index is already in the terminal phase.
cjcenizal
added
the
Team:Deployment Management
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Currently when an ILM policy finishes its execution, the index moves into the
TerminalPolicyStep,denoted by a completed/completed/completed phase/action/step lifecycle execution state.
This commit changes the behavior so that the index lifecycle execution state halts at the last
configured phase's
PhaseCompleteStep, so for instance, if an index were configured with a policycontaining a
hotandcoldphase, the index would stop at thecold/complete/completePhaseCompleteStep. This allows an ILM user to update the policy to add any later phases and haveindices configured to use that policy pick up execution at the newly added "later" phase. For
example, if a
deletephase were added to the policy specified about, the index would then movefrom
cold/complete/completeinto thedeletephase.Relates to #48431