From 63f377cf618c50deed79ed9da2ffeca2c984485c Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Fri, 7 Feb 2020 19:32:38 +0000 Subject: [PATCH 01/10] ILM fix the init step to actually be retryable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We marked the `init` ILM step as retryable but our test used `waitUntil` without an assert so we didn’t catch the fact that we were not actually able to retry this step as our ILM state didn’t contain any information about the policy execution (as we were in the process of initialising it). This commit manually sets the current step to `init` when we’re moving the ilm policy into the ERROR step (this enables us to successfully move to the error step and later retry the step) --- .../core/ilm/InitializePolicyContextStep.java | 18 +++++++++++------- .../core/ilm/InitializePolicyException.java | 13 +++++++++++++ .../ilm/TimeSeriesLifecycleActionsIT.java | 9 +++++---- .../xpack/ilm/IndexLifecycleTransition.java | 10 ++++++++-- 4 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java index 194dcaa2e1eb8..f3766a3434604 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java @@ -47,13 +47,17 @@ public ClusterState performAction(Index index, ClusterState clusterState) { IndexMetaData.Builder indexMetadataBuilder = IndexMetaData.builder(indexMetaData); if (shouldParseIndexName(indexMetaData.getSettings())) { - long parsedOriginationDate = parseIndexNameAndExtractDate(index.getName()); - indexMetadataBuilder.settingsVersion(indexMetaData.getSettingsVersion() + 1) - .settings(Settings.builder() - .put(indexMetaData.getSettings()) - .put(LifecycleSettings.LIFECYCLE_ORIGINATION_DATE, parsedOriginationDate) - .build() - ); + try { + long parsedOriginationDate = parseIndexNameAndExtractDate(index.getName()); + indexMetadataBuilder.settingsVersion(indexMetaData.getSettingsVersion() + 1) + .settings(Settings.builder() + .put(indexMetaData.getSettings()) + .put(LifecycleSettings.LIFECYCLE_ORIGINATION_DATE, parsedOriginationDate) + .build() + ); + } catch (Exception e) { + throw new InitializePolicyException(e.getMessage(), e); + } } ClusterState.Builder newClusterStateBuilder = ClusterState.builder(clusterState); diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java new file mode 100644 index 0000000000000..f62ffd034760e --- /dev/null +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java @@ -0,0 +1,13 @@ +package org.elasticsearch.xpack.core.ilm; + +import org.elasticsearch.ElasticsearchException; + +/** + * Exception thrown when a problem is encountered while initialising an ILM policy for an index. + */ +public class InitializePolicyException extends ElasticsearchException { + + public InitializePolicyException(String msg, Throwable cause, Object... args) { + super(msg, cause, args); + } +} diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index 667104bc61bb4..f8503d7e3f22f 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -1376,16 +1376,17 @@ public void testRetryableInitializationStep() throws Exception { assertOK(client().performRequest(startReq)); // Wait until an error has occurred. - waitUntil(() -> { + assertThat("ILM did not start retrying the init step", waitUntil(() -> { try { Map explainIndexResponse = explainIndex(index); - String step = (String) explainIndexResponse.get("step"); + String failedStep = (String) explainIndexResponse.get("failed_step"); Integer retryCount = (Integer) explainIndexResponse.get(FAILED_STEP_RETRY_COUNT_FIELD); - return step != null && step.equals(InitializePolicyContextStep.KEY.getAction()) && retryCount != null && retryCount >= 1; + return failedStep != null && failedStep.equals(InitializePolicyContextStep.KEY.getAction()) && retryCount != null + && retryCount >= 1; } catch (IOException e) { return false; } - }, 30, TimeUnit.SECONDS); + }, 30, TimeUnit.SECONDS), is(true)); // Turn origination date parsing back off updateIndexSettings(index, Settings.builder() diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java index 6b38515b10e0a..d1b9441f7fcff 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java @@ -24,6 +24,7 @@ import org.elasticsearch.xpack.core.ilm.ErrorStep; import org.elasticsearch.xpack.core.ilm.IndexLifecycleMetadata; import org.elasticsearch.xpack.core.ilm.InitializePolicyContextStep; +import org.elasticsearch.xpack.core.ilm.InitializePolicyException; import org.elasticsearch.xpack.core.ilm.LifecycleExecutionState; import org.elasticsearch.xpack.core.ilm.LifecyclePolicyMetadata; import org.elasticsearch.xpack.core.ilm.LifecycleSettings; @@ -133,8 +134,13 @@ static ClusterState moveClusterStateToErrorStep(Index index, ClusterState cluste ElasticsearchException.generateThrowableXContent(causeXContentBuilder, STACKTRACE_PARAMS, cause); causeXContentBuilder.endObject(); LifecycleExecutionState currentState = LifecycleExecutionState.fromIndexMetadata(idxMeta); - Step.StepKey currentStep = Objects.requireNonNull(LifecycleExecutionState.getCurrentStepKey(currentState), - "unable to move to an error step where there is no current step, state: " + currentState); + Step.StepKey currentStep; + if (cause instanceof InitializePolicyException) { + currentStep = InitializePolicyContextStep.KEY; + } else { + currentStep = Objects.requireNonNull(LifecycleExecutionState.getCurrentStepKey(currentState), + "unable to move to an error step where there is no current step, state: " + currentState); + } LifecycleExecutionState nextStepState = updateExecutionStateToStep(policyMetadata, currentState, new Step.StepKey(currentStep.getPhase(), currentStep.getAction(), ErrorStep.NAME), nowSupplier, false); From af73584ccd13ce27390374c884ae9f7a1a469b78 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Fri, 7 Feb 2020 19:34:04 +0000 Subject: [PATCH 02/10] ILM assert on the results of `waitUntil` --- .../ilm/TimeSeriesLifecycleActionsIT.java | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index f8503d7e3f22f..9af96d25f72a2 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -1120,26 +1120,26 @@ public void testRolloverStepRetriesUntilRolledOverIndexIsDeleted() throws Except // {@link org.elasticsearch.xpack.core.ilm.ErrorStep} in order to retry the failing step. As {@link #assertBusy} // increases the wait time between calls exponentially, we might miss the window where the policy is on // {@link WaitForRolloverReadyStep} and the move to `attempt-rollover` request will not be successful. - waitUntil(() -> { + assertThat(waitUntil(() -> { try { return client().performRequest(moveToStepRequest).getStatusLine().getStatusCode() == 200; } catch (IOException e) { return false; } - }, 30, TimeUnit.SECONDS); + }, 30, TimeUnit.SECONDS), is(true)); // Similar to above, using {@link #waitUntil} as we want to make sure the `attempt-rollover` step started failing and is being // retried (which means ILM moves back and forth between the `attempt-rollover` step and the `error` step) - waitUntil(() -> { + assertThat("ILM did not start retrying the attempt-rollover step", waitUntil(() -> { try { Map explainIndexResponse = explainIndex(index); - String step = (String) explainIndexResponse.get("step"); + String failedStep = (String) explainIndexResponse.get("failed_step"); Integer retryCount = (Integer) explainIndexResponse.get(FAILED_STEP_RETRY_COUNT_FIELD); - return step != null && step.equals("attempt-rollover") && retryCount != null && retryCount >= 1; + return failedStep != null && failedStep.equals("attempt-rollover") && retryCount != null && retryCount >= 1; } catch (IOException e) { return false; } - }, 30, TimeUnit.SECONDS); + }, 30, TimeUnit.SECONDS), is(true)); deleteIndex(rolledIndex); @@ -1181,16 +1181,17 @@ public void testUpdateRolloverLifecycleDateStepRetriesWhenRolloverInfoIsMissing( "}"); client().performRequest(moveToStepRequest); - waitUntil(() -> { + assertThat("ILM did not start retrying the update-rollover-lifecycle-date step", waitUntil(() -> { try { Map explainIndexResponse = explainIndex(index); - String step = (String) explainIndexResponse.get("step"); + String failedStep = (String) explainIndexResponse.get("failed_step"); Integer retryCount = (Integer) explainIndexResponse.get(FAILED_STEP_RETRY_COUNT_FIELD); - return step != null && step.equals(UpdateRolloverLifecycleDateStep.NAME) && retryCount != null && retryCount >= 1; + return failedStep != null && failedStep.equals(UpdateRolloverLifecycleDateStep.NAME) && retryCount != null + && retryCount >= 1; } catch (IOException e) { return false; } - }); + }, 30, TimeUnit.SECONDS), is(true)); index(client(), index, "1", "foo", "bar"); Request refreshIndex = new Request("POST", "/" + index + "/_refresh"); From 6e46ae3519fad62717c1146b4386ec707337e66d Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Fri, 7 Feb 2020 19:46:10 +0000 Subject: [PATCH 03/10] Add license header --- .../xpack/core/ilm/InitializePolicyException.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java index f62ffd034760e..0023931b3f8a4 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java @@ -1,3 +1,8 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ package org.elasticsearch.xpack.core.ilm; import org.elasticsearch.ElasticsearchException; From 26a18af8a740f49c0f419058940a33528d38a5af Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Mon, 10 Feb 2020 10:34:32 +0000 Subject: [PATCH 04/10] Use assertTrue --- .../xpack/ilm/TimeSeriesLifecycleActionsIT.java | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java index 9af96d25f72a2..8282d95de543e 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/ilm/TimeSeriesLifecycleActionsIT.java @@ -1120,17 +1120,17 @@ public void testRolloverStepRetriesUntilRolledOverIndexIsDeleted() throws Except // {@link org.elasticsearch.xpack.core.ilm.ErrorStep} in order to retry the failing step. As {@link #assertBusy} // increases the wait time between calls exponentially, we might miss the window where the policy is on // {@link WaitForRolloverReadyStep} and the move to `attempt-rollover` request will not be successful. - assertThat(waitUntil(() -> { + assertTrue(waitUntil(() -> { try { return client().performRequest(moveToStepRequest).getStatusLine().getStatusCode() == 200; } catch (IOException e) { return false; } - }, 30, TimeUnit.SECONDS), is(true)); + }, 30, TimeUnit.SECONDS)); // Similar to above, using {@link #waitUntil} as we want to make sure the `attempt-rollover` step started failing and is being // retried (which means ILM moves back and forth between the `attempt-rollover` step and the `error` step) - assertThat("ILM did not start retrying the attempt-rollover step", waitUntil(() -> { + assertTrue("ILM did not start retrying the attempt-rollover step", waitUntil(() -> { try { Map explainIndexResponse = explainIndex(index); String failedStep = (String) explainIndexResponse.get("failed_step"); @@ -1139,7 +1139,7 @@ public void testRolloverStepRetriesUntilRolledOverIndexIsDeleted() throws Except } catch (IOException e) { return false; } - }, 30, TimeUnit.SECONDS), is(true)); + }, 30, TimeUnit.SECONDS)); deleteIndex(rolledIndex); @@ -1181,7 +1181,7 @@ public void testUpdateRolloverLifecycleDateStepRetriesWhenRolloverInfoIsMissing( "}"); client().performRequest(moveToStepRequest); - assertThat("ILM did not start retrying the update-rollover-lifecycle-date step", waitUntil(() -> { + assertTrue("ILM did not start retrying the update-rollover-lifecycle-date step", waitUntil(() -> { try { Map explainIndexResponse = explainIndex(index); String failedStep = (String) explainIndexResponse.get("failed_step"); @@ -1191,7 +1191,7 @@ public void testUpdateRolloverLifecycleDateStepRetriesWhenRolloverInfoIsMissing( } catch (IOException e) { return false; } - }, 30, TimeUnit.SECONDS), is(true)); + }, 30, TimeUnit.SECONDS)); index(client(), index, "1", "foo", "bar"); Request refreshIndex = new Request("POST", "/" + index + "/_refresh"); @@ -1377,7 +1377,7 @@ public void testRetryableInitializationStep() throws Exception { assertOK(client().performRequest(startReq)); // Wait until an error has occurred. - assertThat("ILM did not start retrying the init step", waitUntil(() -> { + assertTrue("ILM did not start retrying the init step", waitUntil(() -> { try { Map explainIndexResponse = explainIndex(index); String failedStep = (String) explainIndexResponse.get("failed_step"); @@ -1387,7 +1387,7 @@ public void testRetryableInitializationStep() throws Exception { } catch (IOException e) { return false; } - }, 30, TimeUnit.SECONDS), is(true)); + }, 30, TimeUnit.SECONDS)); // Turn origination date parsing back off updateIndexSettings(index, Settings.builder() From 27708dd32e0184e1e2a6ade8b2c7650bd2bdf99f Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 12 Feb 2020 12:24:54 +0000 Subject: [PATCH 05/10] Add policy and index in InitalizePolicyExeeption --- .../core/ilm/InitializePolicyContextStep.java | 46 ++++++++++++++----- .../core/ilm/InitializePolicyException.java | 6 ++- .../xpack/core/ilm/LifecyclePolicy.java | 2 +- .../ilm/InitializePolicyContextStepTests.java | 31 +++++++------ 4 files changed, 56 insertions(+), 29 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java index f3766a3434604..b69541ab580ca 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java @@ -13,6 +13,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.Index; +import java.util.Objects; + import static org.elasticsearch.xpack.core.ilm.IndexLifecycleOriginationDateParser.parseIndexNameAndExtractDate; import static org.elasticsearch.xpack.core.ilm.IndexLifecycleOriginationDateParser.shouldParseIndexName; import static org.elasticsearch.xpack.core.ilm.LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY; @@ -25,8 +27,11 @@ public final class InitializePolicyContextStep extends ClusterStateActionStep { public static final StepKey KEY = new StepKey(INITIALIZATION_PHASE, "init", "init"); private static final Logger logger = LogManager.getLogger(InitializePolicyContextStep.class); - InitializePolicyContextStep(Step.StepKey key, StepKey nextStepKey) { + private final String policy; + + InitializePolicyContextStep(String policy, Step.StepKey key, StepKey nextStepKey) { super(key, nextStepKey); + this.policy = policy; } @Override @@ -38,16 +43,15 @@ public ClusterState performAction(Index index, ClusterState clusterState) { return clusterState; } - LifecycleExecutionState lifecycleState = LifecycleExecutionState - .fromIndexMetadata(indexMetaData); - - if (lifecycleState.getLifecycleDate() != null) { - return clusterState; - } - IndexMetaData.Builder indexMetadataBuilder = IndexMetaData.builder(indexMetaData); - if (shouldParseIndexName(indexMetaData.getSettings())) { - try { + LifecycleExecutionState lifecycleState; + try { + lifecycleState = LifecycleExecutionState.fromIndexMetadata(indexMetaData); + if (lifecycleState.getLifecycleDate() != null) { + return clusterState; + } + + if (shouldParseIndexName(indexMetaData.getSettings())) { long parsedOriginationDate = parseIndexNameAndExtractDate(index.getName()); indexMetadataBuilder.settingsVersion(indexMetaData.getSettingsVersion() + 1) .settings(Settings.builder() @@ -55,9 +59,9 @@ public ClusterState performAction(Index index, ClusterState clusterState) { .put(LifecycleSettings.LIFECYCLE_ORIGINATION_DATE, parsedOriginationDate) .build() ); - } catch (Exception e) { - throw new InitializePolicyException(e.getMessage(), e); } + } catch (Exception e) { + throw new InitializePolicyException(policy, index.getName(), e); } ClusterState.Builder newClusterStateBuilder = ClusterState.builder(clusterState); @@ -76,4 +80,22 @@ public ClusterState performAction(Index index, ClusterState clusterState) { public boolean isRetryable() { return true; } + + String policy() { + return policy; + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + if (!super.equals(o)) return false; + InitializePolicyContextStep that = (InitializePolicyContextStep) o; + return policy.equals(that.policy); + } + + @Override + public int hashCode() { + return Objects.hash(super.hashCode(), policy); + } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java index 0023931b3f8a4..8feda10abdbc2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyException.java @@ -7,12 +7,14 @@ import org.elasticsearch.ElasticsearchException; +import java.util.Locale; + /** * Exception thrown when a problem is encountered while initialising an ILM policy for an index. */ public class InitializePolicyException extends ElasticsearchException { - public InitializePolicyException(String msg, Throwable cause, Object... args) { - super(msg, cause, args); + public InitializePolicyException(String policy, String index, Throwable cause) { + super(String.format(Locale.ROOT, "unable to initialize policy [%s] for index [%s]", policy, index), cause); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java index 795a83f64444a..9cd3b1ca1c6e0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java @@ -215,7 +215,7 @@ public List toSteps(Client client) { lastStepKey = phaseAfterStep.getKey(); // init step so that policy is guaranteed to have - steps.add(new InitializePolicyContextStep(InitializePolicyContextStep.KEY, lastStepKey)); + steps.add(new InitializePolicyContextStep(name, InitializePolicyContextStep.KEY, lastStepKey)); Collections.reverse(steps); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java index 404240810f2e6..2db25d719ad4a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java @@ -24,31 +24,34 @@ public InitializePolicyContextStep createRandomInstance() { StepKey stepKey = randomStepKey(); StepKey nextStepKey = randomStepKey(); - return new InitializePolicyContextStep(stepKey, nextStepKey); + return new InitializePolicyContextStep(randomAlphaOfLengthBetween(1, 10), stepKey, nextStepKey); } @Override public InitializePolicyContextStep mutateInstance(InitializePolicyContextStep instance) { + String policy = instance.policy(); StepKey key = instance.getKey(); StepKey nextKey = instance.getNextStepKey(); - switch (between(0, 1)) { - case 0: - key = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5)); - break; - case 1: - nextKey = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5)); - break; - default: - throw new AssertionError("Illegal randomisation branch"); + switch (between(0, 2)) { + case 0: + policy = randomAlphaOfLengthBetween(1, 10); + case 1: + key = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5)); + break; + case 2: + nextKey = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5)); + break; + default: + throw new AssertionError("Illegal randomisation branch"); } - return new InitializePolicyContextStep(key, nextKey); + return new InitializePolicyContextStep(policy, key, nextKey); } @Override public InitializePolicyContextStep copyInstance(InitializePolicyContextStep instance) { - return new InitializePolicyContextStep(instance.getKey(), instance.getNextStepKey()); + return new InitializePolicyContextStep(instance.policy(), instance.getKey(), instance.getNextStepKey()); } public void testAddCreationDate() { @@ -63,7 +66,7 @@ public void testAddCreationDate() { .build(); Index index = indexMetadata.getIndex(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metaData(metaData).build(); - InitializePolicyContextStep step = new InitializePolicyContextStep(null, null); + InitializePolicyContextStep step = new InitializePolicyContextStep(randomAlphaOfLengthBetween(1, 10), null, null); ClusterState newState = step.performAction(index, clusterState); assertThat(getIndexLifecycleDate(index, newState), equalTo(creationDate)); } @@ -83,7 +86,7 @@ public void testDoNothing() { .build(); Index index = indexMetadata.getIndex(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metaData(metaData).build(); - InitializePolicyContextStep step = new InitializePolicyContextStep(null, null); + InitializePolicyContextStep step = new InitializePolicyContextStep(randomAlphaOfLengthBetween(1, 10), null, null); ClusterState newState = step.performAction(index, clusterState); assertTrue(newState == clusterState); } From 8523f21de8ceec8709b5a2938ec471a0dfac3aad Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 12 Feb 2020 12:25:21 +0000 Subject: [PATCH 06/10] Document moving to error step from the init step --- .../org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java index d1b9441f7fcff..95b441861bfcd 100644 --- a/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java +++ b/x-pack/plugin/ilm/src/main/java/org/elasticsearch/xpack/ilm/IndexLifecycleTransition.java @@ -135,6 +135,9 @@ static ClusterState moveClusterStateToErrorStep(Index index, ClusterState cluste causeXContentBuilder.endObject(); LifecycleExecutionState currentState = LifecycleExecutionState.fromIndexMetadata(idxMeta); Step.StepKey currentStep; + // if an error is encountered while initialising the policy the lifecycle execution state will not yet contain any step information + // as we haven't yet initialised the policy, so we'll manually set the current step to be the "initialize policy" step so we can + // record the error (and later retry the init policy step) if (cause instanceof InitializePolicyException) { currentStep = InitializePolicyContextStep.KEY; } else { From b609366cdc67b21836d0e8488094ba9b64ce5b65 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 12 Feb 2020 12:25:43 +0000 Subject: [PATCH 07/10] ShrunkenIndexCheckStep: Use correct logger --- .../elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStep.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStep.java index 6b6a70ca50172..715352dce2b43 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrunkenIndexCheckStep.java @@ -25,7 +25,7 @@ */ public class ShrunkenIndexCheckStep extends ClusterStateWaitStep { public static final String NAME = "is-shrunken-index"; - private static final Logger logger = LogManager.getLogger(InitializePolicyContextStep.class); + private static final Logger logger = LogManager.getLogger(ShrunkenIndexCheckStep.class); private String shrunkIndexPrefix; public ShrunkenIndexCheckStep(StepKey key, StepKey nextStepKey, String shrunkIndexPrefix) { From 3355825b3d4947d8fe6b8c55dedfda91aacaf985 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 12 Feb 2020 12:42:26 +0000 Subject: [PATCH 08/10] break in case statement --- .../xpack/core/ilm/InitializePolicyContextStepTests.java | 1 + 1 file changed, 1 insertion(+) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java index 2db25d719ad4a..93289bf854c1d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java @@ -36,6 +36,7 @@ public InitializePolicyContextStep mutateInstance(InitializePolicyContextStep in switch (between(0, 2)) { case 0: policy = randomAlphaOfLengthBetween(1, 10); + break; case 1: key = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5)); break; From 04c444c03d7add3ffea66ede07768d8c35eb8392 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 12 Feb 2020 16:40:39 +0000 Subject: [PATCH 09/10] Derive the policy name from the index metadata --- .../core/ilm/InitializePolicyContextStep.java | 24 ++----------------- .../xpack/core/ilm/LifecyclePolicy.java | 2 +- .../ilm/InitializePolicyContextStepTests.java | 18 ++++++-------- 3 files changed, 10 insertions(+), 34 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java index b69541ab580ca..25c22ee0ea56f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java @@ -27,11 +27,8 @@ public final class InitializePolicyContextStep extends ClusterStateActionStep { public static final StepKey KEY = new StepKey(INITIALIZATION_PHASE, "init", "init"); private static final Logger logger = LogManager.getLogger(InitializePolicyContextStep.class); - private final String policy; - - InitializePolicyContextStep(String policy, Step.StepKey key, StepKey nextStepKey) { + InitializePolicyContextStep(Step.StepKey key, StepKey nextStepKey) { super(key, nextStepKey); - this.policy = policy; } @Override @@ -61,6 +58,7 @@ public ClusterState performAction(Index index, ClusterState clusterState) { ); } } catch (Exception e) { + String policy = indexMetaData.getSettings().get(LifecycleSettings.LIFECYCLE_NAME); throw new InitializePolicyException(policy, index.getName(), e); } @@ -80,22 +78,4 @@ public ClusterState performAction(Index index, ClusterState clusterState) { public boolean isRetryable() { return true; } - - String policy() { - return policy; - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - if (!super.equals(o)) return false; - InitializePolicyContextStep that = (InitializePolicyContextStep) o; - return policy.equals(that.policy); - } - - @Override - public int hashCode() { - return Objects.hash(super.hashCode(), policy); - } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java index 9cd3b1ca1c6e0..795a83f64444a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecyclePolicy.java @@ -215,7 +215,7 @@ public List toSteps(Client client) { lastStepKey = phaseAfterStep.getKey(); // init step so that policy is guaranteed to have - steps.add(new InitializePolicyContextStep(name, InitializePolicyContextStep.KEY, lastStepKey)); + steps.add(new InitializePolicyContextStep(InitializePolicyContextStep.KEY, lastStepKey)); Collections.reverse(steps); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java index 93289bf854c1d..482f3e4183778 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStepTests.java @@ -24,35 +24,31 @@ public InitializePolicyContextStep createRandomInstance() { StepKey stepKey = randomStepKey(); StepKey nextStepKey = randomStepKey(); - return new InitializePolicyContextStep(randomAlphaOfLengthBetween(1, 10), stepKey, nextStepKey); + return new InitializePolicyContextStep(stepKey, nextStepKey); } @Override public InitializePolicyContextStep mutateInstance(InitializePolicyContextStep instance) { - String policy = instance.policy(); StepKey key = instance.getKey(); StepKey nextKey = instance.getNextStepKey(); - switch (between(0, 2)) { + switch (between(0, 1)) { case 0: - policy = randomAlphaOfLengthBetween(1, 10); - break; - case 1: key = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5)); break; - case 2: + case 1: nextKey = new StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5)); break; default: throw new AssertionError("Illegal randomisation branch"); } - return new InitializePolicyContextStep(policy, key, nextKey); + return new InitializePolicyContextStep(key, nextKey); } @Override public InitializePolicyContextStep copyInstance(InitializePolicyContextStep instance) { - return new InitializePolicyContextStep(instance.policy(), instance.getKey(), instance.getNextStepKey()); + return new InitializePolicyContextStep(instance.getKey(), instance.getNextStepKey()); } public void testAddCreationDate() { @@ -67,7 +63,7 @@ public void testAddCreationDate() { .build(); Index index = indexMetadata.getIndex(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metaData(metaData).build(); - InitializePolicyContextStep step = new InitializePolicyContextStep(randomAlphaOfLengthBetween(1, 10), null, null); + InitializePolicyContextStep step = new InitializePolicyContextStep(null, null); ClusterState newState = step.performAction(index, clusterState); assertThat(getIndexLifecycleDate(index, newState), equalTo(creationDate)); } @@ -87,7 +83,7 @@ public void testDoNothing() { .build(); Index index = indexMetadata.getIndex(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).metaData(metaData).build(); - InitializePolicyContextStep step = new InitializePolicyContextStep(randomAlphaOfLengthBetween(1, 10), null, null); + InitializePolicyContextStep step = new InitializePolicyContextStep(null, null); ClusterState newState = step.performAction(index, clusterState); assertTrue(newState == clusterState); } From 569509156640ae2fcad3f887314a63fe5524d14e Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 12 Feb 2020 16:42:02 +0000 Subject: [PATCH 10/10] Remove unused import --- .../xpack/core/ilm/InitializePolicyContextStep.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java index 25c22ee0ea56f..536d72613cccc 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/InitializePolicyContextStep.java @@ -13,8 +13,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.Index; -import java.util.Objects; - import static org.elasticsearch.xpack.core.ilm.IndexLifecycleOriginationDateParser.parseIndexNameAndExtractDate; import static org.elasticsearch.xpack.core.ilm.IndexLifecycleOriginationDateParser.shouldParseIndexName; import static org.elasticsearch.xpack.core.ilm.LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY;