move phase-after steps to have the previous phase's phase in its key (#4387)

Previously, phase X's `after` step had `X` as its associated phase. This causes confusion because
we have only entered phase `X` once the `after` step is complete. Therefore, this refactor
pushes the after's phase to be associated with the previous phase. This first phase is an exception. 
The first phase's `after` step is associated with the first phase (not some non-existent prior phase).
This commit is contained in:
Tal Levy 2018-04-16 21:19:30 -07:00 committed by GitHub
parent d509834c00
commit 689d628d67
3 changed files with 78 additions and 21 deletions

View File

@ -12,6 +12,7 @@ import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.index.Index;
public class InitializePolicyContextStep extends Step {
public static final StepKey KEY = new StepKey("pre-phase", "pre-action", "init");
public InitializePolicyContextStep(Step.StepKey key, StepKey nextStepKey) {
super(key, nextStepKey);

View File

@ -139,6 +139,32 @@ public class LifecyclePolicy extends AbstractDiffable<LifecyclePolicy>
return builder;
}
/**
* This method is used to compile this policy into its execution plan built out
* of {@link Step} instances. The order of the {@link Phase}s and {@link LifecycleAction}s is
* determined by the {@link LifecycleType} associated with this policy.
*
* The order of the policy will have this structure:
*
* - initialize policy context step
* - phase-1 phase-after-step
* - ... phase-1 action steps
* - phase-2 phase-after-step
* - ...
* - terminal policy step
*
* We first initialize the policy's context and ensure that the index has proper settings set.
* Then we begin each phase's after-step along with all its actions as steps. Finally, we have
* a terminal step to inform us that this policy's steps are all complete. Each phase's `after`
* step is associated with the previous phase's phase. For example, the warm phase's `after` is
* associated with the hot phase so that it is clear that we haven't stepped into the warm phase
* just yet (until this step is complete).
*
* @param client The Elasticsearch Client to use during execution of {@link AsyncActionStep}
* and {@link AsyncWaitStep} steps.
* @param nowSupplier The supplier of the current time for {@link PhaseAfterStep} steps.
* @return The list of {@link Step} objects in order of their execution.
*/
public List<Step> toSteps(Client client, LongSupplier nowSupplier) {
List<Step> steps = new ArrayList<>();
List<Phase> orderedPhases = type.getOrderedPhases(phases);
@ -148,9 +174,19 @@ public class LifecyclePolicy extends AbstractDiffable<LifecyclePolicy>
steps.add(TerminalPolicyStep.INSTANCE);
Step.StepKey lastStepKey = TerminalPolicyStep.KEY;
Phase phase = null;
// add steps for each phase, in reverse
while (phaseIterator.hasPrevious()) {
Phase phase = phaseIterator.previous();
// add `after` step for phase before next
if (phase != null) {
Step.StepKey afterStepKey = new Step.StepKey(phase.getName(), "pre-" + lastStepKey.getAction(), "after");
Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey);
steps.add(phaseAfterStep);
lastStepKey = phaseAfterStep.getKey();
}
phase = phaseIterator.previous();
List<LifecycleAction> orderedActions = type.getOrderedActions(phase);
ListIterator<LifecycleAction> actionIterator = orderedActions.listIterator(orderedActions.size());
// add steps for each action, in reverse
@ -164,23 +200,24 @@ public class LifecyclePolicy extends AbstractDiffable<LifecyclePolicy>
lastStepKey = step.getKey();
}
}
}
// add `after` step for phase
Step.StepKey afterStepKey = new Step.StepKey(phase.getName(), "pre-action", "after");
if (phase != null) {
Step.StepKey afterStepKey = new Step.StepKey(phase.getName(), "pre-" + lastStepKey.getAction(), "after");
Step phaseAfterStep = new PhaseAfterStep(nowSupplier, phase.getAfter(), afterStepKey, lastStepKey);
steps.add(phaseAfterStep);
lastStepKey = phaseAfterStep.getKey();
}
// init step so that policy is guaranteed to have
steps.add(new InitializePolicyContextStep(
new Step.StepKey("pre-phase", "pre-action", "init"), lastStepKey));
steps.add(new InitializePolicyContextStep(InitializePolicyContextStep.KEY, lastStepKey));
Collections.reverse(steps);
logger.debug("STEP COUNT: " + steps.size());
for (Step step : steps) {
logger.debug(step.getKey() + " -> " + step.getNextStepKey());
}
return steps;
}

View File

@ -15,6 +15,7 @@ import org.elasticsearch.common.unit.TimeValue;
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
import org.elasticsearch.common.xcontent.XContentParser;
import org.elasticsearch.test.AbstractSerializingTestCase;
import org.elasticsearch.xpack.core.indexlifecycle.Step.StepKey;
import java.io.IOException;
import java.util.ArrayList;
@ -27,6 +28,7 @@ import java.util.Map;
import java.util.function.LongSupplier;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
import static org.mockito.Mockito.mock;
public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecyclePolicy> {
@ -105,38 +107,55 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
assertSame(TimeseriesLifecycleType.INSTANCE, policy.getType());
}
public void testFirstAndLastSteps() {
Client client = mock(Client.class);
LongSupplier nowSupplier = () -> 0L;
lifecycleName = randomAlphaOfLengthBetween(1, 20);
Map<String, Phase> phases = new LinkedHashMap<>();
LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
List<Step> steps = policy.toSteps(client, nowSupplier);
assertThat(steps.size(), equalTo(2));
assertThat(steps.get(0), instanceOf(InitializePolicyContextStep.class));
assertThat(steps.get(0).getKey(), equalTo(new StepKey("pre-phase", "pre-action", "init")));
assertThat(steps.get(0).getNextStepKey(), equalTo(TerminalPolicyStep.KEY));
assertSame(steps.get(1), TerminalPolicyStep.INSTANCE);
}
public void testToStepsWithOneStep() {
Client client = mock(Client.class);
LongSupplier nowSupplier = () -> 0L;
MockStep firstStep = new MockStep(new Step.StepKey("test", "test", "test"), null);
MockStep mockStep = new MockStep(
new Step.StepKey("test", "test", "test"), TerminalPolicyStep.KEY);
lifecycleName = randomAlphaOfLengthBetween(1, 20);
Map<String, Phase> phases = new LinkedHashMap<>();
LifecycleAction firstAction = new MockAction(Arrays.asList(firstStep));
LifecycleAction firstAction = new MockAction(Arrays.asList(mockStep));
Map<String, LifecycleAction> actions = Collections.singletonMap(MockAction.NAME, firstAction);
Phase firstPhase = new Phase("test", TimeValue.ZERO, actions);
phases.put(firstPhase.getName(), firstPhase);
LifecyclePolicy policy = new LifecyclePolicy(TestLifecycleType.INSTANCE, lifecycleName, phases);
StepKey firstStepKey = InitializePolicyContextStep.KEY;
StepKey secondStepKey = new StepKey("test", "pre-test", "after");
List<Step> steps = policy.toSteps(client, nowSupplier);
assertThat(steps.size(), equalTo(4));
assertThat(steps.get(0).getKey(), equalTo(new Step.StepKey("pre-phase", "pre-action", "init")));
assertThat(steps.get(0).getNextStepKey(), equalTo(new Step.StepKey("test", "pre-action", "after")));
assertThat(steps.get(1).getKey(), equalTo(new Step.StepKey("test", "pre-action", "after")));
assertThat(steps.get(1).getNextStepKey(), equalTo(firstStep.getKey()));
assertThat(steps.get(2), equalTo(firstStep));
assertNull(steps.get(2).getNextStepKey());
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);
}
public void testToStepsWithTwoPhases() {
Client client = mock(Client.class);
LongSupplier nowSupplier = () -> 0L;
MockStep secondActionStep = new MockStep(new Step.StepKey("second_phase", "test", "test"), null);
MockStep secondAfter = new MockStep(new Step.StepKey("second_phase", "pre-action", "after"), secondActionStep.getKey());
MockStep firstActionAnotherStep = new MockStep(new Step.StepKey("first_phase", "test", "test"), secondAfter.getKey());
MockStep firstActionStep = new MockStep(new Step.StepKey("first_phase", "test", "test"), firstActionAnotherStep.getKey());
MockStep firstAfter = new MockStep(new Step.StepKey("first_phase", "pre-action", "after"), firstActionStep.getKey());
MockStep init = new MockStep(new Step.StepKey("pre-phase", "pre-action", "init"), firstAfter.getKey());
MockStep secondActionStep = new MockStep(new StepKey("second_phase", "test2", "test"), TerminalPolicyStep.KEY);
MockStep secondAfter = new MockStep(new StepKey("second_phase", "pre-test2", "after"), secondActionStep.getKey());
MockStep firstActionAnotherStep = new MockStep(new StepKey("first_phase", "test", "bar"), secondAfter.getKey());
MockStep firstActionStep = new MockStep(new StepKey("first_phase", "test", "foo"), firstActionAnotherStep.getKey());
MockStep firstAfter = new MockStep(new StepKey("first_phase", "pre-test", "after"), firstActionStep.getKey());
MockStep init = new MockStep(InitializePolicyContextStep.KEY, firstAfter.getKey());
lifecycleName = randomAlphaOfLengthBetween(1, 20);
Map<String, Phase> phases = new LinkedHashMap<>();
@ -164,6 +183,6 @@ public class LifecyclePolicyTests extends AbstractSerializingTestCase<LifecycleP
assertThat(steps.get(4).getKey(), equalTo(secondAfter.getKey()));
assertThat(steps.get(4).getNextStepKey(), equalTo(secondAfter.getNextStepKey()));
assertThat(steps.get(5), equalTo(secondActionStep));
assertThat(steps.get(6).getClass(), equalTo(TerminalPolicyStep.class));
assertSame(steps.get(6), TerminalPolicyStep.INSTANCE);
}
}