Skip to content

Commit ca8ae6e

Browse files
committed
Replace PhaseAfterStep with PhaseCompleteStep
This removes `PhaseAfterStep` in favor of a new `PhaseCompleteStep`. This step in only a marker that the `LifecyclePolicyRunner` needs to halt until the time indicated for entering the next phase. This also fixes a bug where phase times were encapsulated into the policy instead of dynamically adjusting to policy changes. Supersedes elastic#33140, which it replaces
1 parent 30544cb commit ca8ae6e

File tree

18 files changed

+272
-265
lines changed

18 files changed

+272
-265
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/InitializePolicyContextStep.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
import org.elasticsearch.index.Index;
1313

1414
public final class InitializePolicyContextStep extends ClusterStateActionStep {
15-
public static final StepKey KEY = new StepKey("new", "init", "init");
15+
public static final String INITIALIZATION_PHASE = "new";
16+
public static final StepKey KEY = new StepKey(INITIALIZATION_PHASE, "init", "init");
1617

1718
public InitializePolicyContextStep(Step.StepKey key, StepKey nextStepKey) {
1819
super(key, nextStepKey);

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

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@
2828
import java.util.Map;
2929
import java.util.Objects;
3030
import java.util.function.Function;
31-
import java.util.function.LongSupplier;
3231
import java.util.stream.Collectors;
3332

3433
/**
@@ -165,10 +164,9 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
165164
*
166165
* @param client The Elasticsearch Client to use during execution of {@link AsyncActionStep}
167166
* and {@link AsyncWaitStep} steps.
168-
* @param nowSupplier The supplier of the current time for {@link PhaseAfterStep} steps.
169167
* @return The list of {@link Step} objects in order of their execution.
170168
*/
171-
public List<Step> toSteps(Client client, LongSupplier nowSupplier) {
169+
public List<Step> toSteps(Client client) {
172170
List<Step> steps = new ArrayList<>();
173171
List<Phase> orderedPhases = type.getOrderedPhases(phases);
174172
ListIterator<Phase> phaseIterator = orderedPhases.listIterator(orderedPhases.size());
@@ -187,8 +185,8 @@ public List<Step> toSteps(Client client, LongSupplier nowSupplier) {
187185
if (phase != null) {
188186
// after step should have the name of the previous phase since the index is still in the
189187
// previous phase until the after condition is reached
190-
Step.StepKey afterStepKey = new Step.StepKey(previousPhase.getName(), PhaseAfterStep.NAME, PhaseAfterStep.NAME);
191-
Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey);
188+
Step.StepKey afterStepKey = new Step.StepKey(previousPhase.getName(), PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
189+
Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
192190
steps.add(phaseAfterStep);
193191
lastStepKey = phaseAfterStep.getKey();
194192
}
@@ -211,8 +209,8 @@ public List<Step> toSteps(Client client, LongSupplier nowSupplier) {
211209

212210
if (phase != null) {
213211
// The very first after step is in a phase before the hot phase so call this "new"
214-
Step.StepKey afterStepKey = new Step.StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME);
215-
Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey);
212+
Step.StepKey afterStepKey = new Step.StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
213+
Step phaseAfterStep = new PhaseCompleteStep(afterStepKey, lastStepKey);
216214
steps.add(phaseAfterStep);
217215
lastStepKey = phaseAfterStep.getKey();
218216
}
@@ -289,7 +287,7 @@ private StepKey getNextAfterStep(String currentPhaseName) {
289287
// there are no more steps we should execute
290288
return TerminalPolicyStep.KEY;
291289
} else {
292-
return new StepKey(currentPhaseName, PhaseAfterStep.NAME, PhaseAfterStep.NAME);
290+
return new StepKey(currentPhaseName, PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
293291
}
294292
}
295293

@@ -306,7 +304,7 @@ private StepKey getAfterStepBeforePhase(String currentPhaseName) {
306304
// InitializePolicyContextStep
307305
return InitializePolicyContextStep.KEY;
308306
}
309-
return new StepKey(prevPhaseName, PhaseAfterStep.NAME, PhaseAfterStep.NAME);
307+
return new StepKey(prevPhaseName, PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
310308
}
311309
}
312310

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseAfterStep.java

Lines changed: 0 additions & 60 deletions
This file was deleted.
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
/*
2+
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
3+
* or more contributor license agreements. Licensed under the Elastic License;
4+
* you may not use this file except in compliance with the Elastic License.
5+
*/
6+
package org.elasticsearch.xpack.core.indexlifecycle;
7+
8+
/**
9+
* This is essentially a marker that a phase has ended, and we need to check
10+
* the age of an index before proceeding to the next phase.
11+
*/
12+
public class PhaseCompleteStep extends Step {
13+
public static final String NAME = "complete";
14+
15+
public PhaseCompleteStep(StepKey key, StepKey nextStepKey) {
16+
super(key, nextStepKey);
17+
}
18+
}

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/TerminalPolicyStep.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,8 @@
66
package org.elasticsearch.xpack.core.indexlifecycle;
77

88
public class TerminalPolicyStep extends Step {
9-
public static final StepKey KEY = new StepKey("completed", "completed", "completed");
9+
public static final String TERMINAL_PHASE = "completed";
10+
public static final StepKey KEY = new StepKey(TERMINAL_PHASE, "completed", "completed");
1011
public static final TerminalPolicyStep INSTANCE = new TerminalPolicyStep(KEY, null);
1112

1213
TerminalPolicyStep(StepKey key, StepKey nextStepKey) {

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

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import java.util.Objects;
3131
import java.util.Set;
3232
import java.util.function.Function;
33-
import java.util.function.LongSupplier;
3433
import java.util.stream.Collectors;
3534

3635
import static org.hamcrest.Matchers.equalTo;
@@ -170,11 +169,10 @@ protected Reader<LifecyclePolicy> instanceReader() {
170169

171170
public void testFirstAndLastSteps() {
172171
Client client = mock(Client.class);
173-
LongSupplier nowSupplier = () -> 0L;
174172
lifecycleName = randomAlphaOfLengthBetween(1, 20);
175173
Map<String, Phase> phases = new LinkedHashMap<>();
176174
LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
177-
List<Step> steps = policy.toSteps(client, nowSupplier);
175+
List<Step> steps = policy.toSteps(client);
178176
assertThat(steps.size(), equalTo(2));
179177
assertThat(steps.get(0), instanceOf(InitializePolicyContextStep.class));
180178
assertThat(steps.get(0).getKey(), equalTo(new StepKey("new", "init", "init")));
@@ -184,7 +182,6 @@ public void testFirstAndLastSteps() {
184182

185183
public void testToStepsWithOneStep() {
186184
Client client = mock(Client.class);
187-
LongSupplier nowSupplier = () -> 0L;
188185
MockStep mockStep = new MockStep(
189186
new Step.StepKey("test", "test", "test"), TerminalPolicyStep.KEY);
190187

@@ -196,8 +193,8 @@ public void testToStepsWithOneStep() {
196193
phases.put(firstPhase.getName(), firstPhase);
197194
LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
198195
StepKey firstStepKey = InitializePolicyContextStep.KEY;
199-
StepKey secondStepKey = new StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME);
200-
List<Step> steps = policy.toSteps(client, nowSupplier);
196+
StepKey secondStepKey = new StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME);
197+
List<Step> steps = policy.toSteps(client);
201198
assertThat(steps.size(), equalTo(4));
202199
assertSame(steps.get(0).getKey(), firstStepKey);
203200
assertThat(steps.get(0).getNextStepKey(), equalTo(secondStepKey));
@@ -210,13 +207,12 @@ public void testToStepsWithOneStep() {
210207

211208
public void testToStepsWithTwoPhases() {
212209
Client client = mock(Client.class);
213-
LongSupplier nowSupplier = () -> 0L;
214210
MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"), TerminalPolicyStep.KEY);
215-
MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseAfterStep.NAME, PhaseAfterStep.NAME),
211+
MockStep secondAfter = new MockStep(new StepKey("first_phase", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME),
216212
secondActionStep.getKey());
217213
MockStep firstActionAnotherStep = new MockStep(new StepKey("first_phase", "test", "bar"), secondAfter.getKey());
218214
MockStep firstActionStep = new MockStep(new StepKey("first_phase", "test", "foo"), firstActionAnotherStep.getKey());
219-
MockStep firstAfter = new MockStep(new StepKey("new", PhaseAfterStep.NAME, PhaseAfterStep.NAME), firstActionStep.getKey());
215+
MockStep firstAfter = new MockStep(new StepKey("new", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), firstActionStep.getKey());
220216
MockStep init = new MockStep(InitializePolicyContextStep.KEY, firstAfter.getKey());
221217

222218
lifecycleName = randomAlphaOfLengthBetween(1, 20);
@@ -231,17 +227,17 @@ public void testToStepsWithTwoPhases() {
231227
phases.put(secondPhase.getName(), secondPhase);
232228
LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
233229

234-
List<Step> steps = policy.toSteps(client, nowSupplier);
230+
List<Step> steps = policy.toSteps(client);
235231
assertThat(steps.size(), equalTo(7));
236232
assertThat(steps.get(0).getClass(), equalTo(InitializePolicyContextStep.class));
237233
assertThat(steps.get(0).getKey(), equalTo(init.getKey()));
238234
assertThat(steps.get(0).getNextStepKey(), equalTo(init.getNextStepKey()));
239-
assertThat(steps.get(1).getClass(), equalTo(PhaseAfterStep.class));
235+
assertThat(steps.get(1).getClass(), equalTo(PhaseCompleteStep.class));
240236
assertThat(steps.get(1).getKey(), equalTo(firstAfter.getKey()));
241237
assertThat(steps.get(1).getNextStepKey(), equalTo(firstAfter.getNextStepKey()));
242238
assertThat(steps.get(2), equalTo(firstActionStep));
243239
assertThat(steps.get(3), equalTo(firstActionAnotherStep));
244-
assertThat(steps.get(4).getClass(), equalTo(PhaseAfterStep.class));
240+
assertThat(steps.get(4).getClass(), equalTo(PhaseCompleteStep.class));
245241
assertThat(steps.get(4).getKey(), equalTo(secondAfter.getKey()));
246242
assertThat(steps.get(4).getNextStepKey(), equalTo(secondAfter.getNextStepKey()));
247243
assertThat(steps.get(5), equalTo(secondActionStep));
@@ -337,7 +333,7 @@ public void testGetNextValidStep() {
337333
currentStep = new StepKey("phase_1", "action_3", "step_missing");
338334
nextStep = policy.getNextValidStep(currentStep);
339335
assertNotNull(nextStep);
340-
assertEquals(new StepKey("phase_1", PhaseAfterStep.NAME, PhaseAfterStep.NAME), nextStep);
336+
assertEquals(new StepKey("phase_1", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), nextStep);
341337

342338
// current action exists but step does not and action is last in the
343339
// last phase
@@ -356,7 +352,7 @@ public void testGetNextValidStep() {
356352
currentStep = new StepKey("phase_1", "action_4", "step_2");
357353
nextStep = policy.getNextValidStep(currentStep);
358354
assertNotNull(nextStep);
359-
assertEquals(new StepKey("phase_1", PhaseAfterStep.NAME, PhaseAfterStep.NAME), nextStep);
355+
assertEquals(new StepKey("phase_1", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), nextStep);
360356

361357
// current action no longer exists and action was last in the last phase
362358
currentStep = new StepKey("phase_4", "action_4", "step_2");
@@ -368,7 +364,7 @@ public void testGetNextValidStep() {
368364
currentStep = new StepKey("phase_3", "action_2", "step_2");
369365
nextStep = policy.getNextValidStep(currentStep);
370366
assertNotNull(nextStep);
371-
assertEquals(new StepKey("phase_2", PhaseAfterStep.NAME, PhaseAfterStep.NAME), nextStep);
367+
assertEquals(new StepKey("phase_2", PhaseCompleteStep.NAME, PhaseCompleteStep.NAME), nextStep);
372368

373369
// current phase no longer exists and was last phase
374370
currentStep = new StepKey("phase_5", "action_2", "step_2");

x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/PhaseAfterStepTests.java

Lines changed: 0 additions & 103 deletions
This file was deleted.

0 commit comments

Comments
 (0)