Skip to content

Commit 3adf3c7

Browse files
dakronedakrone
committed
Stop policy on last PhaseCompleteStep instead of TerminalPolicyStep
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
1 parent f9ba80a commit 3adf3c7

File tree

11 files changed

+156
-144
lines changed

11 files changed

+156
-144
lines changed

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java

+7-11
Original file line numberDiff line numberDiff line change
@@ -174,9 +174,7 @@ public List<Step> toSteps(Client client) {
174174
List<Phase> orderedPhases = type.getOrderedPhases(phases);
175175
ListIterator<Phase> phaseIterator = orderedPhases.listIterator(orderedPhases.size());
176176

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

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

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

213-
if (phase != null) {
214-
// The very first after step is in a phase before the hot phase so call this "new"
215-
Step.StepKey afterStepKey = new Step.StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
216-
Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
217-
steps.add(phaseAfterStep);
218-
lastStepKey = phaseAfterStep.getKey();
219-
}
211+
// The very first after step is in a phase before the hot phase so call this "new"
212+
Step.StepKey afterStepKey = new Step.StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
213+
Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
214+
steps.add(phaseAfterStep);
215+
lastStepKey = phaseAfterStep.getKey();
220216

221217
// init step so that policy is guaranteed to have
222218
steps.add(new InitializePolicyContextStep(InitializePolicyContextStep.KEY, lastStepKey));

‎x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/PhaseCompleteStep.java

+4
Original file line numberDiff line numberDiff line change
@@ -15,4 +15,8 @@ public class PhaseCompleteStep extends Step {
1515
public PhaseCompleteStep(StepKey key, StepKey nextStepKey) {
1616
super(key, nextStepKey);
1717
}
18+
19+
public static PhaseCompleteStep finalStep(String phase) {
20+
return new PhaseCompleteStep(new StepKey(phase, NAME, NAME), null);
21+
}
1822
}

‎x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicyTests.java

+9-8
Original file line numberDiff line numberDiff line change
@@ -247,14 +247,14 @@ public void testFirstAndLastSteps() {
247247
assertThat(steps.size(), equalTo(2));
248248
assertThat(steps.get(0), instanceOf(InitializePolicyContextStep.class));
249249
assertThat(steps.get(0).getKey(), equalTo(new StepKey("new", "init", "init")));
250-
assertThat(steps.get(0).getNextStepKey(), equalTo(TerminalPolicyStep.KEY));
251-
assertSame(steps.get(1), TerminalPolicyStep.INSTANCE);
250+
assertThat(steps.get(0).getNextStepKey(), equalTo(PhaseCompleteStep.finalStep("new").getKey()));
251+
assertThat(steps.get(1), equalTo(PhaseCompleteStep.finalStep("new")));
252252
}
253253

254254
public void testToStepsWithOneStep() {
255255
Client client = mock(Client.class);
256256
MockStep mockStep = new MockStep(
257-
new Step.StepKey("test", "test", "test"), TerminalPolicyStep.KEY);
257+
new Step.StepKey("test", "test", "test"), PhaseCompleteStep.finalStep("test").getKey());
258258

259259
lifecycleName = randomAlphaOfLengthBetween(1, 20);
260260
Map<String, Phase> phases = new LinkedHashMap<>();
@@ -264,21 +264,22 @@ public void testToStepsWithOneStep() {
264264
phases.put(firstPhase.getName(), firstPhase);
265265
LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
266266
StepKey firstStepKey = InitializePolicyContextStep.KEY;
267-
StepKey secondStepKey = new StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
267+
StepKey secondStepKey = PhaseCompleteStep.finalStep("new").getKey();
268268
List<Step> steps = policy.toSteps(client);
269269
assertThat(steps.size(), equalTo(4));
270270
assertSame(steps.get(0).getKey(), firstStepKey);
271271
assertThat(steps.get(0).getNextStepKey(), equalTo(secondStepKey));
272272
assertThat(steps.get(1).getKey(), equalTo(secondStepKey));
273273
assertThat(steps.get(1).getNextStepKey(), equalTo(mockStep.getKey()));
274274
assertThat(steps.get(2).getKey(), equalTo(mockStep.getKey()));
275-
assertThat(steps.get(2).getNextStepKey(), equalTo(TerminalPolicyStep.KEY));
276-
assertSame(steps.get(3), TerminalPolicyStep.INSTANCE);
275+
assertThat(steps.get(2).getNextStepKey(), equalTo(PhaseCompleteStep.finalStep("test").getKey()));
276+
assertThat(steps.get(3), equalTo(PhaseCompleteStep.finalStep("test")));
277277
}
278278

279279
public void testToStepsWithTwoPhases() {
280280
Client client = mock(Client.class);
281-
MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"), TerminalPolicyStep.KEY);
281+
MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"),
282+
PhaseCompleteStep.finalStep("second_phase").getKey());
282283
MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME),
283284
secondActionStep.getKey());
284285
MockStep firstActionAnotherStep = new MockStep(new StepKey("first_phase", "test", "bar"), secondAfter.getKey());
@@ -312,7 +313,7 @@ public void testToStepsWithTwoPhases() {
312313
assertThat(steps.get(4).getKey(), equalTo(secondAfter.getKey()));
313314
assertThat(steps.get(4).getNextStepKey(), equalTo(secondAfter.getNextStepKey()));
314315
assertThat(steps.get(5), equalTo(secondActionStep));
315-
assertSame(steps.get(6), TerminalPolicyStep.INSTANCE);
316+
assertThat(steps.get(6), equalTo(PhaseCompleteStep.finalStep("second_phase")));
316317
}
317318

318319
public void testIsActionSafe() {

‎x-pack/plugin/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java

+10-9
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,8 @@ public void testCCRUnfollowDuringSnapshot() throws Exception {
146146
.build());
147147

148148
// start snapshot
149-
request = new Request("PUT", "/_snapshot/repo/snapshot");
149+
String snapName = "snapshot-" + randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
150+
request = new Request("PUT", "/_snapshot/repo/" + snapName);
150151
request.addParameter("wait_for_completion", "false");
151152
request.setJsonEntity("{\"indices\": \"" + indexName + "\"}");
152153
assertOK(client().performRequest(request));
@@ -165,12 +166,12 @@ public void testCCRUnfollowDuringSnapshot() throws Exception {
165166
// Following index should have the document
166167
assertDocumentExists(client(), indexName, "1");
167168
// ILM should have completed the unfollow
168-
assertILMPolicy(client(), indexName, "unfollow-only", "completed");
169+
assertILMPolicy(client(), indexName, "unfollow-only", "hot", "complete", "complete");
169170
}, 2, TimeUnit.MINUTES);
170171

171172
// assert that snapshot succeeded
172-
assertThat(getSnapshotState("snapshot"), equalTo("SUCCESS"));
173-
assertOK(client().performRequest(new Request("DELETE", "/_snapshot/repo/snapshot")));
173+
assertThat(getSnapshotState(snapName), equalTo("SUCCESS"));
174+
assertOK(client().performRequest(new Request("DELETE", "/_snapshot/repo/" + snapName)));
174175
}
175176
} else {
176177
fail("unexpected target cluster [" + targetCluster + "]");
@@ -341,7 +342,7 @@ public void testAliasReplicatedOnShrink() throws Exception {
341342
assertBusy(() -> assertOK(client().performRequest(new Request("HEAD", "/" + shrunkenIndexName + "/_alias/" + indexName))));
342343

343344
// Wait for the index to complete its policy
344-
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, "completed", "completed", "completed"));
345+
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, null, "complete", "complete"));
345346
}
346347
}
347348

@@ -388,7 +389,7 @@ public void testUnfollowInjectedBeforeShrink() throws Exception {
388389
assertBusy(() -> assertTrue(indexExists(shrunkenIndexName)));
389390

390391
// Wait for the index to complete its policy
391-
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, "completed", "completed", "completed"));
392+
assertBusy(() -> assertILMPolicy(client(), shrunkenIndexName, policyName, null, "complete", "complete"));
392393
}
393394
}
394395

@@ -458,8 +459,8 @@ public void testCannotShrinkLeaderIndex() throws Exception {
458459
assertEquals(RestStatus.OK.getStatus(), shrunkenIndexExistsResponse.getStatusLine().getStatusCode());
459460

460461
// And both of these should now finish their policies
461-
assertILMPolicy(leaderClient, shrunkenIndexName, policyName, "completed");
462-
assertILMPolicy(client(), indexName, policyName, "completed");
462+
assertILMPolicy(leaderClient, shrunkenIndexName, policyName, null, "complete", "complete");
463+
assertILMPolicy(client(), indexName, policyName, "hot", "complete", "complete");
463464
});
464465
}
465466
} else {
@@ -539,7 +540,7 @@ public void testILMUnfollowFailsToRemoveRetentionLeases() throws Exception {
539540
client().performRequest(new Request("POST", "/_ilm/start"));
540541
// Wait for the policy to be complete
541542
assertBusy(() -> {
542-
assertILMPolicy(client(), followerIndex, policyName, "completed", "completed", "completed");
543+
assertILMPolicy(client(), followerIndex, policyName, "hot", "complete", "complete");
543544
});
544545

545546
// Ensure the "follower" index has successfully unfollowed

‎x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/ChangePolicyforIndexIT.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,9 @@
2020
import org.elasticsearch.xpack.core.ilm.LifecyclePolicy;
2121
import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
2222
import org.elasticsearch.xpack.core.ilm.Phase;
23+
import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep;
2324
import org.elasticsearch.xpack.core.ilm.RolloverAction;
2425
import org.elasticsearch.xpack.core.ilm.Step.StepKey;
25-
import org.elasticsearch.xpack.core.ilm.TerminalPolicyStep;
2626
import org.elasticsearch.xpack.core.ilm.WaitForRolloverReadyStep;
2727

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

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

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

0 commit comments

Comments
 (0)