From 0fbb8a16bcca543e0507a558a27bdecab7270ee6 Mon Sep 17 00:00:00 2001 From: Gordon Brown Date: Mon, 5 Nov 2018 09:20:43 -0700 Subject: [PATCH] Skip Rollover step if next index already exists (#35168) 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 | 22 +++++- .../UpdateRolloverLifecycleDateStep.java | 3 +- .../UpdateRolloverLifecycleDateStepTests.java | 3 +- .../TimeSeriesLifecycleActionsIT.java | 71 ++++++++++++++++--- 4 files changed, 88 insertions(+), 11 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 94505c620e7..399f90df31d 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,10 @@ */ package org.elasticsearch.xpack.core.indexlifecycle; +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; import org.elasticsearch.client.Client; @@ -22,6 +26,8 @@ import java.util.Objects; 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 +69,21 @@ public class RolloverStep extends AsyncWaitStep { 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 + 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); + } + })); } ByteSizeValue getMaxSize() { 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 9d1c7701faa..704d122f571 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 class UpdateRolloverLifecycleDateStep extends ClusterStateActionStep { } 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 5a4c88eaa6a..6e492e24f9b 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 class UpdateRolloverLifecycleDateStepTests extends AbstractStepTestCase 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 352f52bd1aa..8dc8427bc76 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.common.xcontent.XContentType; 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,39 @@ public class TimeSeriesLifecycleActionsIT extends ESRestTestCase { 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("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")); + }); + } + 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 +470,33 @@ public class TimeSeriesLifecycleActionsIT extends ESRestTestCase { } 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 +504,9 @@ public class TimeSeriesLifecycleActionsIT extends ESRestTestCase { 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 {