Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stop policy on last PhaseCompleteStep instead of TerminalPolic… #51631

Merged
merged 1 commit into from
Jan 31, 2020
Merged

Stop policy on last PhaseCompleteStep instead of TerminalPolic… #51631

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 7 additions & 11 deletions x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
19 changes: 10 additions & 9 deletions ...n/ilm/qa/multi-cluster/src/test/java/org/elasticsearch/xpack/ilm/CCRIndexLifecycleIT.java
View file
Open in desktop
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,12 +166,12 @@ 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)));
}
} else {
fail("unexpected target cluster [" + targetCluster + "]");
Expand Down Expand Up @@ -341,7 +342,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 @@ -388,7 +389,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 @@ -458,8 +459,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 @@ -539,7 +540,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
Open in desktop
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