Skip to content

Commit

Permalink
[7.x] Stop policy on last PhaseCompleteStep instead of Termina… (#51758)
Browse files Browse the repository at this point in the history 
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 #48431
  • Loading branch information
@dakrone
dakrone committed Jan 31, 2020
1 parent 3147453 commit deefc85
Show file tree
Hide file tree
Showing 11 changed files with 157 additions and 145 deletions.
18 changes: 7 additions & 11 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,9 +174,7 @@ public List<Step> toSteps(Client client) {
List<Phase> orderedPhases = type.getOrderedPhases(phases);
ListIterator<Phase> phaseIterator = orderedPhases.listIterator(orderedPhases.size());

// final step so that policy can properly update cluster-state with last action completed
steps.add(TerminalPolicyStep.INSTANCE);
Step.StepKey lastStepKey = TerminalPolicyStep.KEY;
Step.StepKey lastStepKey = null;

Phase phase = null;
// add steps for each phase, in reverse
Expand All @@ -185,7 +183,7 @@ public List<Step> toSteps(Client client) {
Phase previousPhase = phaseIterator.previous();

// add `after` step for phase before next
if (phase != null) {
if (previousPhase != null) {
// after step should have the name of the previous phase since the index is still in the
// previous phase until the after condition is reached
Step.StepKey afterStepKey = new Step.StepKey(previousPhase.getName(), PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
Expand All @@ -210,13 +208,11 @@ public List<Step> toSteps(Client client) {
}
}

if (phase != null) {
// The very first after step is in a phase before the hot phase so call this "new"
Step.StepKey afterStepKey = new Step.StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
steps.add(phaseAfterStep);
lastStepKey = phaseAfterStep.getKey();
}
// The very first after step is in a phase before the hot phase so call this "new"
Step.StepKey afterStepKey = new Step.StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
steps.add(phaseAfterStep);
lastStepKey = phaseAfterStep.getKey();

// init step so that policy is guaranteed to have
steps.add(new InitializePolicyContextStep(InitializePolicyContextStep.KEY, lastStepKey));
Expand Down
4 changes: 4 additions & 0 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/PhaseCompleteStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,8 @@ public class PhaseCompleteStep extends Step {
public PhaseCompleteStep(StepKey key, StepKey nextStepKey) {
super(key, nextStepKey);
}

public static PhaseCompleteStep finalStep(String phase) {
return new PhaseCompleteStep(new StepKey(phase, NAME, NAME), null);
}
}
17 changes: 9 additions & 8 deletions x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -247,14 +247,14 @@ public void testFirstAndLastSteps() {
assertThat(steps.size(), equalTo(2));
assertThat(steps.get(0), instanceOf(InitializePolicyContextStep.class));
assertThat(steps.get(0).getKey(), equalTo(new StepKey("new", "init", "init")));
assertThat(steps.get(0).getNextStepKey(), equalTo(TerminalPolicyStep.KEY));
assertSame(steps.get(1), TerminalPolicyStep.INSTANCE);
assertThat(steps.get(0).getNextStepKey(), equalTo(PhaseCompleteStep.finalStep("new").getKey()));
assertThat(steps.get(1), equalTo(PhaseCompleteStep.finalStep("new")));
}

public void testToStepsWithOneStep() {
Client client = mock(Client.class);
MockStep mockStep = new MockStep(
new Step.StepKey("test", "test", "test"), TerminalPolicyStep.KEY);
new Step.StepKey("test", "test", "test"), PhaseCompleteStep.finalStep("test").getKey());

lifecycleName = randomAlphaOfLengthBetween(1, 20);
Map<String, Phase> phases = new LinkedHashMap<>();
Expand All @@ -264,21 +264,22 @@ public void testToStepsWithOneStep() {
phases.put(firstPhase.getName(), firstPhase);
LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
StepKey firstStepKey = InitializePolicyContextStep.KEY;
StepKey secondStepKey = new StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
StepKey secondStepKey = PhaseCompleteStep.finalStep("new").getKey();
List<Step> steps = policy.toSteps(client);
assertThat(steps.size(), equalTo(4));
assertSame(steps.get(0).getKey(), firstStepKey);
assertThat(steps.get(0).getNextStepKey(), equalTo(secondStepKey));
assertThat(steps.get(1).getKey(), equalTo(secondStepKey));
assertThat(steps.get(1).getNextStepKey(), equalTo(mockStep.getKey()));
assertThat(steps.get(2).getKey(), equalTo(mockStep.getKey()));
assertThat(steps.get(2).getNextStepKey(), equalTo(TerminalPolicyStep.KEY));
assertSame(steps.get(3), TerminalPolicyStep.INSTANCE);
assertThat(steps.get(2).getNextStepKey(), equalTo(PhaseCompleteStep.finalStep("test").getKey()));
assertThat(steps.get(3), equalTo(PhaseCompleteStep.finalStep("test")));
}

public void testToStepsWithTwoPhases() {
Client client = mock(Client.class);
MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"), TerminalPolicyStep.KEY);
MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"),
PhaseCompleteStep.finalStep("second_phase").getKey());
MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME),
secondActionStep.getKey());
MockStep firstActionAnotherStep = new MockStep(new StepKey("first_phase", "test", "bar"), secondAfter.getKey());
Expand Down Expand Up @@ -312,7 +313,7 @@ public void testToStepsWithTwoPhases() {
assertThat(steps.get(4).getKey(), equalTo(secondAfter.getKey()));
assertThat(steps.get(4).getNextStepKey(), equalTo(secondAfter.getNextStepKey()));
assertThat(steps.get(5), equalTo(secondActionStep));
assertSame(steps.get(6), TerminalPolicyStep.INSTANCE);
assertThat(steps.get(6), equalTo(PhaseCompleteStep.finalStep("second_phase")));
}

public void testIsActionSafe() {
Expand Down
21 changes: 11 additions & 10 deletions ...n/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,8 @@ public void testCCRUnfollowDuringSnapshot() throws Exception {
.build());

// start snapshot
request = new Request("PUT", "/_snapshot/repo/snapshot");
String snapName = "snapshot-" + randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
request = new Request("PUT", "/_snapshot/repo/" + snapName);
request.addParameter("wait_for_completion", "false");
request.setJsonEntity("{\"indices\": \"" + indexName + "\"}");
assertOK(client().performRequest(request));
Expand All @@ -165,14 +166,14 @@ public void testCCRUnfollowDuringSnapshot() throws Exception {
// Following index should have the document
assertDocumentExists(client(), indexName, "1");
// ILM should have completed the unfollow
assertILMPolicy(client(), indexName, "unfollow-only", "completed");
assertILMPolicy(client(), indexName, "unfollow-only", "hot", "complete", "complete");
}, 2, TimeUnit.MINUTES);

// assert that snapshot succeeded
assertThat(getSnapshotState("snapshot"), equalTo("SUCCESS"));
assertOK(client().performRequest(new Request("DELETE", "/_snapshot/repo/snapshot")));
assertThat(getSnapshotState(snapName), equalTo("SUCCESS"));
assertOK(client().performRequest(new Request("DELETE", "/_snapshot/repo/" + snapName)));
ResponseException e = expectThrows(ResponseException.class,
() -> client().performRequest(new Request("GET", "/_snapshot/repo/snapshot")));
() -> client().performRequest(new Request("GET", "/_snapshot/repo/" + snapName)));
assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(404));
}
} else {
Expand Down Expand Up @@ -344,7 +345,7 @@ public void testAliasReplicatedOnShrink() throws Exception {
assertBusy(() -> assertOK(client().performRequest(new Request("HEAD", "/" + shrunkenIndexName + "/_alias/" + indexName))));

// Wait for the index to complete its policy
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, "completed", "completed", "completed"));
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, null, "complete", "complete"));
}
}

Expand Down Expand Up @@ -391,7 +392,7 @@ public void testUnfollowInjectedBeforeShrink() throws Exception {
assertBusy(() -> assertTrue(indexExists(shrunkenIndexName)));

// Wait for the index to complete its policy
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, "completed", "completed", "completed"));
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, null, "complete", "complete"));
}
}

Expand Down Expand Up @@ -461,8 +462,8 @@ public void testCannotShrinkLeaderIndex() throws Exception {
assertEquals(RestStatus.OK.getStatus(), shrunkenIndexExistsResponse.getStatusLine().getStatusCode());

// And both of these should now finish their policies
assertILMPolicy(leaderClient, shrunkenIndexName, policyName, "completed");
assertILMPolicy(client(), indexName, policyName, "completed");
assertILMPolicy(leaderClient, shrunkenIndexName, policyName, null, "complete", "complete");
assertILMPolicy(client(), indexName, policyName, "hot", "complete", "complete");
});
}
} else {
Expand Down Expand Up @@ -542,7 +543,7 @@ public void testILMUnfollowFailsToRemoveRetentionLeases() throws Exception {
client().performRequest(new Request("POST", "/_ilm/start"));
// Wait for the policy to be complete
assertBusy(() -> {
assertILMPolicy(client(), followerIndex, policyName, "completed", "completed", "completed");
assertILMPolicy(client(), followerIndex, policyName, "hot", "complete", "complete");
});

// Ensure the "follower" index has successfully unfollowed
Expand Down
4 changes: 2 additions & 2 deletions ...n/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/ChangePolicyforIndexIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,9 @@
import org.elasticsearch.xpack.core.ilm.LifecyclePolicy;
import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
import org.elasticsearch.xpack.core.ilm.Phase;
import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep;
import org.elasticsearch.xpack.core.ilm.RolloverAction;
import org.elasticsearch.xpack.core.ilm.Step.StepKey;
import org.elasticsearch.xpack.core.ilm.TerminalPolicyStep;
import org.elasticsearch.xpack.core.ilm.WaitForRolloverReadyStep;

import java.io.IOException;
Expand Down Expand Up @@ -113,7 +113,7 @@ public void testChangePolicyForIndex() throws Exception {
assertOK(client().performRequest(request));

// Check the index goes to the warm phase and completes
assertBusy(() -> assertStep(indexName, TerminalPolicyStep.KEY), 30, TimeUnit.SECONDS);
assertBusy(() -> assertStep(indexName, PhaseCompleteStep.finalStep("warm").getKey()), 30, TimeUnit.SECONDS);

// Check index is allocated on integTest-1 and integTest-2 as per policy_2
Request getSettingsRequest = new Request("GET", "/" + indexName + "/_settings");
Expand Down
Loading

0 comments on commit deefc85

Please sign in to comment.