Skip to content

Commit e978b72

Browse files
andreidanandreidan
authored
ILM: execute cached steps even if policy is updated (#75296)
This makes ILM honour the cached phase even when the underlying policy is updated to not contain the cached actions.
1 parent cf575f4 commit e978b72

File tree

5 files changed

+111
-7
lines changed

5 files changed

+111
-7
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -224,8 +224,8 @@ public static boolean isIndexPhaseDefinitionUpdatable(final NamedXContentRegistr
224224
* information, returns null.
225225
*/
226226
@Nullable
227-
static Set<Step.StepKey> readStepKeys(final NamedXContentRegistry xContentRegistry, final Client client,
228-
final String phaseDef, final String currentPhase, final XPackLicenseState licenseState) {
227+
public static Set<Step.StepKey> readStepKeys(final NamedXContentRegistry xContentRegistry, final Client client,
228+
final String phaseDef, final String currentPhase, final XPackLicenseState licenseState) {
229229
final PhaseExecutionInfo phaseExecutionInfo;
230230
try (XContentParser parser = JsonXContent.jsonXContent.createParser(xContentRegistry,
231231
DeprecationHandler.THROW_UNSUPPORTED_OPERATION, phaseDef)) {

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

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import org.elasticsearch.xpack.core.ilm.Phase;
2424
import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep;
2525
import org.elasticsearch.xpack.core.ilm.RolloverAction;
26+
import org.elasticsearch.xpack.core.ilm.SetPriorityAction;
2627
import org.elasticsearch.xpack.core.ilm.Step.StepKey;
2728
import org.elasticsearch.xpack.core.ilm.WaitForRolloverReadyStep;
2829

@@ -33,6 +34,9 @@
3334

3435
import static java.util.Collections.singletonMap;
3536
import static org.elasticsearch.common.xcontent.XContentFactory.jsonBuilder;
37+
import static org.elasticsearch.xpack.TimeSeriesRestDriver.createIndexWithSettings;
38+
import static org.elasticsearch.xpack.TimeSeriesRestDriver.createNewSingletonPolicy;
39+
import static org.elasticsearch.xpack.TimeSeriesRestDriver.indexDocument;
3640

3741
public class ChangePolicyforIndexIT extends ESRestTestCase {
3842

@@ -124,6 +128,37 @@ public void testChangePolicyForIndex() throws Exception {
124128
assertEquals("javaRestTest-0,javaRestTest-1,javaRestTest-2,javaRestTest-3", includesAllocation);
125129
}
126130

131+
public void testILMHonoursTheCachedPhaseAfterPolicyUpdate() throws Exception {
132+
String indexName = "test-000001";
133+
String policyName = "rolloverPolicy";
134+
String alias = "thealias";
135+
createNewSingletonPolicy(client(), policyName, "hot", new RolloverAction(null, null, null, 1L));
136+
137+
createIndexWithSettings(client(), indexName, alias, Settings.builder()
138+
.put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
139+
.put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0)
140+
.put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, alias)
141+
.put(LifecycleSettings.LIFECYCLE_NAME, policyName));
142+
143+
// Check the index is on the check-rollover-ready step
144+
assertBusy(() -> assertStep(indexName, new StepKey("hot", RolloverAction.NAME, WaitForRolloverReadyStep.NAME)), 30,
145+
TimeUnit.SECONDS);
146+
147+
// update the policy to not contain rollover
148+
createNewSingletonPolicy(client(), policyName, "hot", new SetPriorityAction(200));
149+
150+
// Check the index is on the check-rollover-ready step
151+
assertBusy(() -> assertStep(indexName, new StepKey("hot", RolloverAction.NAME, WaitForRolloverReadyStep.NAME)), 30,
152+
TimeUnit.SECONDS);
153+
154+
indexDocument(client(), indexName, true);
155+
156+
String rolloverIndex = "test-000002";
157+
// let's check the cached rollover action still executed and the rollover index exists
158+
assertBusy(() -> indexExists(rolloverIndex), 30, TimeUnit.SECONDS);
159+
assertBusy(() -> assertStep(indexName, PhaseCompleteStep.finalStep("hot").getKey()), 30, TimeUnit.SECONDS);
160+
}
161+
127162
private void assertStep(String indexName, StepKey expectedStep) throws IOException {
128163
Response explainResponse = client().performRequest(new Request("GET", "/" + indexName + "/_ilm/explain"));
129164
assertOK(explainResponse);

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@
4343
import java.util.List;
4444
import java.util.Objects;
4545
import java.util.Optional;
46+
import java.util.Set;
4647
import java.util.function.BiFunction;
4748
import java.util.function.LongSupplier;
4849

@@ -84,8 +85,13 @@ public static void validateTransition(IndexMetadata idxMeta, Step.StepKey curren
8485
"], currently: [" + realKey + "]");
8586
}
8687

87-
// Always allow moving to the terminal step, even if it doesn't exist in the policy
88-
if (stepRegistry.stepExists(indexPolicySetting, newStepKey) == false && newStepKey.equals(TerminalPolicyStep.KEY) == false) {
88+
final Set<Step.StepKey> cachedStepKeys =
89+
stepRegistry.parseStepKeysFromPhase(lifecycleState.getPhaseDefinition(), lifecycleState.getPhase());
90+
boolean isNewStepCached = cachedStepKeys != null && cachedStepKeys.contains(newStepKey);
91+
92+
// Always allow moving to the terminal step or to a step that's present in the cached phase, even if it doesn't exist in the policy
93+
if (isNewStepCached == false &&
94+
(stepRegistry.stepExists(indexPolicySetting, newStepKey) == false && newStepKey.equals(TerminalPolicyStep.KEY) == false)) {
8995
throw new IllegalArgumentException("step [" + newStepKey + "] for index [" + idxMeta.getIndex().getName() +
9096
"] with policy [" + indexPolicySetting + "] does not exist");
9197
}

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,15 +14,15 @@
1414
import org.elasticsearch.cluster.Diff;
1515
import org.elasticsearch.cluster.DiffableUtils;
1616
import org.elasticsearch.cluster.metadata.IndexMetadata;
17-
import org.elasticsearch.core.Nullable;
1817
import org.elasticsearch.common.io.stream.StreamInput;
1918
import org.elasticsearch.common.io.stream.StreamOutput;
20-
import org.elasticsearch.core.TimeValue;
2119
import org.elasticsearch.common.xcontent.DeprecationHandler;
2220
import org.elasticsearch.common.xcontent.NamedXContentRegistry;
2321
import org.elasticsearch.common.xcontent.XContentParseException;
2422
import org.elasticsearch.common.xcontent.XContentParser;
2523
import org.elasticsearch.common.xcontent.json.JsonXContent;
24+
import org.elasticsearch.core.Nullable;
25+
import org.elasticsearch.core.TimeValue;
2626
import org.elasticsearch.index.Index;
2727
import org.elasticsearch.license.XPackLicenseState;
2828
import org.elasticsearch.xpack.core.ClientHelper;
@@ -34,6 +34,7 @@
3434
import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata;
3535
import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
3636
import org.elasticsearch.xpack.core.ilm.Phase;
37+
import org.elasticsearch.xpack.core.ilm.PhaseCacheManagement;
3738
import org.elasticsearch.xpack.core.ilm.PhaseExecutionInfo;
3839
import org.elasticsearch.xpack.core.ilm.Step;
3940
import org.elasticsearch.xpack.core.ilm.TerminalPolicyStep;
@@ -45,6 +46,7 @@
4546
import java.util.List;
4647
import java.util.Map;
4748
import java.util.Optional;
49+
import java.util.Set;
4850
import java.util.SortedMap;
4951
import java.util.TreeMap;
5052
import java.util.stream.Collectors;
@@ -143,6 +145,15 @@ public LifecyclePolicyMetadata read(StreamInput in, String key) {
143145
}
144146
}
145147

148+
/**
149+
* Parses the step keys from the {@code phaseDef} for the given phase.
150+
* Returns null if there's a parsing error.
151+
*/
152+
@Nullable
153+
public Set<Step.StepKey> parseStepKeysFromPhase(String phaseDef, String currentPhase) {
154+
return PhaseCacheManagement.readStepKeys(xContentRegistry, client, phaseDef, currentPhase, licenseState);
155+
}
156+
146157
private List<Step> parseStepsFromPhase(String policy, String currentPhase, String phaseDef) throws IOException {
147158
final PhaseExecutionInfo phaseExecutionInfo;
148159
LifecyclePolicyMetadata policyMetadata = lifecyclePolicyMap.get(policy);

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

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,8 +39,10 @@
3939
import org.elasticsearch.xpack.core.ilm.Phase;
4040
import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep;
4141
import org.elasticsearch.xpack.core.ilm.RolloverAction;
42+
import org.elasticsearch.xpack.core.ilm.RolloverStep;
4243
import org.elasticsearch.xpack.core.ilm.SetPriorityAction;
4344
import org.elasticsearch.xpack.core.ilm.Step;
45+
import org.elasticsearch.xpack.core.ilm.WaitForRolloverReadyStep;
4446

4547
import java.io.IOException;
4648
import java.util.ArrayList;
@@ -489,11 +491,61 @@ public void testValidateValidTransition() {
489491
try {
490492
IndexLifecycleTransition.validateTransition(indexMetadata, currentStepKey, nextStepKey, policyRegistry);
491493
} catch (Exception e) {
492-
logger.error(e);
494+
logger.error(e.getMessage(), e);
493495
fail("validateTransition should not throw exception on valid transitions");
494496
}
495497
}
496498

499+
public void testValidateTransitionToCachedStepMissingFromPolicy() {
500+
LifecycleExecutionState.Builder executionState = LifecycleExecutionState.builder()
501+
.setPhase("hot")
502+
.setAction("rollover")
503+
.setStep("check-rollover-ready")
504+
.setPhaseDefinition("{\n" +
505+
" \"policy\" : \"my-policy\",\n" +
506+
" \"phase_definition\" : {\n" +
507+
" \"min_age\" : \"20m\",\n" +
508+
" \"actions\" : {\n" +
509+
" \"rollover\" : {\n" +
510+
" \"max_age\" : \"5s\"\n" +
511+
" },\n" +
512+
" \"set_priority\" : {\n" +
513+
" \"priority\" : 150\n" +
514+
" }\n" +
515+
" }\n" +
516+
" },\n" +
517+
" \"version\" : 1,\n" +
518+
" \"modified_date_in_millis\" : 1578521007076\n" +
519+
" }");
520+
521+
IndexMetadata meta = buildIndexMetadata("my-policy", executionState);
522+
523+
Map<String, LifecycleAction> actions = new HashMap<>();
524+
actions.put(SetPriorityAction.NAME, new SetPriorityAction(100));
525+
Phase hotPhase = new Phase("hot", TimeValue.ZERO, actions);
526+
Map<String, Phase> phases = Collections.singletonMap("hot", hotPhase);
527+
LifecyclePolicy policyWithoutRollover = new LifecyclePolicy("my-policy", phases);
528+
LifecyclePolicyMetadata policyMetadata = new LifecyclePolicyMetadata(policyWithoutRollover, Collections.emptyMap(), 2L, 2L);
529+
530+
ClusterState existingState = ClusterState.builder(ClusterState.EMPTY_STATE)
531+
.metadata(Metadata.builder(Metadata.EMPTY_METADATA)
532+
.put(meta, false)
533+
.build())
534+
.build();
535+
try (Client client = new NoOpClient(getTestName())) {
536+
Step.StepKey currentStepKey = new Step.StepKey("hot", RolloverAction.NAME, WaitForRolloverReadyStep.NAME);
537+
Step.StepKey nextStepKey = new Step.StepKey("hot", RolloverAction.NAME, RolloverStep.NAME);
538+
Step currentStep = new WaitForRolloverReadyStep(currentStepKey, nextStepKey, client, null, null, null, 1L);
539+
try {
540+
IndexLifecycleTransition.validateTransition(meta, currentStepKey, nextStepKey, createOneStepPolicyStepRegistry("my-policy",
541+
currentStep));
542+
} catch (Exception e) {
543+
logger.error(e.getMessage(), e);
544+
fail("validateTransition should not throw exception on valid transitions");
545+
}
546+
}
547+
}
548+
497549
public void testMoveClusterStateToFailedStep() {
498550
String indexName = "my_index";
499551
String policyName = "my_policy";

0 commit comments

Comments
 (0)