Browse Source

Generate random rollup index names for RollupILMAction (#69237)

This commit moves away from the static `rollup-{indexName}` rollup index
naming strategy and moves towards a randomized rollup index name scheme.

This will reduce the complications that exist if the RollupStep fails and retries
in any way. A separate cleanup will still be required for failed temporary indices,
but at least there will not be a conflict.

This commit generates the new rollup index name in the LifecycleExecutionState so
that it can be used in RollupStep and UpdateRollupIndexPolicyStep on a per-index
basis.
Tal Levy 4 years ago
parent
commit
c1c5103756

+ 1 - 1
docs/reference/ilm/actions/ilm-rollup.asciidoc

@@ -8,7 +8,7 @@ Aggregates an index's time series data and stores the results in a new read-only
 index. For example, you can roll up hourly data into daily or weekly summaries.
 
 This action corresponds to the <<rollup-api,rollup API>>. The name of the
-resulting rollup index is `rollup-<original-index-name>`. If {ilm-init} performs
+resulting rollup index is `rollup-<original-index-name>-<random-uuid>`. If {ilm-init} performs
 the `rollup` action on a backing index for a data stream, the rollup index is a
 backing index for the same stream.
 

+ 80 - 0
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/GenerateRollupIndexNameStep.java

@@ -0,0 +1,80 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+package org.elasticsearch.xpack.core.ilm;
+
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.Metadata;
+import org.elasticsearch.common.UUIDs;
+import org.elasticsearch.index.Index;
+
+import java.util.Locale;
+import java.util.Objects;
+
+import static org.elasticsearch.xpack.core.ilm.LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY;
+import static org.elasticsearch.xpack.core.ilm.LifecycleExecutionState.fromIndexMetadata;
+
+/**
+ * Generates a new rollup index name for the current managed index.
+ * <p>
+ * The generated rollup index name will be in the format {rollup-indexName-randomUUID}
+ * eg.: rollup-myindex-cmuce-qfvmn_dstqw-ivmjc1etsa
+ */
+public class GenerateRollupIndexNameStep extends ClusterStateActionStep {
+
+    public static final String NAME = "generate-rollup-name";
+
+    private static final Logger logger = LogManager.getLogger(GenerateRollupIndexNameStep.class);
+
+    public GenerateRollupIndexNameStep(StepKey key, StepKey nextStepKey) {
+        super(key, nextStepKey);
+    }
+
+    @Override
+    public ClusterState performAction(Index index, ClusterState clusterState) {
+        IndexMetadata indexMetaData = clusterState.metadata().index(index);
+        if (indexMetaData == null) {
+            // Index must have been since deleted, ignore it
+            logger.debug("[{}] lifecycle action for index [{}] executed but index no longer exists", getKey().getAction(), index.getName());
+            return clusterState;
+        }
+        ClusterState.Builder newClusterStateBuilder = ClusterState.builder(clusterState);
+        LifecycleExecutionState lifecycleState = fromIndexMetadata(indexMetaData);
+        assert lifecycleState.getRollupIndexName() == null : "index " + index.getName() +
+            " should have a rollup index name generated by the ilm policy but has " + lifecycleState.getRollupIndexName();
+        LifecycleExecutionState.Builder newCustomData = LifecycleExecutionState.builder(lifecycleState);
+        String rollupIndexName = generateRollupIndexName(index.getName());
+        newCustomData.setRollupIndexName(rollupIndexName);
+        IndexMetadata.Builder indexMetadataBuilder = IndexMetadata.builder(indexMetaData);
+        indexMetadataBuilder.putCustom(ILM_CUSTOM_METADATA_KEY, newCustomData.build().asMap());
+        newClusterStateBuilder.metadata(Metadata.builder(clusterState.getMetadata()).put(indexMetadataBuilder));
+        return newClusterStateBuilder.build();
+    }
+
+    // TODO(talevy): find alternative to lowercasing UUID? kind of defeats the expectation of the UUID here. index names must lowercase
+    public static String generateRollupIndexName(String indexName) {
+        return "rollup-" + indexName + "-" +  UUIDs.randomBase64UUID().toLowerCase(Locale.ROOT);
+    }
+
+    @Override
+    public int hashCode() {
+        return Objects.hash(super.hashCode());
+    }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == null) {
+            return false;
+        }
+        if (getClass() != obj.getClass()) {
+            return false;
+        }
+        return super.equals(obj);
+    }
+}

+ 23 - 2
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/LifecycleExecutionState.java

@@ -40,6 +40,7 @@ public class LifecycleExecutionState {
     private static final String SNAPSHOT_NAME = "snapshot_name";
     private static final String SNAPSHOT_REPOSITORY = "snapshot_repository";
     private static final String SNAPSHOT_INDEX_NAME = "snapshot_index_name";
+    private static final String ROLLUP_INDEX_NAME = "rollup_index_name";
 
     private final String phase;
     private final String action;
@@ -56,11 +57,12 @@ public class LifecycleExecutionState {
     private final String snapshotName;
     private final String snapshotRepository;
     private final String snapshotIndexName;
+    private final String rollupIndexName;
 
     private LifecycleExecutionState(String phase, String action, String step, String failedStep, Boolean isAutoRetryableError,
                                     Integer failedStepRetryCount, String stepInfo, String phaseDefinition, Long lifecycleDate,
                                     Long phaseTime, Long actionTime, Long stepTime, String snapshotRepository, String snapshotName,
-                                    String snapshotIndexName) {
+                                    String snapshotIndexName, String rollupIndexName) {
         this.phase = phase;
         this.action = action;
         this.step = step;
@@ -76,6 +78,7 @@ public class LifecycleExecutionState {
         this.snapshotRepository = snapshotRepository;
         this.snapshotName = snapshotName;
         this.snapshotIndexName = snapshotIndexName;
+        this.rollupIndexName = rollupIndexName;
     }
 
     /**
@@ -136,6 +139,7 @@ public class LifecycleExecutionState {
             .setSnapshotRepository(state.snapshotRepository)
             .setSnapshotName(state.snapshotName)
             .setSnapshotIndexName(state.snapshotIndexName)
+            .setRollupIndexName(state.rollupIndexName)
             .setStepTime(state.stepTime);
     }
 
@@ -206,6 +210,9 @@ public class LifecycleExecutionState {
         if (customData.containsKey(SNAPSHOT_INDEX_NAME)) {
             builder.setSnapshotIndexName(customData.get(SNAPSHOT_INDEX_NAME));
         }
+        if (customData.containsKey(ROLLUP_INDEX_NAME)) {
+            builder.setRollupIndexName(customData.get(ROLLUP_INDEX_NAME));
+        }
         return builder.build();
     }
 
@@ -261,6 +268,9 @@ public class LifecycleExecutionState {
         if (snapshotIndexName != null) {
             result.put(SNAPSHOT_INDEX_NAME, snapshotIndexName);
         }
+        if (rollupIndexName != null) {
+            result.put(ROLLUP_INDEX_NAME, rollupIndexName);
+        }
         return Collections.unmodifiableMap(result);
     }
 
@@ -324,6 +334,10 @@ public class LifecycleExecutionState {
         return snapshotIndexName;
     }
 
+    public String getRollupIndexName() {
+        return rollupIndexName;
+    }
+
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -374,6 +388,7 @@ public class LifecycleExecutionState {
         private String snapshotName;
         private String snapshotRepository;
         private String snapshotIndexName;
+        private String rollupIndexName;
 
         public Builder setPhase(String phase) {
             this.phase = phase;
@@ -450,9 +465,15 @@ public class LifecycleExecutionState {
             return this;
         }
 
+        public Builder setRollupIndexName(String rollupIndexName) {
+            this.rollupIndexName = rollupIndexName;
+            return this;
+        }
+
         public LifecycleExecutionState build() {
             return new LifecycleExecutionState(phase, action, step, failedStep, isAutoRetryableError, failedStepRetryCount, stepInfo,
-                phaseDefinition, indexCreationDate, phaseTime, actionTime, stepTime, snapshotRepository, snapshotName, snapshotIndexName);
+                phaseDefinition, indexCreationDate, phaseTime, actionTime, stepTime, snapshotRepository, snapshotName,
+                snapshotIndexName, rollupIndexName);
         }
     }
 

+ 5 - 3
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/RollupILMAction.java

@@ -97,19 +97,21 @@ public class RollupILMAction implements LifecycleAction {
     public List<Step> toSteps(Client client, String phase, StepKey nextStepKey) {
         StepKey checkNotWriteIndex = new StepKey(phase, NAME, CheckNotDataStreamWriteIndexStep.NAME);
         StepKey readOnlyKey = new StepKey(phase, NAME, ReadOnlyStep.NAME);
+        StepKey generateRollupIndexNameKey = new StepKey(phase, NAME, GenerateRollupIndexNameStep.NAME);
         StepKey rollupKey = new StepKey(phase, NAME, NAME);
         CheckNotDataStreamWriteIndexStep checkNotWriteIndexStep = new CheckNotDataStreamWriteIndexStep(checkNotWriteIndex,
             readOnlyKey);
-        ReadOnlyStep readOnlyStep = new ReadOnlyStep(readOnlyKey, rollupKey, client);
+        ReadOnlyStep readOnlyStep = new ReadOnlyStep(readOnlyKey, generateRollupIndexNameKey, client);
+        GenerateRollupIndexNameStep generateRollupIndexNameStep = new GenerateRollupIndexNameStep(generateRollupIndexNameKey, rollupKey);
         if (rollupPolicy == null) {
             Step rollupStep = new RollupStep(rollupKey, nextStepKey, client, config);
-            return List.of(checkNotWriteIndexStep, readOnlyStep, rollupStep);
+            return List.of(checkNotWriteIndexStep, readOnlyStep, generateRollupIndexNameStep, rollupStep);
         } else {
             StepKey updateRollupIndexPolicyStepKey = new StepKey(phase, NAME, UpdateRollupIndexPolicyStep.NAME);
             Step rollupStep = new RollupStep(rollupKey, updateRollupIndexPolicyStepKey, client, config);
             Step updateRollupIndexPolicyStep = new UpdateRollupIndexPolicyStep(updateRollupIndexPolicyStepKey, nextStepKey,
                 client, rollupPolicy);
-            return List.of(checkNotWriteIndexStep, readOnlyStep, rollupStep, updateRollupIndexPolicyStep);
+            return List.of(checkNotWriteIndexStep, readOnlyStep, generateRollupIndexNameStep, rollupStep, updateRollupIndexPolicyStep);
         }
     }
 

+ 13 - 6
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/RollupStep.java

@@ -11,11 +11,14 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.ClusterStateObserver;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.xpack.core.rollup.RollupActionConfig;
 import org.elasticsearch.xpack.core.rollup.action.RollupAction;
 
 import java.util.Objects;
 
+import static org.elasticsearch.xpack.core.ilm.LifecycleExecutionState.fromIndexMetadata;
+
 /**
  * Rolls up index using a {@link RollupActionConfig}
  */
@@ -30,10 +33,6 @@ public class RollupStep extends AsyncActionStep {
         this.config = config;
     }
 
-    public static String getRollupIndexName(String index) {
-        return ROLLUP_INDEX_NAME_PREFIX + index;
-    }
-
     @Override
     public boolean isRetryable() {
         return true;
@@ -41,8 +40,16 @@ public class RollupStep extends AsyncActionStep {
 
     @Override
     public void performAction(IndexMetadata indexMetadata, ClusterState currentState, ClusterStateObserver observer, Listener listener) {
-        String originalIndex = indexMetadata.getIndex().getName();
-        RollupAction.Request request = new RollupAction.Request(originalIndex, getRollupIndexName(originalIndex), config);
+        final String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
+        final String indexName = indexMetadata.getIndex().getName();
+        final LifecycleExecutionState lifecycleState = fromIndexMetadata(indexMetadata);
+        final String rollupIndexName = lifecycleState.getRollupIndexName();
+        if (Strings.hasText(rollupIndexName) == false) {
+            listener.onFailure(new IllegalStateException("rollup index name was not generated for policy [" + policyName +
+                "] and index [" + indexName + "]"));
+            return;
+        }
+        RollupAction.Request request = new RollupAction.Request(indexName, rollupIndexName, config);
         // currently RollupAction always acknowledges action was complete when no exceptions are thrown.
         getClient().execute(RollupAction.INSTANCE, request,
             ActionListener.wrap(response -> listener.onResponse(true), listener::onFailure));

+ 13 - 2
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/UpdateRollupIndexPolicyStep.java

@@ -13,10 +13,13 @@ import org.elasticsearch.client.Client;
 import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.ClusterStateObserver;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.settings.Settings;
 
 import java.util.Objects;
 
+import static org.elasticsearch.xpack.core.ilm.LifecycleExecutionState.fromIndexMetadata;
+
 /**
  * Updates the lifecycle policy for the rollup index for the original/currently managed index
  */
@@ -41,9 +44,17 @@ public class UpdateRollupIndexPolicyStep extends AsyncActionStep {
 
     @Override
     public void performAction(IndexMetadata indexMetadata, ClusterState currentState, ClusterStateObserver observer, Listener listener) {
-        String rollupIndex = RollupStep.getRollupIndexName(indexMetadata.getIndex().getName());
+        final String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
+        final String indexName = indexMetadata.getIndex().getName();
+        final LifecycleExecutionState lifecycleState = fromIndexMetadata(indexMetadata);
+        final String rollupIndexName = lifecycleState.getRollupIndexName();
+        if (Strings.hasText(rollupIndexName) == false) {
+            listener.onFailure(new IllegalStateException("rollup index name was not generated for policy [" + policyName +
+                "] and index [" + indexName + "]"));
+            return;
+        }
         Settings settings = Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, rollupPolicy).build();
-        UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(rollupIndex)
+        UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(rollupIndexName)
             .masterNodeTimeout(getMasterTimeout(currentState))
             .settings(settings);
         getClient().admin().indices().updateSettings(updateSettingsRequest, ActionListener.wrap(response -> {

+ 76 - 0
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/GenerateRollupIndexNameStepTests.java

@@ -0,0 +1,76 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+package org.elasticsearch.xpack.core.ilm;
+
+import org.elasticsearch.Version;
+import org.elasticsearch.cluster.ClusterState;
+import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.Metadata;
+
+import java.util.Locale;
+
+import static org.elasticsearch.xpack.core.ilm.AbstractStepMasterTimeoutTestCase.emptyClusterState;
+import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.startsWith;
+
+public class GenerateRollupIndexNameStepTests extends AbstractStepTestCase<GenerateRollupIndexNameStep> {
+
+    @Override
+    protected GenerateRollupIndexNameStep createRandomInstance() {
+        return new GenerateRollupIndexNameStep(randomStepKey(), randomStepKey());
+    }
+
+    @Override
+    protected GenerateRollupIndexNameStep mutateInstance(GenerateRollupIndexNameStep instance) {
+        Step.StepKey key = instance.getKey();
+        Step.StepKey nextKey = instance.getNextStepKey();
+
+        switch (between(0, 1)) {
+            case 0:
+                key = new Step.StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5));
+                break;
+            case 1:
+                nextKey = new Step.StepKey(key.getPhase(), key.getAction(), key.getName() + randomAlphaOfLength(5));
+                break;
+            default:
+                throw new AssertionError("Illegal randomisation branch");
+        }
+        return new GenerateRollupIndexNameStep(key, nextKey);
+    }
+
+    @Override
+    protected GenerateRollupIndexNameStep copyInstance(GenerateRollupIndexNameStep instance) {
+        return new GenerateRollupIndexNameStep(instance.getKey(), instance.getNextStepKey());
+    }
+
+    public void testPerformAction() {
+        String indexName = randomAlphaOfLength(10).toLowerCase(Locale.ROOT);
+        String policyName = "test-ilm-policy";
+        IndexMetadata.Builder indexMetadataBuilder =
+            IndexMetadata.builder(indexName).settings(settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, policyName))
+                .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5));
+
+        final IndexMetadata indexMetadata = indexMetadataBuilder.build();
+        ClusterState clusterState = ClusterState.builder(emptyClusterState())
+                .metadata(Metadata.builder().put(indexMetadata, false).build()).build();
+
+        GenerateRollupIndexNameStep generateRollupIndexNameStep = createRandomInstance();
+        ClusterState newClusterState = generateRollupIndexNameStep.performAction(indexMetadata.getIndex(), clusterState);
+
+        LifecycleExecutionState executionState = LifecycleExecutionState.fromIndexMetadata(newClusterState.metadata().index(indexName));
+        assertThat("the " + GenerateRollupIndexNameStep.NAME + " step must generate a rollup index name",
+            executionState.getRollupIndexName(), notNullValue());
+        assertThat(executionState.getRollupIndexName(), containsString("rollup-" + indexName + "-"));
+    }
+
+    public void testGenerateRollupIndexName() {
+        String indexName = randomAlphaOfLength(5);
+        String generated = GenerateRollupIndexNameStep.generateRollupIndexName(indexName);
+        assertThat(generated, startsWith("rollup-" + indexName + "-"));
+    }
+}

+ 6 - 4
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/RollupILMActionTests.java

@@ -55,13 +55,15 @@ public class RollupILMActionTests extends AbstractActionTestCase<RollupILMAction
             randomAlphaOfLengthBetween(1, 10));
         List<Step> steps = action.toSteps(null, phase, nextStepKey);
         assertNotNull(steps);
-        assertEquals(3, steps.size());
+        assertEquals(4, steps.size());
         assertThat(steps.get(0).getKey().getName(), equalTo(CheckNotDataStreamWriteIndexStep.NAME));
         assertThat(steps.get(0).getNextStepKey().getName(), equalTo(ReadOnlyStep.NAME));
         assertThat(steps.get(1).getKey().getName(), equalTo(ReadOnlyStep.NAME));
-        assertThat(steps.get(1).getNextStepKey().getName(), equalTo(RollupStep.NAME));
-        assertThat(steps.get(2).getKey().getName(), equalTo(RollupStep.NAME));
-        assertThat(steps.get(2).getNextStepKey(), equalTo(nextStepKey));
+        assertThat(steps.get(1).getNextStepKey().getName(), equalTo(GenerateRollupIndexNameStep.NAME));
+        assertThat(steps.get(2).getKey().getName(), equalTo(GenerateRollupIndexNameStep.NAME));
+        assertThat(steps.get(2).getNextStepKey().getName(), equalTo(RollupStep.NAME));
+        assertThat(steps.get(3).getKey().getName(), equalTo(RollupStep.NAME));
+        assertThat(steps.get(3).getNextStepKey(), equalTo(nextStepKey));
     }
 
     public void testEqualsAndHashCode() {

+ 36 - 6
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/RollupStepTests.java

@@ -21,10 +21,15 @@ import org.elasticsearch.xpack.core.rollup.RollupActionConfigTests;
 import org.elasticsearch.xpack.core.rollup.action.RollupAction;
 import org.mockito.Mockito;
 
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 
 import static org.elasticsearch.cluster.DataStreamTestHelper.createTimestampField;
+import static org.elasticsearch.xpack.core.ilm.AbstractStepMasterTimeoutTestCase.emptyClusterState;
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
 
 public class RollupStepTests extends AbstractStepTestCase<RollupStep> {
 
@@ -61,14 +66,18 @@ public class RollupStepTests extends AbstractStepTestCase<RollupStep> {
     }
 
     private IndexMetadata getIndexMetadata(String index) {
-        return IndexMetadata.builder(index)
-            .settings(settings(Version.CURRENT))
-            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
+        Map<String, String> ilmCustom = Collections.singletonMap("rollup_index_name", "rollup-index");
+        return IndexMetadata.builder(index).settings(
+            settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, "test-ilm-policy"))
+            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5))
+            .putCustom(LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY, ilmCustom)
+            .build();
     }
 
     private static void assertRollupActionRequest(RollupAction.Request request, String sourceIndex) {
         assertNotNull(request);
         assertThat(request.getSourceIndex(), equalTo(sourceIndex));
+        assertThat(request.getRollupIndex(), equalTo("rollup-index"));
     }
 
     public void testPerformAction() {
@@ -102,12 +111,33 @@ public class RollupStepTests extends AbstractStepTestCase<RollupStep> {
         assertEquals(true, actionCompleted.get());
     }
 
+    public void testPerformActionFailureInvalidExecutionState() {
+        IndexMetadata indexMetadata = IndexMetadata.builder(randomAlphaOfLength(10)).settings(
+            settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, "test-ilm-policy"))
+            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5))
+            .build();
+        String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
+        String indexName = indexMetadata.getIndex().getName();
+        RollupStep step = createRandomInstance();
+        step.performAction(indexMetadata, emptyClusterState(), null, new AsyncActionStep.Listener() {
+            @Override
+            public void onResponse(boolean complete) {
+                fail("expecting a failure as the index doesn't have any rollup index name in its ILM execution state");
+            }
+
+            @Override
+            public void onFailure(Exception e) {
+                assertThat(e, instanceOf(IllegalStateException.class));
+                assertThat(e.getMessage(),
+                    is("rollup index name was not generated for policy [" + policyName + "] and index [" + indexName + "]"));
+            }
+        });
+    }
+
     public void testPerformActionOnDataStream() {
         String dataStreamName = "test-datastream";
         String backingIndexName = DataStream.getDefaultBackingIndexName(dataStreamName, 1);
-        IndexMetadata indexMetadata = IndexMetadata.builder(backingIndexName)
-            .settings(settings(Version.CURRENT))
-            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
+        IndexMetadata indexMetadata = getIndexMetadata(backingIndexName);
 
         RollupStep step = createRandomInstance();
 

+ 41 - 7
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/UpdateRollupIndexPolicyStepTests.java

@@ -12,13 +12,20 @@ import org.elasticsearch.Version;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.admin.indices.settings.put.UpdateSettingsRequest;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
+import org.elasticsearch.cluster.ClusterState;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
+import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.xpack.core.ilm.AsyncActionStep.Listener;
 import org.elasticsearch.xpack.core.ilm.Step.StepKey;
 import org.mockito.Mockito;
 
+import java.util.Collections;
+import java.util.Map;
+
 import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.instanceOf;
+import static org.hamcrest.Matchers.is;
 
 public class UpdateRollupIndexPolicyStepTests extends AbstractStepMasterTimeoutTestCase<UpdateRollupIndexPolicyStep> {
 
@@ -62,13 +69,16 @@ public class UpdateRollupIndexPolicyStepTests extends AbstractStepMasterTimeoutT
 
     @Override
     protected IndexMetadata getIndexMetadata() {
-        return IndexMetadata.builder(randomAlphaOfLength(10)).settings(settings(Version.CURRENT))
-            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5)).build();
+        Map<String, String> ilmCustom = Collections.singletonMap("rollup_index_name", "rollup-index");
+        return IndexMetadata.builder(randomAlphaOfLength(10)).settings(
+            settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, "test-ilm-policy"))
+            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5))
+            .putCustom(LifecycleExecutionState.ILM_CUSTOM_METADATA_KEY, ilmCustom)
+            .build();
     }
 
     public void testPerformAction() {
         IndexMetadata indexMetadata = getIndexMetadata();
-        String rollupIndex = RollupStep.getRollupIndexName(indexMetadata.getIndex().getName());
         UpdateRollupIndexPolicyStep step = createRandomInstance();
         Settings settings = Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, step.getRollupPolicy()).build();
 
@@ -77,7 +87,7 @@ public class UpdateRollupIndexPolicyStepTests extends AbstractStepMasterTimeoutT
             @SuppressWarnings("unchecked")
             ActionListener<AcknowledgedResponse> listener = (ActionListener<AcknowledgedResponse>) invocation.getArguments()[1];
             assertThat(request.settings(), equalTo(settings));
-            assertThat(request.indices(), equalTo(new String[] { rollupIndex }));
+            assertThat(request.indices(), equalTo(new String[] { "rollup-index" }));
             listener.onResponse(AcknowledgedResponse.TRUE);
             return null;
         }).when(indicesClient).updateSettings(Mockito.any(), Mockito.any());
@@ -106,7 +116,8 @@ public class UpdateRollupIndexPolicyStepTests extends AbstractStepMasterTimeoutT
 
     public void testPerformActionFailure() {
         IndexMetadata indexMetadata = getIndexMetadata();
-        String rollupIndex = RollupStep.getRollupIndexName(indexMetadata.getIndex().getName());
+        ClusterState clusterState =
+            ClusterState.builder(emptyClusterState()).metadata(Metadata.builder().put(indexMetadata, true).build()).build();
         Exception exception = new RuntimeException();
         UpdateRollupIndexPolicyStep step = createRandomInstance();
         Settings settings = Settings.builder().put(LifecycleSettings.LIFECYCLE_NAME, step.getRollupPolicy()).build();
@@ -116,13 +127,13 @@ public class UpdateRollupIndexPolicyStepTests extends AbstractStepMasterTimeoutT
             @SuppressWarnings("unchecked")
             ActionListener<AcknowledgedResponse> listener = (ActionListener<AcknowledgedResponse>) invocation.getArguments()[1];
             assertThat(request.settings(), equalTo(settings));
-            assertThat(request.indices(), equalTo(new String[] { rollupIndex }));
+            assertThat(request.indices(), equalTo(new String[] { "rollup-index" }));
             listener.onFailure(exception);
             return null;
         }).when(indicesClient).updateSettings(Mockito.any(), Mockito.any());
 
         SetOnce<Boolean> exceptionThrown = new SetOnce<>();
-        step.performAction(indexMetadata, emptyClusterState(), null, new Listener() {
+        step.performAction(indexMetadata, clusterState, null, new Listener() {
 
             @Override
             public void onResponse(boolean complete) {
@@ -142,4 +153,27 @@ public class UpdateRollupIndexPolicyStepTests extends AbstractStepMasterTimeoutT
         Mockito.verify(adminClient, Mockito.only()).indices();
         Mockito.verify(indicesClient, Mockito.only()).updateSettings(Mockito.any(), Mockito.any());
     }
+
+    public void testPerformActionFailureIllegalExecutionState() {
+        IndexMetadata indexMetadata = IndexMetadata.builder(randomAlphaOfLength(10)).settings(
+            settings(Version.CURRENT).put(LifecycleSettings.LIFECYCLE_NAME, "test-ilm-policy"))
+            .numberOfShards(randomIntBetween(1, 5)).numberOfReplicas(randomIntBetween(0, 5))
+            .build();
+        String policyName = indexMetadata.getSettings().get(LifecycleSettings.LIFECYCLE_NAME);
+        String indexName = indexMetadata.getIndex().getName();
+        UpdateRollupIndexPolicyStep step = createRandomInstance();
+        step.performAction(indexMetadata, emptyClusterState(), null, new Listener() {
+            @Override
+            public void onResponse(boolean complete) {
+                fail("expecting a failure as the index doesn't have any rollup index name in its ILM execution state");
+            }
+
+            @Override
+            public void onFailure(Exception e) {
+                assertThat(e, instanceOf(IllegalStateException.class));
+                assertThat(e.getMessage(),
+                    is("rollup index name was not generated for policy [" + policyName + "] and index [" + indexName + "]"));
+            }
+        });
+    }
 }

+ 23 - 3
x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/RollupActionIT.java

@@ -7,21 +7,24 @@
 
 package org.elasticsearch.xpack.ilm.actions;
 
+import org.elasticsearch.client.Request;
+import org.elasticsearch.client.Response;
 import org.elasticsearch.cluster.metadata.IndexMetadata;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.search.aggregations.bucket.histogram.DateHistogramInterval;
 import org.elasticsearch.test.rest.ESRestTestCase;
 import org.elasticsearch.xpack.core.ilm.LifecycleSettings;
 import org.elasticsearch.xpack.core.ilm.RollupILMAction;
-import org.elasticsearch.xpack.core.ilm.RollupStep;
 import org.elasticsearch.xpack.core.rollup.RollupActionConfig;
 import org.elasticsearch.xpack.core.rollup.RollupActionDateHistogramGroupConfig;
 import org.elasticsearch.xpack.core.rollup.RollupActionGroupConfig;
 import org.elasticsearch.xpack.core.rollup.job.MetricConfig;
 import org.junit.Before;
 
+import java.io.IOException;
 import java.util.Collections;
 import java.util.Locale;
+import java.util.Map;
 
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createIndexWithSettings;
 import static org.elasticsearch.xpack.TimeSeriesRestDriver.createNewSingletonPolicy;
@@ -47,7 +50,6 @@ public class RollupActionIT extends ESRestTestCase {
     public void testRollupIndex() throws Exception {
         createIndexWithSettings(client(), index, alias, Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
             .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0));
-        String rollupIndex = RollupStep.getRollupIndexName(index);
         index(client(), index, "_id", "timestamp", "2020-01-01T05:10:00Z", "volume", 11.0);
         RollupActionConfig rollupConfig = new RollupActionConfig(
             new RollupActionGroupConfig(new RollupActionDateHistogramGroupConfig.FixedInterval("timestamp", DateHistogramInterval.DAY)),
@@ -56,6 +58,8 @@ public class RollupActionIT extends ESRestTestCase {
         createNewSingletonPolicy(client(), policy, "cold", new RollupILMAction(rollupConfig, null));
         updatePolicy(client(), index, policy);
 
+        assertBusy(() -> assertNotNull(getRollupIndexName(index)));
+        String rollupIndex = getRollupIndexName(index);
         assertBusy(() -> assertTrue(indexExists(rollupIndex)));
         assertBusy(() -> assertFalse(getOnlyIndexSettings(client(), rollupIndex).containsKey(LifecycleSettings.LIFECYCLE_NAME)));
         assertBusy(() -> assertTrue(indexExists(index)));
@@ -64,7 +68,6 @@ public class RollupActionIT extends ESRestTestCase {
     public void testRollupIndexAndSetNewRollupPolicy() throws Exception {
         createIndexWithSettings(client(), index, alias, Settings.builder().put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1)
             .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0));
-        String rollupIndex = RollupStep.ROLLUP_INDEX_NAME_PREFIX + index;
         index(client(), index, "_id", "timestamp", "2020-01-01T05:10:00Z", "volume", 11.0);
         RollupActionConfig rollupConfig = new RollupActionConfig(
             new RollupActionGroupConfig(new RollupActionDateHistogramGroupConfig.FixedInterval("timestamp", DateHistogramInterval.DAY)),
@@ -73,9 +76,26 @@ public class RollupActionIT extends ESRestTestCase {
         createNewSingletonPolicy(client(), policy, "cold", new RollupILMAction(rollupConfig, policy));
         updatePolicy(client(), index, policy);
 
+        assertBusy(() -> assertNotNull(getRollupIndexName(index)));
+        String rollupIndex = getRollupIndexName(index);
         assertBusy(() -> assertTrue(indexExists(rollupIndex)));
         assertBusy(() -> assertThat(getOnlyIndexSettings(client(), rollupIndex).get(LifecycleSettings.LIFECYCLE_NAME), equalTo(policy)));
         assertBusy(() -> assertTrue(indexExists(index)));
     }
 
+    /**
+     * gets the generated rollup index name for a given index by looking at newly created indices that match the rollup index name pattern
+     *
+     * @param index the name of the source index used to generate the rollup index name
+     * @return the name of the rollup index for a given index, null if none exist
+     * @throws IOException if request fails
+     */
+    private String getRollupIndexName(String index) throws IOException {
+        Response response = client().performRequest(new Request("GET", "/rollup-" + index + "-*"));
+        Map<String, Object> asMap = responseAsMap(response);
+        if (asMap.size() == 1) {
+            return (String) asMap.keySet().toArray()[0];
+        }
+        return null;
+    }
 }

+ 14 - 8
x-pack/plugin/rollup/src/main/java/org/elasticsearch/xpack/rollup/v2/TransportRollupAction.java

@@ -30,6 +30,8 @@ import org.elasticsearch.cluster.metadata.Metadata;
 import org.elasticsearch.cluster.metadata.RollupGroup;
 import org.elasticsearch.cluster.metadata.RollupMetadata;
 import org.elasticsearch.cluster.service.ClusterService;
+import org.elasticsearch.common.Randomness;
+import org.elasticsearch.common.UUIDs;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.time.WriteableZoneId;
@@ -84,7 +86,15 @@ public class TransportRollupAction extends AcknowledgedTransportMasterNodeAction
     protected void masterOperation(Task task, RollupAction.Request request, ClusterState state,
                                    ActionListener<AcknowledgedResponse> listener) {
         String originalIndexName = request.getSourceIndex();
-        String tmpIndexName = ".rolluptmp-" + request.getRollupIndex();
+
+        final String rollupIndexName;
+        if (request.getRollupIndex() == null) {
+            rollupIndexName = "rollup-" + originalIndexName + "-" + UUIDs.randomBase64UUID(Randomness.get());
+        } else {
+            rollupIndexName = request.getRollupIndex();
+        }
+
+        String tmpIndexName = ".rolluptmp-" + rollupIndexName;
 
         final XContentBuilder mapping;
         try {
@@ -118,7 +128,7 @@ public class TransportRollupAction extends AcknowledgedTransportMasterNodeAction
                         if (updateSettingsResponse.isAcknowledged()) {
                             client.admin().indices().resizeIndex(resizeRequest, ActionListener.wrap(resizeResponse -> {
                                 if (resizeResponse.isAcknowledged()) {
-                                    publishMetadata(request, originalIndexName, tmpIndexName, listener);
+                                    publishMetadata(request.getRollupConfig(), originalIndexName, tmpIndexName, rollupIndexName, listener);
                                 } else {
                                     deleteTmpIndex(originalIndexName, tmpIndexName, listener,
                                         new ElasticsearchException("Unable to resize temp rollup index [" + tmpIndexName + "]"));
@@ -197,7 +207,7 @@ public class TransportRollupAction extends AcknowledgedTransportMasterNodeAction
         return builder.endObject();
     }
 
-    private void publishMetadata(RollupAction.Request request, String originalIndexName, String tmpIndexName,
+    private void publishMetadata(RollupActionConfig config, String originalIndexName, String tmpIndexName, String rollupIndexName,
                                  ActionListener<AcknowledgedResponse> listener) {
         // update Rollup metadata to include this index
         clusterService.submitStateUpdateTask("update-rollup-metadata", new ClusterStateUpdateTask() {
@@ -209,12 +219,8 @@ public class TransportRollupAction extends AcknowledgedTransportMasterNodeAction
 
             @Override
             public ClusterState execute(ClusterState currentState) {
-                String rollupIndexName = request.getRollupIndex();
                 IndexMetadata rollupIndexMetadata = currentState.getMetadata().index(rollupIndexName);
                 Index rollupIndex = rollupIndexMetadata.getIndex();
-                // TODO(talevy): find better spot to get the original index name
-                // extract created rollup index original index name to be used as metadata key
-                String originalIndexName = request.getSourceIndex();
                 Map<String, String> idxMetadata = currentState.getMetadata().index(originalIndexName)
                     .getCustomData(RollupMetadata.TYPE);
                 String rollupGroupKeyName = (idxMetadata == null) ?
@@ -228,7 +234,7 @@ public class TransportRollupAction extends AcknowledgedTransportMasterNodeAction
                 } else {
                     rollupGroups = new HashMap<>(rollupMetadata.rollupGroups());
                 }
-                RollupActionDateHistogramGroupConfig dateConfig = request.getRollupConfig().getGroupConfig().getDateHistogram();
+                RollupActionDateHistogramGroupConfig dateConfig = config.getGroupConfig().getDateHistogram();
                 WriteableZoneId rollupDateZoneId = WriteableZoneId.of(dateConfig.getTimeZone());
                 if (rollupGroups.containsKey(rollupGroupKeyName)) {
                     RollupGroup group = rollupGroups.get(rollupGroupKeyName);