From 610651db01b2a17e32b4881565a517c64fcf8b99 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Wed, 31 Oct 2018 21:01:19 -0600 Subject: [PATCH 1/3] Skip Rollover step if next index already exists If the Rollover step would fail due to the next index in sequence already existing, just skip to the next step instead of going to the Error step. This prevents spurious `ResourceAlreadyExistsException`s created by simultaneous RolloverStep executions from causing ILM to error out unnecessarily. --- .../core/indexlifecycle/RolloverStep.java | 16 ++++- .../TimeSeriesLifecycleActionsIT.java | 70 ++++++++++++++++--- 2 files changed, 77 insertions(+), 9 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java index 94505c620e750..69f5e43d26fd0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java @@ -5,6 +5,9 @@ */ package org.elasticsearch.xpack.core.indexlifecycle; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; +import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; import org.elasticsearch.client.Client; @@ -22,6 +25,8 @@ public class RolloverStep extends AsyncWaitStep { public static final String NAME = "attempt_rollover"; + private static final Logger logger = LogManager.getLogger(RolloverStep.class); + private ByteSizeValue maxSize; private TimeValue maxAge; private Long maxDocs; @@ -63,7 +68,16 @@ public void evaluateCondition(IndexMetaData indexMetaData, Listener listener) { rolloverRequest.addMaxIndexDocsCondition(maxDocs); } getClient().admin().indices().rolloverIndex(rolloverRequest, - ActionListener.wrap(response -> listener.onResponse(response.isRolledOver(), new EmptyInfo()), listener::onFailure)); + ActionListener.wrap(response -> listener.onResponse(response.isRolledOver(), new EmptyInfo()), exception -> { + if (exception instanceof ResourceAlreadyExistsException) { + // This can happen sometimes when this step is executed multiple times + logger.debug("index [{}] cannot roll over because the next index already exists, skipping to next step", + indexMetaData.getIndex()); + listener.onResponse(true, new EmptyInfo()); + } else { + listener.onFailure(exception); + } + })); } ByteSizeValue getMaxSize() { diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java index 352f52bd1aa7a..7aded999491a4 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java @@ -22,6 +22,7 @@ import org.elasticsearch.test.rest.ESRestTestCase; import org.elasticsearch.xpack.core.indexlifecycle.AllocateAction; import org.elasticsearch.xpack.core.indexlifecycle.DeleteAction; +import org.elasticsearch.xpack.core.indexlifecycle.ErrorStep; import org.elasticsearch.xpack.core.indexlifecycle.ForceMergeAction; import org.elasticsearch.xpack.core.indexlifecycle.LifecycleAction; import org.elasticsearch.xpack.core.indexlifecycle.LifecyclePolicy; @@ -192,6 +193,38 @@ public void testRolloverAction() throws Exception { assertBusy(() -> assertTrue(indexExists(originalIndex))); } + public void testRolloverAlreadyExists() throws Exception { + String originalIndex = index + "-000001"; + String secondIndex = index + "-000002"; + createIndexWithSettings(originalIndex, Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0) + .put(RolloverAction.LIFECYCLE_ROLLOVER_ALIAS, "alias")); + + // create policy + createNewSingletonPolicy("hot", new RolloverAction(null, null, 1L)); + // update policy on index + updatePolicy(originalIndex, policy); + + // Manually create the new index + Request request = new Request("PUT", "/" + secondIndex); + request.setJsonEntity("{\n \"settings\": " + Strings.toString(Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0).build()) + "}"); + client().performRequest(request); + // wait for the shards to initialize + ensureGreen(secondIndex); + + // index another doc to trigger the policy + index(client(), originalIndex, "_id", "foo", "bar"); + + assertBusy(() -> { + logger.info(originalIndex + ": " + getStepKeyForIndex(originalIndex)); + logger.info(secondIndex + ": " + getStepKeyForIndex(secondIndex)); + assertThat(getStepKeyForIndex(originalIndex), equalTo(new StepKey("hot", RolloverAction.NAME, ErrorStep.NAME))); + assertThat(getFailedStepForIndex(originalIndex), equalTo("update-rollover-lifecycle-date")); + assertThat(getReasonForIndex(originalIndex), equalTo("index [" + originalIndex + "] has not rolled over yet")); + }); + } + public void testAllocateOnlyAllocation() throws Exception { createIndexWithSettings(index, Settings.builder().put(IndexMetaData.SETTING_NUMBER_OF_SHARDS, 2) .put(IndexMetaData.SETTING_NUMBER_OF_REPLICAS, 0)); @@ -436,6 +469,33 @@ private Map getOnlyIndexSettings(String index) throws IOExceptio } private StepKey getStepKeyForIndex(String indexName) throws IOException { + Map indexResponse = explainIndex(indexName); + if (indexResponse == null) { + return new StepKey(null, null, null); + } + + String phase = (String) indexResponse.get("phase"); + String action = (String) indexResponse.get("action"); + String step = (String) indexResponse.get("step"); + return new StepKey(phase, action, step); + } + + private String getFailedStepForIndex(String indexName) throws IOException { + Map indexResponse = explainIndex(indexName); + if (indexResponse == null) return null; + + return (String) indexResponse.get("failed_step"); + } + + @SuppressWarnings("unchecked") + private String getReasonForIndex(String indexName) throws IOException { + Map indexResponse = explainIndex(indexName); + if (indexResponse == null) return null; + + return ((Map) indexResponse.get("step_info")).get("reason"); + } + + private Map explainIndex(String indexName) throws IOException { Request explainRequest = new Request("GET", indexName + "/_ilm/explain"); Response response = client().performRequest(explainRequest); Map responseMap; @@ -443,15 +503,9 @@ private StepKey getStepKeyForIndex(String indexName) throws IOException { responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); } - @SuppressWarnings("unchecked") Map indexResponse = ((Map>) responseMap.get("indices")) + @SuppressWarnings("unchecked") Map indexResponse = ((Map>) responseMap.get("indices")) .get(indexName); - if (indexResponse == null) { - return new StepKey(null, null, null); - } - String phase = indexResponse.get("phase"); - String action = indexResponse.get("action"); - String step = indexResponse.get("step"); - return new StepKey(phase, action, step); + return indexResponse; } private void indexDocument() throws IOException { From 1e60f850e00d6d804f1a458db59310b33b47a5fc Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Thu, 1 Nov 2018 12:56:30 -0600 Subject: [PATCH 2/3] Review feedback on logging --- .../xpack/core/indexlifecycle/RolloverStep.java | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java index 69f5e43d26fd0..399f90df31dae 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/RolloverStep.java @@ -7,6 +7,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.ResourceAlreadyExistsException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.rollover.RolloverRequest; @@ -71,8 +72,13 @@ public void evaluateCondition(IndexMetaData indexMetaData, Listener listener) { ActionListener.wrap(response -> listener.onResponse(response.isRolledOver(), new EmptyInfo()), exception -> { if (exception instanceof ResourceAlreadyExistsException) { // This can happen sometimes when this step is executed multiple times - logger.debug("index [{}] cannot roll over because the next index already exists, skipping to next step", - indexMetaData.getIndex()); + if (logger.isTraceEnabled()) { + logger.debug(() -> new ParameterizedMessage("{} index cannot roll over because the next index already exists, " + + "skipping to next step", indexMetaData.getIndex()), exception); + } else { + logger.debug("{} index cannot roll over because the next index already exists, skipping to next step", + indexMetaData.getIndex()); + } listener.onResponse(true, new EmptyInfo()); } else { listener.onFailure(exception); From d75b78ed970d6de11e71b9ecf2bbec8ecf2132e0 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Fri, 2 Nov 2018 11:04:14 -0600 Subject: [PATCH 3/3] Fix error message to include both possiblities --- .../core/indexlifecycle/UpdateRolloverLifecycleDateStep.java | 3 ++- .../indexlifecycle/UpdateRolloverLifecycleDateStepTests.java | 3 ++- .../xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java | 3 ++- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStep.java index 9d1c7701faa5a..704d122f571a6 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStep.java @@ -32,7 +32,8 @@ public ClusterState performAction(Index index, ClusterState currentState) { } RolloverInfo rolloverInfo = indexMetaData.getRolloverInfos().get(rolloverAlias); if (rolloverInfo == null) { - throw new IllegalStateException("index [" + indexMetaData.getIndex().getName() + "] has not rolled over yet"); + throw new IllegalStateException("no rollover info found for [" + indexMetaData.getIndex().getName() + "], either the index " + + "has not yet rolled over or a subsequent index was created outside of Index Lifecycle Management"); } LifecycleExecutionState.Builder newLifecycleState = LifecycleExecutionState diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStepTests.java index 5a4c88eaa6ab2..6e492e24f9b33 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/UpdateRolloverLifecycleDateStepTests.java @@ -87,7 +87,8 @@ public void testPerformActionBeforeRolloverHappened() { IllegalStateException exceptionThrown = expectThrows(IllegalStateException.class, () -> step.performAction(indexMetaData.getIndex(), clusterState)); assertThat(exceptionThrown.getMessage(), - equalTo("index [" + indexMetaData.getIndex().getName() + "] has not rolled over yet")); + equalTo("no rollover info found for [" + indexMetaData.getIndex().getName() + "], either the index " + + "has not yet rolled over or a subsequent index was created outside of Index Lifecycle Management")); } public void testPerformActionWithNoRolloverAliasSetting() { diff --git a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java index 7aded999491a4..8dc8427bc7640 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/test/java/org/elasticsearch/xpack/indexlifecycle/TimeSeriesLifecycleActionsIT.java @@ -221,7 +221,8 @@ public void testRolloverAlreadyExists() throws Exception { logger.info(secondIndex + ": " + getStepKeyForIndex(secondIndex)); assertThat(getStepKeyForIndex(originalIndex), equalTo(new StepKey("hot", RolloverAction.NAME, ErrorStep.NAME))); assertThat(getFailedStepForIndex(originalIndex), equalTo("update-rollover-lifecycle-date")); - assertThat(getReasonForIndex(originalIndex), equalTo("index [" + originalIndex + "] has not rolled over yet")); + assertThat(getReasonForIndex(originalIndex), equalTo("no rollover info found for [" + originalIndex + "], either the index " + + "has not yet rolled over or a subsequent index was created outside of Index Lifecycle Management")); }); }