From af86b1020813665ac4f7604dcd34adda8b20d8d9 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 10 Feb 2021 19:15:23 +0000 Subject: [PATCH 1/6] ILM: searchable snapshot executes before migrate in cold/frozen This moves the execution of the `searchable_snapshot` action before the `migrate` action in the `cold` and `frozen` phases for a more efficient data migration (ie. mounting it as a searchable snapshot directly on the target tier) --- .../xpack/core/ilm/MigrateAction.java | 23 ++++-- .../core/ilm/TimeseriesLifecycleType.java | 17 +++-- .../xpack/core/ilm/MigrateActionTests.java | 72 ++++++++++++++++--- .../ilm/TimeseriesLifecycleTypeTests.java | 8 +++ .../actions/SearchableSnapshotActionIT.java | 61 ++++++++++++++++ 5 files changed, 161 insertions(+), 20 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java index a59306400d245..6ea4d759ce56e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.core.ilm; import org.elasticsearch.client.Client; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.ParseField; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -25,6 +26,8 @@ import java.util.Objects; import java.util.stream.Collectors; +import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.FROZEN_PHASE; + /** * A {@link LifecycleAction} which enables or disables the automatic migration of data between * {@link org.elasticsearch.xpack.core.DataTier}s. @@ -33,6 +36,7 @@ public class MigrateAction implements LifecycleAction { public static final String NAME = "migrate"; public static final ParseField ENABLED_FIELD = new ParseField("enabled"); + static final String CONDITIONAL_SKIP_MIGRATE_STEP = BranchingStep.NAME + "-check-existing-routing"; // Represents an ordered list of data tiers from frozen to hot (or slow to fast) private static final List FROZEN_TO_HOT_TIERS = List.of(DataTier.DATA_FROZEN, DataTier.DATA_COLD, DataTier.DATA_WARM, DataTier.DATA_HOT); @@ -92,22 +96,33 @@ public boolean isSafeAction() { @Override public List toSteps(Client client, String phase, StepKey nextStepKey) { if (enabled) { + StepKey preMigrateBranchingKey = new StepKey(phase, NAME, CONDITIONAL_SKIP_MIGRATE_STEP); StepKey migrationKey = new StepKey(phase, NAME, NAME); StepKey migrationRoutedKey = new StepKey(phase, NAME, DataTierMigrationRoutedStep.NAME); Settings.Builder migrationSettings = Settings.builder(); - String dataTierName = "data_" + phase; - assert DataTier.validTierName(dataTierName) : "invalid data tier name:" + dataTierName; - migrationSettings.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getPreferredTiersConfiguration(dataTierName)); + String targetTier = "data_" + phase; + assert DataTier.validTierName(targetTier) : "invalid data tier name:" + targetTier; + + BranchingStep conditionalSkipActionStep = new BranchingStep(preMigrateBranchingKey, migrationKey, nextStepKey, + (index, clusterState) -> skipMigrateAction(phase, clusterState.metadata().index(index))); + migrationSettings.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getPreferredTiersConfiguration(targetTier)); UpdateSettingsStep updateMigrationSettingStep = new UpdateSettingsStep(migrationKey, migrationRoutedKey, client, migrationSettings.build()); DataTierMigrationRoutedStep migrationRoutedStep = new DataTierMigrationRoutedStep(migrationRoutedKey, nextStepKey); - return Arrays.asList(updateMigrationSettingStep, migrationRoutedStep); + return Arrays.asList(conditionalSkipActionStep, updateMigrationSettingStep, migrationRoutedStep); } else { return List.of(); } } + static boolean skipMigrateAction(String phase, IndexMetadata indexMetadata) { + // if the index is a searchable snapshot we skip the migrate action (as mounting an index as searchable snapshot + // configures the tier allocation preference), unless we're in the frozen phase + return (indexMetadata.getSettings().get(LifecycleSettings.SNAPSHOT_INDEX_NAME) != null) + && (phase.equals(FROZEN_PHASE) == false); + } + /** * Based on the provided target tier it will return a comma separated list of preferred tiers. * ie. if `data_cold` is the target tier, it will return `data_cold,data_warm,data_hot` diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java index 08712b51c46a1..17ec6c7a1821c 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java @@ -47,8 +47,8 @@ public class TimeseriesLifecycleType implements LifecycleType { static final List ORDERED_VALID_WARM_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, ReadOnlyAction.NAME, AllocateAction.NAME, MigrateAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME); static final List ORDERED_VALID_COLD_ACTIONS; - static final List ORDERED_VALID_FROZEN_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, ReadOnlyAction.NAME, - AllocateAction.NAME, MigrateAction.NAME, FreezeAction.NAME, SearchableSnapshotAction.NAME); + static final List ORDERED_VALID_FROZEN_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, + ReadOnlyAction.NAME, SearchableSnapshotAction.NAME, AllocateAction.NAME, MigrateAction.NAME, FreezeAction.NAME); static final List ORDERED_VALID_DELETE_ACTIONS = Arrays.asList(WaitForSnapshotAction.NAME, DeleteAction.NAME); static final Set VALID_HOT_ACTIONS; static final Set VALID_WARM_ACTIONS = Sets.newHashSet(ORDERED_VALID_WARM_ACTIONS); @@ -67,13 +67,13 @@ public class TimeseriesLifecycleType implements LifecycleType { if (RollupV2.isEnabled()) { ORDERED_VALID_HOT_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, RolloverAction.NAME, ReadOnlyAction.NAME, RollupILMAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME, SearchableSnapshotAction.NAME); - ORDERED_VALID_COLD_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, AllocateAction.NAME, - MigrateAction.NAME, FreezeAction.NAME, RollupILMAction.NAME, SearchableSnapshotAction.NAME); + ORDERED_VALID_COLD_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, SearchableSnapshotAction.NAME, + AllocateAction.NAME, MigrateAction.NAME, FreezeAction.NAME, RollupILMAction.NAME); } else { ORDERED_VALID_HOT_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, RolloverAction.NAME, ReadOnlyAction.NAME, ShrinkAction.NAME, ForceMergeAction.NAME, SearchableSnapshotAction.NAME); - ORDERED_VALID_COLD_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, AllocateAction.NAME, - MigrateAction.NAME, FreezeAction.NAME, SearchableSnapshotAction.NAME); + ORDERED_VALID_COLD_ACTIONS = Arrays.asList(SetPriorityAction.NAME, UnfollowAction.NAME, SearchableSnapshotAction.NAME, + AllocateAction.NAME, MigrateAction.NAME, FreezeAction.NAME); } VALID_HOT_ACTIONS = Sets.newHashSet(ORDERED_VALID_HOT_ACTIONS); VALID_COLD_ACTIONS = Sets.newHashSet(ORDERED_VALID_COLD_ACTIONS); @@ -133,6 +133,11 @@ static boolean shouldInjectMigrateStepForPhase(Phase phase) { } } + if (phase.getActions().get(SearchableSnapshotAction.NAME) != null) { + // the `searchable_snapshot` action defines migration rules itself, so no need to inject a migrate action + return false; + } + MigrateAction migrateAction = (MigrateAction) phase.getActions().get(MigrateAction.NAME); // if the user configured the {@link MigrateAction} already we won't automatically configure it return migrateAction == null; diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/MigrateActionTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/MigrateActionTests.java index 3b3af017e9b02..be26acdd97a73 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/MigrateActionTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/MigrateActionTests.java @@ -6,20 +6,29 @@ */ package org.elasticsearch.xpack.core.ilm; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.io.stream.Writeable.Reader; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider; import org.elasticsearch.xpack.core.ilm.Step.StepKey; import java.io.IOException; +import java.util.Arrays; import java.util.List; +import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_REPLICAS; +import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_NUMBER_OF_SHARDS; +import static org.elasticsearch.cluster.metadata.IndexMetadata.SETTING_VERSION_CREATED; import static org.elasticsearch.xpack.core.DataTier.DATA_COLD; import static org.elasticsearch.xpack.core.DataTier.DATA_HOT; import static org.elasticsearch.xpack.core.DataTier.DATA_WARM; import static org.elasticsearch.xpack.core.ilm.MigrateAction.getPreferredTiersConfiguration; +import static org.elasticsearch.xpack.core.ilm.MigrateAction.skipMigrateAction; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.COLD_PHASE; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.DELETE_PHASE; +import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.FROZEN_PHASE; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.HOT_PHASE; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.WARM_PHASE; import static org.hamcrest.CoreMatchers.is; @@ -49,15 +58,18 @@ public void testToSteps() { MigrateAction action = new MigrateAction(); List steps = action.toSteps(null, phase, nextStepKey); assertNotNull(steps); - assertEquals(2, steps.size()); - StepKey expectedFirstStepKey = new StepKey(phase, MigrateAction.NAME, MigrateAction.NAME); - StepKey expectedSecondStepKey = new StepKey(phase, MigrateAction.NAME, DataTierMigrationRoutedStep.NAME); - UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0); - DataTierMigrationRoutedStep secondStep = (DataTierMigrationRoutedStep) steps.get(1); + assertEquals(3, steps.size()); + StepKey expectedFirstStepKey = new StepKey(phase, MigrateAction.NAME, MigrateAction.CONDITIONAL_SKIP_MIGRATE_STEP); + StepKey expectedSecondStepKey = new StepKey(phase, MigrateAction.NAME, MigrateAction.NAME); + StepKey expectedThirdStepKey = new StepKey(phase, MigrateAction.NAME, DataTierMigrationRoutedStep.NAME); + BranchingStep firstStep = (BranchingStep) steps.get(0); + UpdateSettingsStep secondStep = (UpdateSettingsStep) steps.get(1); + DataTierMigrationRoutedStep thirdStep = (DataTierMigrationRoutedStep) steps.get(2); assertEquals(expectedFirstStepKey, firstStep.getKey()); - assertEquals(expectedSecondStepKey, firstStep.getNextStepKey()); assertEquals(expectedSecondStepKey, secondStep.getKey()); - assertEquals(nextStepKey, secondStep.getNextStepKey()); + assertEquals(expectedThirdStepKey, secondStep.getNextStepKey()); + assertEquals(expectedThirdStepKey, thirdStep.getKey()); + assertEquals(nextStepKey, thirdStep.getNextStepKey()); } { @@ -81,21 +93,61 @@ public void testMigrateActionsConfiguresTierPreference() { MigrateAction action = new MigrateAction(); { List steps = action.toSteps(null, HOT_PHASE, nextStepKey); - UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0); + UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1); assertThat(DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING.get(firstStep.getSettings()), is(DATA_HOT)); } { List steps = action.toSteps(null, WARM_PHASE, nextStepKey); - UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0); + UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1); assertThat(DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING.get(firstStep.getSettings()), is(DATA_WARM + "," + DATA_HOT)); } { List steps = action.toSteps(null, COLD_PHASE, nextStepKey); - UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0); + UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(1); assertThat(DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING.get(firstStep.getSettings()), is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT)); } } + + public void testSkipMigrateAction() { + IndexMetadata snappedIndex = IndexMetadata.builder("snapped_index") + .settings( + Settings.builder() + .put(LifecycleSettings.SNAPSHOT_INDEX_NAME, "snapped") + .put(SETTING_VERSION_CREATED, Version.CURRENT) + .put(SETTING_NUMBER_OF_SHARDS, 1) + .put(SETTING_NUMBER_OF_REPLICAS, 0) + ) + .build(); + + IndexMetadata regularIndex = IndexMetadata.builder("regular_index") + .settings( + Settings.builder() + .put(SETTING_VERSION_CREATED, Version.CURRENT) + .put(SETTING_NUMBER_OF_SHARDS, 1) + .put(SETTING_NUMBER_OF_REPLICAS, 0) + ) + .build(); + + { + // migrate action is not skipped if the index is not a searchable snapshot + Arrays.asList(HOT_PHASE, WARM_PHASE, COLD_PHASE, FROZEN_PHASE) + .forEach(phase -> assertThat(skipMigrateAction(phase, regularIndex), is(false))); + } + + { + // migrate action is skipped if the index is a searchable snapshot for phases hot -> cold + Arrays.asList(HOT_PHASE, WARM_PHASE, COLD_PHASE) + .forEach(phase -> assertThat(skipMigrateAction(phase, snappedIndex), is(true))); + } + + { + // migrate action is never skipped for the frozen phase + assertThat(skipMigrateAction(FROZEN_PHASE, snappedIndex), is(false)); + assertThat(skipMigrateAction(FROZEN_PHASE, regularIndex), is(false)); + } + } + } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java index 763e389549ea4..1249b69a602a9 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java @@ -590,6 +590,14 @@ public void testShouldMigrateDataToTiers() { Phase phase = new Phase(WARM_PHASE, TimeValue.ZERO, actions); assertThat(TimeseriesLifecycleType.shouldInjectMigrateStepForPhase(phase), is(false)); } + + { + // test phase defines a `searchable_snapshot` action + Map actions = new HashMap<>(); + actions.put(TEST_SEARCHABLE_SNAPSHOT_ACTION.getWriteableName(), TEST_SEARCHABLE_SNAPSHOT_ACTION); + Phase phase = new Phase(COLD_PHASE, TimeValue.ZERO, actions); + assertThat(TimeseriesLifecycleType.shouldInjectMigrateStepForPhase(phase), is(false)); + } } private void assertNextActionName(String phaseName, String currentAction, String expectedNextAction, String... availableActionNames) { diff --git a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java index a3b657e3cb9fb..676160a09b2d0 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/SearchableSnapshotActionIT.java @@ -21,12 +21,14 @@ import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.test.rest.ESRestTestCase; +import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider; import org.elasticsearch.xpack.core.ilm.DeleteAction; import org.elasticsearch.xpack.core.ilm.ForceMergeAction; import org.elasticsearch.xpack.core.ilm.FreezeAction; import org.elasticsearch.xpack.core.ilm.LifecycleAction; import org.elasticsearch.xpack.core.ilm.LifecyclePolicy; import org.elasticsearch.xpack.core.ilm.LifecycleSettings; +import org.elasticsearch.xpack.core.ilm.MigrateAction; import org.elasticsearch.xpack.core.ilm.Phase; import org.elasticsearch.xpack.core.ilm.PhaseCompleteStep; import org.elasticsearch.xpack.core.ilm.RolloverAction; @@ -39,6 +41,7 @@ import java.io.IOException; import java.io.InputStream; +import java.time.ZonedDateTime; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -558,4 +561,62 @@ public void testSecondSearchableSnapshotChangesRepo() throws Exception { ((List) responseMap.get("responses")).get(0)).get("snapshots")).size(), equalTo(1)); } } + + public void testSearchableSnapshotActionOverridesMigrateAction() throws Exception { + createSnapshotRepo(client(), snapshotRepo, randomBoolean()); + createPolicy(client(), policy, + new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, new RolloverAction(null, null, 1L), + SearchableSnapshotAction.NAME, new SearchableSnapshotAction( + snapshotRepo, randomBoolean(), MountSearchableSnapshotRequest.Storage.FULL_COPY)) + ), + new Phase("warm", TimeValue.ZERO, Map.of(MigrateAction.NAME, new MigrateAction(true))), + // this time transition condition will make sure we catch ILM in the warm phase so we can assert the warm migrate action + // didn't re-configure the tier allocation settings set by the searchable_snapshot action in the hot phase + // we'll use the origination date to kick off ILM to complete the policy + new Phase("cold", TimeValue.timeValueDays(5L), Map.of(MigrateAction.NAME, new MigrateAction(true))), + new Phase("frozen", TimeValue.ZERO, Map.of(MigrateAction.NAME, new MigrateAction(true))), + null + ); + + createComposableTemplate(client(), randomAlphaOfLengthBetween(5, 10).toLowerCase(), dataStream, + new Template(Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(LifecycleSettings.LIFECYCLE_NAME, policy) + .build(), null, null) + ); + + indexDocument(client(), dataStream, true); + String firstGenIndex = DataStream.getDefaultBackingIndexName(dataStream, 1L); + Map indexSettings = getIndexSettingsAsMap(firstGenIndex); + assertThat(indexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), is("data_hot")); + + // rollover the data stream so searchable_snapshot can complete + rolloverMaxOneDocCondition(client(), dataStream); + + final String restoredIndex = SearchableSnapshotAction.FULL_RESTORED_INDEX_PREFIX + firstGenIndex; + assertBusy(() -> { + logger.info("--> waiting for [{}] to exist...", restoredIndex); + assertTrue(indexExists(restoredIndex)); + }, 30, TimeUnit.SECONDS); + assertBusy(() -> assertThat(getStepKeyForIndex(client(), restoredIndex), is(PhaseCompleteStep.finalStep("warm").getKey())), + 30, TimeUnit.SECONDS); + + Map warmIndexSettings = getIndexSettingsAsMap(restoredIndex); + // the warm phase shouldn't have changed the data_cold -> data_hot configuration + assertThat(warmIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), + is("data_cold,data_warm,data_hot")); + + // make the index 100 days old so the cold phase transition timing passes + updateIndexSettings(restoredIndex, Settings.builder().put(LifecycleSettings.LIFECYCLE_ORIGINATION_DATE, + ZonedDateTime.now().toInstant().toEpochMilli() - TimeValue.timeValueDays(100).getMillis())); + + // let's wait for ILM to finish + assertBusy(() -> assertThat(getStepKeyForIndex(client(), restoredIndex), is(PhaseCompleteStep.finalStep("frozen").getKey()))); + + Map frozenIndexSettings = getIndexSettingsAsMap(restoredIndex); + // the frozen phase should've reconfigured the allocation preference + assertThat(frozenIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), + is("data_frozen,data_cold,data_warm,data_hot")); + } + } From 7c3ffd209561f042c6fc77d60c0b20b9cedb77e1 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 11 Feb 2021 10:39:27 +0000 Subject: [PATCH 2/6] Inject a migrate action in the frozen phase --- .../xpack/core/ilm/TimeseriesLifecycleType.java | 6 ++++-- .../xpack/core/ilm/TimeseriesLifecycleTypeTests.java | 10 ++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java index 17ec6c7a1821c..e55a9f6535dc0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleType.java @@ -133,8 +133,10 @@ static boolean shouldInjectMigrateStepForPhase(Phase phase) { } } - if (phase.getActions().get(SearchableSnapshotAction.NAME) != null) { - // the `searchable_snapshot` action defines migration rules itself, so no need to inject a migrate action + if (phase.getActions().get(SearchableSnapshotAction.NAME) != null && phase.getName().equals(FROZEN_PHASE) == false) { + // the `searchable_snapshot` action defines migration rules itself, so no need to inject a migrate action, unless we're in the + // frozen phase (as the migrate action would also include the `data_frozen` role which is not guaranteed to be included by all + // types of searchable snapshots) return false; } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java index 1249b69a602a9..5e7e40e2162f1 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/TimeseriesLifecycleTypeTests.java @@ -28,6 +28,7 @@ import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.ACTIONS_CANNOT_FOLLOW_SEARCHABLE_SNAPSHOT; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.COLD_PHASE; +import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.FROZEN_PHASE; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.HOT_PHASE; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.ORDERED_VALID_COLD_ACTIONS; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.ORDERED_VALID_DELETE_ACTIONS; @@ -598,6 +599,15 @@ public void testShouldMigrateDataToTiers() { Phase phase = new Phase(COLD_PHASE, TimeValue.ZERO, actions); assertThat(TimeseriesLifecycleType.shouldInjectMigrateStepForPhase(phase), is(false)); } + + { + // test `frozen` phase defines a `searchable_snapshot` action + Map actions = new HashMap<>(); + actions.put(TEST_SEARCHABLE_SNAPSHOT_ACTION.getWriteableName(), TEST_SEARCHABLE_SNAPSHOT_ACTION); + Phase phase = new Phase(FROZEN_PHASE, TimeValue.ZERO, actions); + assertThat(TimeseriesLifecycleType.shouldInjectMigrateStepForPhase(phase), is(true)); + } + } private void assertNextActionName(String phaseName, String currentAction, String expectedNextAction, String... availableActionNames) { From 420a2ce73bd4a63b4f1eee9371f24dda0550b6c8 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 11 Feb 2021 10:59:51 +0000 Subject: [PATCH 3/6] Log when skipping the migrate action --- .../xpack/core/ilm/MigrateAction.java | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java index 6ea4d759ce56e..8896483d48418 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java @@ -6,6 +6,8 @@ */ package org.elasticsearch.xpack.core.ilm; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.elasticsearch.client.Client; import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.common.ParseField; @@ -36,6 +38,7 @@ public class MigrateAction implements LifecycleAction { public static final String NAME = "migrate"; public static final ParseField ENABLED_FIELD = new ParseField("enabled"); + private static final Logger logger = LogManager.getLogger(MigrateAction.class); static final String CONDITIONAL_SKIP_MIGRATE_STEP = BranchingStep.NAME + "-check-existing-routing"; // Represents an ordered list of data tiers from frozen to hot (or slow to fast) private static final List FROZEN_TO_HOT_TIERS = @@ -105,7 +108,18 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { assert DataTier.validTierName(targetTier) : "invalid data tier name:" + targetTier; BranchingStep conditionalSkipActionStep = new BranchingStep(preMigrateBranchingKey, migrationKey, nextStepKey, - (index, clusterState) -> skipMigrateAction(phase, clusterState.metadata().index(index))); + (index, clusterState) -> { + if (skipMigrateAction(phase, clusterState.metadata().index(index))) { + String policyName = + LifecycleSettings.LIFECYCLE_NAME_SETTING.get(clusterState.metadata().index(index).getSettings()); + logger.debug("[{}] action is configured for index [{}] in policy [{}] which is already mounted as a searchable " + + "snapshot. skipping this action", MigrateAction.NAME, index.getName(), policyName); + return true; + } + + // don't skip the migrate action as the index is not mounted as searchable snapshot or we're in the frozen phase + return false; + }); migrationSettings.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getPreferredTiersConfiguration(targetTier)); UpdateSettingsStep updateMigrationSettingStep = new UpdateSettingsStep(migrationKey, migrationRoutedKey, client, migrationSettings.build()); From 51ba7c7df84f51332c7e1825b827c4ddab719d4e Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 11 Feb 2021 11:51:01 +0000 Subject: [PATCH 4/6] Docs: migrate auto-injection relation to searchable snapshots --- docs/reference/ilm/actions/ilm-migrate.asciidoc | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/docs/reference/ilm/actions/ilm-migrate.asciidoc b/docs/reference/ilm/actions/ilm-migrate.asciidoc index ecf44958bdc3a..b5793aa9904d5 100644 --- a/docs/reference/ilm/actions/ilm-migrate.asciidoc +++ b/docs/reference/ilm/actions/ilm-migrate.asciidoc @@ -14,6 +14,11 @@ replicas, {ilm-init} reduces the number of replicas before migrating the index. To prevent automatic migration without specifying allocation options, you can explicitly include the migrate action and set the enabled option to `false`. +If the `cold` phase defines a <> the `migrate` +action will not be injected automatically in the `cold` phase because the managed index will be +mounted directly on the target tier using the same <> +infrastructure the `migrate` actions configures. + In the warm phase, the `migrate` action sets <> to `data_warm,data_hot`. This moves the index to nodes in the <>. If there are no nodes in the warm tier, it falls back to the From 89a47249b366edc096b089508242e2beeddf6650 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 11 Feb 2021 13:16:09 +0000 Subject: [PATCH 5/6] Change migrate skip name to be more meaningful --- .../java/org/elasticsearch/xpack/core/ilm/MigrateAction.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java index 8896483d48418..a31eb4d8274ac 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MigrateAction.java @@ -39,7 +39,7 @@ public class MigrateAction implements LifecycleAction { public static final ParseField ENABLED_FIELD = new ParseField("enabled"); private static final Logger logger = LogManager.getLogger(MigrateAction.class); - static final String CONDITIONAL_SKIP_MIGRATE_STEP = BranchingStep.NAME + "-check-existing-routing"; + static final String CONDITIONAL_SKIP_MIGRATE_STEP = BranchingStep.NAME + "-check-skip-action"; // Represents an ordered list of data tiers from frozen to hot (or slow to fast) private static final List FROZEN_TO_HOT_TIERS = List.of(DataTier.DATA_FROZEN, DataTier.DATA_COLD, DataTier.DATA_WARM, DataTier.DATA_HOT); From 5300b2fa7ec21c1336fd65c856dfa52047c966b3 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 11 Feb 2021 13:21:46 +0000 Subject: [PATCH 6/6] Fix transition for mounted index from searchable_snapshot to another action Not that searchable_snapshot can precede other actions in the same phase (eg. in frozen it is followed by `migrate`) we need to allow the mounted index to resume executing the ILM policy starting with a step that's part of a new action (ie. migrate). This adds support to resume the execution of the mounted index from another action. With older versions the execution would resume from the PhaseCompleteStep as it was the last action in a phase, which was handled as a special case in the `CopyExecutionStateStep`. This commit generalises the `CopyExecutionStateStep` to be able to resume from any `StepKey`. --- .../core/ilm/CopyExecutionStateStep.java | 30 ++++++++----------- .../core/ilm/SearchableSnapshotAction.java | 4 +-- .../xpack/core/ilm/ShrinkAction.java | 2 +- .../core/ilm/CopyExecutionStateStepTests.java | 20 +++++++------ 4 files changed, 26 insertions(+), 30 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java index ae12edb2fe16b..206eb95656542 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStep.java @@ -21,8 +21,7 @@ /** * Copies the execution state data from one index to another, typically after a * new index has been created. As part of the execution state copy it will set the target index - * "current step" to the provided step name (part of the same phase and action as the current step's, unless - * the "complete" step is configured in which case the action will be changed to "complete" as well) + * "current step" to the provided target next step {@link org.elasticsearch.xpack.core.ilm.Step.StepKey}. * * Useful for actions such as shrink. */ @@ -32,20 +31,20 @@ public class CopyExecutionStateStep extends ClusterStateActionStep { private static final Logger logger = LogManager.getLogger(CopyExecutionStateStep.class); private final String targetIndexPrefix; - private final String targetNextStepName; + private final StepKey targetNextStepKey; - public CopyExecutionStateStep(StepKey key, StepKey nextStepKey, String targetIndexPrefix, String targetNextStepName) { + public CopyExecutionStateStep(StepKey key, StepKey nextStepKey, String targetIndexPrefix, StepKey targetNextStepKey) { super(key, nextStepKey); this.targetIndexPrefix = targetIndexPrefix; - this.targetNextStepName = targetNextStepName; + this.targetNextStepKey = targetNextStepKey; } String getTargetIndexPrefix() { return targetIndexPrefix; } - String getTargetNextStepName() { - return targetNextStepName; + StepKey getTargetNextStepKey() { + return targetNextStepKey; } @Override @@ -69,20 +68,17 @@ public ClusterState performAction(Index index, ClusterState clusterState) { "] to [" + targetIndexName + "] as target index does not exist"); } + String phase = targetNextStepKey.getPhase(); + String action = targetNextStepKey.getAction(); + String step = targetNextStepKey.getName(); LifecycleExecutionState lifecycleState = LifecycleExecutionState.fromIndexMetadata(indexMetadata); - String phase = lifecycleState.getPhase(); - String action = lifecycleState.getAction(); long lifecycleDate = lifecycleState.getLifecycleDate(); LifecycleExecutionState.Builder relevantTargetCustomData = LifecycleExecutionState.builder(); relevantTargetCustomData.setIndexCreationDate(lifecycleDate); + relevantTargetCustomData.setAction(action); relevantTargetCustomData.setPhase(phase); - relevantTargetCustomData.setStep(targetNextStepName); - if (targetNextStepName.equals(PhaseCompleteStep.NAME)) { - relevantTargetCustomData.setAction(PhaseCompleteStep.NAME); - } else { - relevantTargetCustomData.setAction(action); - } + relevantTargetCustomData.setStep(step); relevantTargetCustomData.setSnapshotRepository(lifecycleState.getSnapshotRepository()); relevantTargetCustomData.setSnapshotName(lifecycleState.getSnapshotName()); relevantTargetCustomData.setSnapshotIndexName(lifecycleState.getSnapshotIndexName()); @@ -107,11 +103,11 @@ public boolean equals(Object o) { } CopyExecutionStateStep that = (CopyExecutionStateStep) o; return Objects.equals(targetIndexPrefix, that.targetIndexPrefix) && - Objects.equals(targetNextStepName, that.targetNextStepName); + Objects.equals(targetNextStepKey, that.targetNextStepKey); } @Override public int hashCode() { - return Objects.hash(super.hashCode(), targetIndexPrefix, targetNextStepName); + return Objects.hash(super.hashCode(), targetIndexPrefix, targetNextStepKey); } } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java index cef440c877388..2a03029a59d08 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SearchableSnapshotAction.java @@ -245,10 +245,8 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { client, getRestoredIndexPrefix(mountSnapshotKey), getConcreteStorageType(mountSnapshotKey)); WaitForIndexColorStep waitForGreenIndexHealthStep = new WaitForIndexColorStep(waitForGreenRestoredIndexKey, copyMetadataKey, ClusterHealthStatus.GREEN, getRestoredIndexPrefix(waitForGreenRestoredIndexKey)); - // a policy with only the cold phase will have a null "nextStepKey", hence the "null" nextStepKey passed in below when that's the - // case CopyExecutionStateStep copyMetadataStep = new CopyExecutionStateStep(copyMetadataKey, copyLifecyclePolicySettingKey, - getRestoredIndexPrefix(copyMetadataKey), nextStepKey != null ? nextStepKey.getName() : "null"); + getRestoredIndexPrefix(copyMetadataKey), nextStepKey); CopySettingsStep copySettingsStep = new CopySettingsStep(copyLifecyclePolicySettingKey, dataStreamCheckBranchingKey, getRestoredIndexPrefix(copyLifecyclePolicySettingKey), LifecycleSettings.LIFECYCLE_NAME); BranchingStep isDataStreamBranchingStep = new BranchingStep(dataStreamCheckBranchingKey, swapAliasesKey, replaceDataStreamIndexKey, diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrinkAction.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrinkAction.java index 0d3d9b31c1f5e..398f276a1ceb3 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrinkAction.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/ShrinkAction.java @@ -184,7 +184,7 @@ public List toSteps(Client client, String phase, Step.StepKey nextStepKey) SHRUNKEN_INDEX_PREFIX); ShrunkShardsAllocatedStep allocated = new ShrunkShardsAllocatedStep(enoughShardsKey, copyMetadataKey, SHRUNKEN_INDEX_PREFIX); CopyExecutionStateStep copyMetadata = new CopyExecutionStateStep(copyMetadataKey, dataStreamCheckBranchingKey, - SHRUNKEN_INDEX_PREFIX, ShrunkenIndexCheckStep.NAME); + SHRUNKEN_INDEX_PREFIX, isShrunkIndexKey); // by the time we get to this step we have 2 indices, the source and the shrunken one. we now need to choose an index // swapping strategy such that the shrunken index takes the place of the source index (which is also deleted). // if the source index is part of a data stream it's a matter of replacing it with the shrunken index one in the data stream and diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStepTests.java index 05bb54d19071d..3af56dcfe3bbf 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/CopyExecutionStateStepTests.java @@ -26,8 +26,8 @@ protected CopyExecutionStateStep createRandomInstance() { StepKey stepKey = randomStepKey(); StepKey nextStepKey = randomStepKey(); String shrunkIndexPrefix = randomAlphaOfLength(10); - String nextStepName = randomStepKey().getName(); - return new CopyExecutionStateStep(stepKey, nextStepKey, shrunkIndexPrefix, nextStepName); + StepKey targetNextStepKey = randomStepKey(); + return new CopyExecutionStateStep(stepKey, nextStepKey, shrunkIndexPrefix, targetNextStepKey); } @Override @@ -35,7 +35,7 @@ protected CopyExecutionStateStep mutateInstance(CopyExecutionStateStep instance) StepKey key = instance.getKey(); StepKey nextKey = instance.getNextStepKey(); String shrunkIndexPrefix = instance.getTargetIndexPrefix(); - String nextStepName = instance.getTargetNextStepName(); + StepKey targetNextStepKey = instance.getTargetNextStepKey(); switch (between(0, 2)) { case 0: @@ -48,19 +48,20 @@ protected CopyExecutionStateStep mutateInstance(CopyExecutionStateStep instance) shrunkIndexPrefix += randomAlphaOfLength(5); break; case 3: - nextStepName = randomAlphaOfLengthBetween(1, 10); + targetNextStepKey = new StepKey(targetNextStepKey.getPhase(), targetNextStepKey.getAction(), + targetNextStepKey.getName() + randomAlphaOfLength(5)); break; default: throw new AssertionError("Illegal randomisation branch"); } - return new CopyExecutionStateStep(key, nextKey, shrunkIndexPrefix, nextStepName); + return new CopyExecutionStateStep(key, nextKey, shrunkIndexPrefix, targetNextStepKey); } @Override protected CopyExecutionStateStep copyInstance(CopyExecutionStateStep instance) { return new CopyExecutionStateStep(instance.getKey(), instance.getNextStepKey(), instance.getTargetIndexPrefix(), - instance.getTargetNextStepName()); + instance.getTargetNextStepKey()); } public void testPerformAction() { @@ -89,10 +90,11 @@ public void testPerformAction() { LifecycleExecutionState newIndexData = LifecycleExecutionState .fromIndexMetadata(newClusterState.metadata().index(step.getTargetIndexPrefix() + indexName)); + StepKey targetNextStepKey = step.getTargetNextStepKey(); assertEquals(newIndexData.getLifecycleDate(), oldIndexData.getLifecycleDate()); - assertEquals(newIndexData.getPhase(), oldIndexData.getPhase()); - assertEquals(newIndexData.getAction(), oldIndexData.getAction()); - assertEquals(newIndexData.getStep(), step.getTargetNextStepName()); + assertEquals(newIndexData.getPhase(), targetNextStepKey.getPhase()); + assertEquals(newIndexData.getAction(), targetNextStepKey.getAction()); + assertEquals(newIndexData.getStep(), targetNextStepKey.getName()); assertEquals(newIndexData.getSnapshotRepository(), oldIndexData.getSnapshotRepository()); assertEquals(newIndexData.getSnapshotName(), oldIndexData.getSnapshotName()); }