From 4cf4555cac34ff05e370757a39c156c6dccd1276 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Thu, 29 Apr 2021 16:24:43 -0400 Subject: [PATCH 01/13] Whitespace --- .../actions/SearchableSnapshotActionIT.java | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) 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 abc50cd065618..78ae1101bb523 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 @@ -161,8 +161,8 @@ public void testSearchableSnapshotForceMergesIndexToOneSegment() throws Exceptio TimeUnit.SECONDS); } - @SuppressWarnings("unchecked") - public void testDeleteActionDeletesSearchableSnapshot() throws Exception { + @SuppressWarnings("unchecked") + public void testDeleteActionDeletesSearchableSnapshot() throws Exception { createSnapshotRepo(client(), snapshotRepo, randomBoolean()); // create policy with cold and delete phases @@ -198,21 +198,21 @@ public void testDeleteActionDeletesSearchableSnapshot() throws Exception { assertBusy(() -> assertFalse(indexExists(restoredIndexName)), 60, TimeUnit.SECONDS); assertTrue("the snapshot we generate in the cold phase should be deleted by the delete phase", waitUntil(() -> { - try { - Request getSnapshotsRequest = new Request("GET", "_snapshot/" + snapshotRepo + "/_all"); - Response getSnapshotsResponse = client().performRequest(getSnapshotsRequest); - - Map responseMap; - try (InputStream is = getSnapshotsResponse.getEntity().getContent()) { - responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); - } - List responses = (List) responseMap.get("responses"); - Object snapshots = ((Map) responses.get(0)).get("snapshots"); - return ((List>) snapshots).size() == 0; - } catch (Exception e) { - logger.error(e.getMessage(), e); - return false; - } + try { + Request getSnapshotsRequest = new Request("GET", "_snapshot/" + snapshotRepo + "/_all"); + Response getSnapshotsResponse = client().performRequest(getSnapshotsRequest); + + Map responseMap; + try (InputStream is = getSnapshotsResponse.getEntity().getContent()) { + responseMap = XContentHelper.convertToMap(XContentType.JSON.xContent(), is, true); + } + List responses = (List) responseMap.get("responses"); + Object snapshots = ((Map) responses.get(0)).get("snapshots"); + return ((List>) snapshots).size() == 0; + } catch (Exception e) { + logger.error(e.getMessage(), e); + return false; + } }, 30, TimeUnit.SECONDS)); } From d632d2655afd32a04cfbf1f85f4cc54f840754e4 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Thu, 29 Apr 2021 17:48:44 -0400 Subject: [PATCH 02/13] Pin searchable snapshots in hot phase to hot nodes --- .../xpack/core/ilm/MountSnapshotStep.java | 12 ++++-- .../actions/SearchableSnapshotActionIT.java | 40 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java index 5ad38d1ac33e0..3990cb5119451 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java @@ -17,6 +17,8 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.IndexSettings; import org.elasticsearch.rest.RestStatus; +import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider; +import org.elasticsearch.xpack.core.DataTier; import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotAction; import org.elasticsearch.xpack.core.searchablesnapshots.MountSearchableSnapshotRequest; @@ -101,10 +103,14 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl indexName = snapshotIndexName; } + Settings.Builder settingsBuilder = Settings.builder(); + settingsBuilder.put(IndexSettings.INDEX_CHECK_ON_STARTUP.getKey(), Boolean.FALSE.toString()); + if (TimeseriesLifecycleType.HOT_PHASE.equals(this.getKey().getPhase())) { + settingsBuilder.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, DataTier.DATA_HOT); + } + final MountSearchableSnapshotRequest mountSearchableSnapshotRequest = new MountSearchableSnapshotRequest(mountedIndexName, - snapshotRepository, snapshotName, indexName, Settings.builder() - .put(IndexSettings.INDEX_CHECK_ON_STARTUP.getKey(), Boolean.FALSE.toString()) - .build(), + snapshotRepository, snapshotName, indexName, settingsBuilder.build(), // we captured the index metadata when we took the snapshot. the index likely had the ILM execution state in the metadata. // if we were to restore the lifecycle.name setting, the restored index would be captured by the ILM runner and, // depending on what ILM execution state was captured at snapshot time, make it's way forward from _that_ step forward in 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 78ae1101bb523..42bfb6228bbed 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 @@ -478,6 +478,46 @@ public void testSecondSearchableSnapshotUsingDifferentRepoThrows() throws Except containsString("policy specifies [searchable_snapshot] action multiple times with differing repositories")); } + public void testFoobar() throws Exception { + createSnapshotRepo(client(), snapshotRepo, randomBoolean()); + createPolicy(client(), policy, + new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, new RolloverAction(null, null, null, 1L), + SearchableSnapshotAction.NAME, new SearchableSnapshotAction( + snapshotRepo, randomBoolean())) + ), + null, null, null, 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("hot").getKey())), + 30, TimeUnit.SECONDS); + + Map hotIndexSettings = getIndexSettingsAsMap(restoredIndex); + // searchable snapshots mounted in the hot phase should be pinned to hot nodes + assertThat(hotIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), + is("data_hot")); + } + + public void testSearchableSnapshotActionOverridesMigrateAction() throws Exception { createSnapshotRepo(client(), snapshotRepo, randomBoolean()); createPolicy(client(), policy, From 2f6fd4856006770dc158eac3c56b58973699aea3 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Tue, 4 May 2021 13:28:59 -0400 Subject: [PATCH 03/13] Heh, oops, fixup --- .../xpack/ilm/actions/SearchableSnapshotActionIT.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 42bfb6228bbed..4dcbd0479dd37 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 @@ -478,7 +478,7 @@ public void testSecondSearchableSnapshotUsingDifferentRepoThrows() throws Except containsString("policy specifies [searchable_snapshot] action multiple times with differing repositories")); } - public void testFoobar() throws Exception { + public void testSearchableSnapshotsInHotPhasePinnedToHotNodes() throws Exception { createSnapshotRepo(client(), snapshotRepo, randomBoolean()); createPolicy(client(), policy, new Phase("hot", TimeValue.ZERO, Map.of(RolloverAction.NAME, new RolloverAction(null, null, null, 1L), @@ -517,7 +517,6 @@ snapshotRepo, randomBoolean())) is("data_hot")); } - public void testSearchableSnapshotActionOverridesMigrateAction() throws Exception { createSnapshotRepo(client(), snapshotRepo, randomBoolean()); createPolicy(client(), policy, From 896ddb72c3c43f9a7f69fce34a845aabe97196f9 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Tue, 4 May 2021 16:28:40 -0400 Subject: [PATCH 04/13] Rewrite skipMigrateAction and adjust tests --- .../xpack/core/ilm/MigrateAction.java | 12 ++--- .../xpack/core/ilm/MigrateActionTests.java | 49 ------------------- .../actions/SearchableSnapshotActionIT.java | 24 ++------- 3 files changed, 8 insertions(+), 77 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 a31eb4d8274ac..8aa4069595319 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 @@ -21,6 +21,7 @@ import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider; import org.elasticsearch.xpack.core.DataTier; import org.elasticsearch.xpack.core.ilm.Step.StepKey; +import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants; import java.io.IOException; import java.util.Arrays; @@ -28,8 +29,6 @@ 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. @@ -109,7 +108,7 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { BranchingStep conditionalSkipActionStep = new BranchingStep(preMigrateBranchingKey, migrationKey, nextStepKey, (index, clusterState) -> { - if (skipMigrateAction(phase, clusterState.metadata().index(index))) { + if (skipMigrateAction(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 " + @@ -130,11 +129,8 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { } } - 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); + private static boolean skipMigrateAction(IndexMetadata indexMetadata) { + return SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(indexMetadata.getSettings()); } /** 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 8149b39ec29cf..833c7c93b6345 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,29 +6,20 @@ */ 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; @@ -115,44 +106,4 @@ public void testMigrateActionsConfiguresTierPreference() { 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/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 4dcbd0479dd37..aa265c8d91fe5 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 @@ -525,11 +525,7 @@ SearchableSnapshotAction.NAME, new SearchableSnapshotAction( snapshotRepo, randomBoolean())) ), 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))), - null, null + null, null, null ); createComposableTemplate(client(), randomAlphaOfLengthBetween(5, 10).toLowerCase(), dataStream, @@ -542,6 +538,7 @@ snapshotRepo, randomBoolean())) indexDocument(client(), dataStream, true); String firstGenIndex = DataStream.getDefaultBackingIndexName(dataStream, 1L); Map indexSettings = getIndexSettingsAsMap(firstGenIndex); + // the searchable snapshot is allocated to the hot tier assertThat(indexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), is("data_hot")); // rollover the data stream so searchable_snapshot can complete @@ -556,21 +553,8 @@ snapshotRepo, randomBoolean())) 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("cold").getKey()))); - - Map coldIndexSettings = getIndexSettingsAsMap(restoredIndex); - // the frozen phase should've reconfigured the allocation preference - assertThat(coldIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), - is("data_cold,data_warm,data_hot")); + // // the searchable snapshot continues on the to warm tier and onward + assertThat(warmIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), is("data_warm,data_hot")); } } From 6adb1260a554258ec6003756c7417992498e203b Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 09:59:51 -0400 Subject: [PATCH 05/13] Prefer List.of --- .../java/org/elasticsearch/xpack/core/ilm/MigrateAction.java | 3 +-- 1 file changed, 1 insertion(+), 2 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 8aa4069595319..9cbb7d7085be4 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 @@ -24,7 +24,6 @@ import org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants; import java.io.IOException; -import java.util.Arrays; import java.util.List; import java.util.Objects; import java.util.stream.Collectors; @@ -123,7 +122,7 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { UpdateSettingsStep updateMigrationSettingStep = new UpdateSettingsStep(migrationKey, migrationRoutedKey, client, migrationSettings.build()); DataTierMigrationRoutedStep migrationRoutedStep = new DataTierMigrationRoutedStep(migrationRoutedKey, nextStepKey); - return Arrays.asList(conditionalSkipActionStep, updateMigrationSettingStep, migrationRoutedStep); + return List.of(conditionalSkipActionStep, updateMigrationSettingStep, migrationRoutedStep); } else { return List.of(); } From 6bd08fec59b301ff63794cbcb443e008016b119b Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 10:00:04 -0400 Subject: [PATCH 06/13] Inline skipMigrateAction --- .../elasticsearch/xpack/core/ilm/MigrateAction.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 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 9cbb7d7085be4..f06a072418ae1 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 @@ -9,7 +9,6 @@ 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; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.StreamInput; @@ -107,9 +106,9 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { BranchingStep conditionalSkipActionStep = new BranchingStep(preMigrateBranchingKey, migrationKey, nextStepKey, (index, clusterState) -> { - if (skipMigrateAction(clusterState.metadata().index(index))) { - String policyName = - LifecycleSettings.LIFECYCLE_NAME_SETTING.get(clusterState.metadata().index(index).getSettings()); + Settings indexSettings = clusterState.metadata().index(index).getSettings(); + if (SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(indexSettings)) { + String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexSettings); 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; @@ -128,10 +127,6 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { } } - private static boolean skipMigrateAction(IndexMetadata indexMetadata) { - return SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(indexMetadata.getSettings()); - } - /** * 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` From 21cabd43d71dd6e77c4d9c1729f05fbe3c7c75f9 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 11:20:13 -0400 Subject: [PATCH 07/13] Add a comment --- .../org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java index 3990cb5119451..5ae746a77f082 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/MountSnapshotStep.java @@ -105,10 +105,10 @@ void performDuringNoSnapshot(IndexMetadata indexMetadata, ClusterState currentCl Settings.Builder settingsBuilder = Settings.builder(); settingsBuilder.put(IndexSettings.INDEX_CHECK_ON_STARTUP.getKey(), Boolean.FALSE.toString()); + // if we are mounting a searchable snapshot in the hot phase, then the index should be pinned to the hot nodes if (TimeseriesLifecycleType.HOT_PHASE.equals(this.getKey().getPhase())) { settingsBuilder.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, DataTier.DATA_HOT); } - final MountSearchableSnapshotRequest mountSearchableSnapshotRequest = new MountSearchableSnapshotRequest(mountedIndexName, snapshotRepository, snapshotName, indexName, settingsBuilder.build(), // we captured the index metadata when we took the snapshot. the index likely had the ILM execution state in the metadata. From 0b9f44a577b384a2f5c1c8a07a6a716c5bcc9a6a Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 12:57:03 -0400 Subject: [PATCH 08/13] Reword debug log message --- .../java/org/elasticsearch/xpack/core/ilm/MigrateAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 f06a072418ae1..0a42cb2fb15a6 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 @@ -109,8 +109,8 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { Settings indexSettings = clusterState.metadata().index(index).getSettings(); if (SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(indexSettings)) { String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexSettings); - 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); + logger.debug("[{}] action in policy [{}] is configured for index [{}] which is a partially mounted index. skipping this action", + MigrateAction.NAME, policyName, index.getName()); return true; } From 9623b09fc929d7c60cb78938696d5146868d5e38 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 13:15:03 -0400 Subject: [PATCH 09/13] Shuffle code just a little --- .../org/elasticsearch/xpack/core/ilm/MigrateAction.java | 6 +++--- 1 file changed, 3 insertions(+), 3 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 0a42cb2fb15a6..973eb6de1e100 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 @@ -100,7 +100,6 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { StepKey migrationKey = new StepKey(phase, NAME, NAME); StepKey migrationRoutedKey = new StepKey(phase, NAME, DataTierMigrationRoutedStep.NAME); - Settings.Builder migrationSettings = Settings.builder(); String targetTier = "data_" + phase; assert DataTier.validTierName(targetTier) : "invalid data tier name:" + targetTier; @@ -117,9 +116,10 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { // 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()); + Settings.builder() + .put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getPreferredTiersConfiguration(targetTier)) + .build()); DataTierMigrationRoutedStep migrationRoutedStep = new DataTierMigrationRoutedStep(migrationRoutedKey, nextStepKey); return List.of(conditionalSkipActionStep, updateMigrationSettingStep, migrationRoutedStep); } else { From 8695b510e32c1972d76492052adf5e680970f10d Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 13:21:28 -0400 Subject: [PATCH 10/13] Rework these comments --- .../java/org/elasticsearch/xpack/core/ilm/MigrateAction.java | 3 ++- 1 file changed, 2 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 973eb6de1e100..b8b4ce4f05b6c 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 @@ -106,6 +106,8 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { BranchingStep conditionalSkipActionStep = new BranchingStep(preMigrateBranchingKey, migrationKey, nextStepKey, (index, clusterState) -> { Settings indexSettings = clusterState.metadata().index(index).getSettings(); + + // partially mounted indices will already have data_frozen, and we don't want to change that if they do if (SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(indexSettings)) { String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexSettings); logger.debug("[{}] action in policy [{}] is configured for index [{}] which is a partially mounted index. skipping this action", @@ -113,7 +115,6 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { 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; }); UpdateSettingsStep updateMigrationSettingStep = new UpdateSettingsStep(migrationKey, migrationRoutedKey, client, From 26f799d32540d177c68d18f9a67e418e6eed8e10 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 13:34:36 -0400 Subject: [PATCH 11/13] Split this line, it's too long --- .../java/org/elasticsearch/xpack/core/ilm/MigrateAction.java | 4 ++-- 1 file changed, 2 insertions(+), 2 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 b8b4ce4f05b6c..7325c1c356090 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 @@ -110,8 +110,8 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { // partially mounted indices will already have data_frozen, and we don't want to change that if they do if (SearchableSnapshotsConstants.isPartialSearchableSnapshotIndex(indexSettings)) { String policyName = LifecycleSettings.LIFECYCLE_NAME_SETTING.get(indexSettings); - logger.debug("[{}] action in policy [{}] is configured for index [{}] which is a partially mounted index. skipping this action", - MigrateAction.NAME, policyName, index.getName()); + logger.debug("[{}] action in policy [{}] is configured for index [{}] which is a partially mounted index. " + + "skipping this action", MigrateAction.NAME, policyName, index.getName()); return true; } From 579c851d203fea67188054149ef3e71d0077448f Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 13:34:54 -0400 Subject: [PATCH 12/13] Oops, fix double comment --- .../xpack/ilm/actions/SearchableSnapshotActionIT.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 aa265c8d91fe5..6665d5d0dec7d 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 @@ -553,7 +553,7 @@ snapshotRepo, randomBoolean())) 30, TimeUnit.SECONDS); Map warmIndexSettings = getIndexSettingsAsMap(restoredIndex); - // // the searchable snapshot continues on the to warm tier and onward + // the searchable snapshot continues on the to warm tier and onward assertThat(warmIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), is("data_warm,data_hot")); } From 46e4ba4fb30bf8100741ccbe2fe2d02160c12173 Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 14 May 2021 15:39:55 -0400 Subject: [PATCH 13/13] Drop integration test and add another unit test --- .../xpack/core/ilm/MigrateActionTests.java | 55 +++++++++++++++++++ .../actions/SearchableSnapshotActionIT.java | 43 --------------- 2 files changed, 55 insertions(+), 43 deletions(-) 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 833c7c93b6345..dd6c35a64c9dc 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,6 +6,11 @@ */ package org.elasticsearch.xpack.core.ilm; +import org.elasticsearch.Version; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.cluster.metadata.Metadata; import org.elasticsearch.common.io.stream.Writeable.Reader; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider; @@ -14,6 +19,7 @@ import java.io.IOException; import java.util.List; +import static org.elasticsearch.index.IndexModule.INDEX_STORE_TYPE_SETTING; 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; @@ -22,6 +28,8 @@ import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.DELETE_PHASE; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.HOT_PHASE; import static org.elasticsearch.xpack.core.ilm.TimeseriesLifecycleType.WARM_PHASE; +import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_DIRECTORY_FACTORY_KEY; +import static org.elasticsearch.xpack.searchablesnapshots.SearchableSnapshotsConstants.SNAPSHOT_PARTIAL_SETTING; import static org.hamcrest.CoreMatchers.is; public class MigrateActionTests extends AbstractActionTestCase { @@ -106,4 +114,51 @@ public void testMigrateActionsConfiguresTierPreference() { is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT)); } } + + public void testMigrateActionWillSkipAPartiallyMountedIndex() { + StepKey nextStepKey = new StepKey(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLengthBetween(1, 10), + randomAlphaOfLengthBetween(1, 10)); + MigrateAction action = new MigrateAction(); + + // does not skip an ordinary index + { + IndexMetadata indexMetadata = IndexMetadata.builder(randomAlphaOfLength(5)) + .settings(settings(Version.CURRENT)) + .numberOfShards(1) + .numberOfReplicas(2) + .build(); + + ClusterState clusterState = ClusterState.builder(new ClusterName("_name")) + .metadata(Metadata.builder().put(indexMetadata, true).build()) + .build(); + + List steps = action.toSteps(null, HOT_PHASE, nextStepKey); + BranchingStep firstStep = (BranchingStep) steps.get(0); + UpdateSettingsStep secondStep = (UpdateSettingsStep) steps.get(1); + firstStep.performAction(indexMetadata.getIndex(), clusterState); + + assertEquals(secondStep.getKey(), firstStep.getNextStepKey()); + } + + // does skip a partially mounted + { + IndexMetadata indexMetadata = IndexMetadata.builder(randomAlphaOfLength(5)) + .settings(settings(Version.CURRENT) + .put(INDEX_STORE_TYPE_SETTING.getKey(), SNAPSHOT_DIRECTORY_FACTORY_KEY) + .put(SNAPSHOT_PARTIAL_SETTING.getKey(), true)) + .numberOfShards(1) + .numberOfReplicas(2) + .build(); + + ClusterState clusterState = ClusterState.builder(new ClusterName("_name")) + .metadata(Metadata.builder().put(indexMetadata, true).build()) + .build(); + + List steps = action.toSteps(null, HOT_PHASE, nextStepKey); + BranchingStep firstStep = (BranchingStep) steps.get(0); + firstStep.performAction(indexMetadata.getIndex(), clusterState); + + assertEquals(nextStepKey, firstStep.getNextStepKey()); + } + } } 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 6665d5d0dec7d..a809b08ce903e 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 @@ -28,7 +28,6 @@ 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; @@ -40,7 +39,6 @@ import java.io.IOException; import java.io.InputStream; -import java.time.ZonedDateTime; import java.util.HashMap; import java.util.List; import java.util.Locale; @@ -516,45 +514,4 @@ snapshotRepo, randomBoolean())) assertThat(hotIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), is("data_hot")); } - - 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, null, 1L), - SearchableSnapshotAction.NAME, new SearchableSnapshotAction( - snapshotRepo, randomBoolean())) - ), - new Phase("warm", TimeValue.ZERO, Map.of(MigrateAction.NAME, new MigrateAction(true))), - null, null, 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); - // the searchable snapshot is allocated to the hot tier - 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 searchable snapshot continues on the to warm tier and onward - assertThat(warmIndexSettings.get(DataTierAllocationDecider.INDEX_ROUTING_PREFER), is("data_warm,data_hot")); - } - }