From a77a99a1cca1a5188cddc8fd830f884ded6d9e88 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 23 Sep 2020 14:46:52 +0100 Subject: [PATCH 1/9] ILM: migrate action configures the _tier_preference setting The `migrate` action will now configure the `index.routing.allocation.include._tier_preference` setting to the corresponding tiers. For the HOT phase it will configure `data_hot`, for the WARM phase it will configure `data_warm,data_hot` and for the COLD phase `data_cold,data_warm,data_cold`. --- .../allocation/DataTierAllocationDecider.java | 2 +- .../core/ilm/DataTierMigrationRoutedStep.java | 46 +++++++++--------- .../xpack/core/ilm/MigrateAction.java | 23 ++++++++- .../ilm/DataTierMigrationRoutedStepTests.java | 5 +- .../xpack/core/ilm/MigrateActionTests.java | 41 ++++++++++++++++ .../xpack/ilm/DataTiersMigrationsTests.java | 47 ++++++++++--------- 6 files changed, 116 insertions(+), 48 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java index a667ff98175e9..cc555b9c8f8ba 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/cluster/routing/allocation/DataTierAllocationDecider.java @@ -238,7 +238,7 @@ private enum OpType { * exist. If no nodes for any of the tiers are available, returns an empty * {@code Optional}. */ - static Optional preferredAvailableTier(String prioritizedTiers, DiscoveryNodes nodes) { + public static Optional preferredAvailableTier(String prioritizedTiers, DiscoveryNodes nodes) { String[] tiers = Strings.tokenizeToStringArray(prioritizedTiers, ","); return Arrays.stream(tiers).filter(tier -> tierNodesPresent(tier, nodes)).findFirst(); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java index fc8ecf107f15d..476bc9c77f108 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java @@ -10,8 +10,6 @@ import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetadata; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.routing.allocation.decider.AllocationDeciders; import org.elasticsearch.common.Strings; import org.elasticsearch.common.settings.ClusterSettings; @@ -26,7 +24,6 @@ import java.util.Locale; import java.util.Set; -import static org.elasticsearch.cluster.node.DiscoveryNodeRole.DATA_ROLE; import static org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING; import static org.elasticsearch.xpack.core.ilm.AllocationRoutedStep.getPendingAllocations; import static org.elasticsearch.xpack.core.ilm.step.info.AllocationInfo.waitingForActiveShardsAllocationInfo; @@ -73,21 +70,30 @@ public Result isConditionMet(Index index, ClusterState clusterState) { logger.debug("[{}] lifecycle action for index [{}] executed but index no longer exists", getKey().getAction(), index.getName()); return new Result(false, null); } - String destinationTier = INDEX_ROUTING_PREFER_SETTING.get(idxMeta.getSettings()); + String preferredTierConfiguration = INDEX_ROUTING_PREFER_SETTING.get(idxMeta.getSettings()); + String availableDestinationTier = DataTierAllocationDecider.preferredAvailableTier(preferredTierConfiguration, + clusterState.getNodes()).orElse(""); + if (ActiveShardCount.ALL.enoughShardsActive(clusterState, index.getName()) == false) { - if (Strings.isEmpty(destinationTier)) { + if (Strings.isEmpty(preferredTierConfiguration)) { logger.debug("[{}] lifecycle action for index [{}] cannot make progress because not all shards are active", getKey().getAction(), index.getName()); } else { - logger.debug("[{}] migration of index [{}] to the [{}] tier cannot progress, as not all shards are active", - getKey().getAction(), index.getName(), destinationTier); + if (Strings.hasText(availableDestinationTier)) { + logger.debug("[{}] migration of index [{}] to the [{}] tier cannot progress, as not all shards are active", + getKey().getAction(), index.getName(), preferredTierConfiguration); + } else { + logger.debug("[{}] migration of index [{}] to the next tier cannot progress as there is no available tier for the " + + "configured preferred tiers [{}] and not all shards are active", getKey().getAction(), index.getName(), + preferredTierConfiguration); + } } return new Result(false, waitingForActiveShardsAllocationInfo(idxMeta.getNumberOfReplicas())); } - if (Strings.isEmpty(destinationTier)) { - logger.debug("index [{}] has no data tier routing setting configured and all its shards are active. considering the [{}] " + - "step condition met and continuing to the next step", index.getName(), getKey().getName()); + if (Strings.isEmpty(preferredTierConfiguration)) { + logger.debug("index [{}] has no data tier routing preference setting configured and all its shards are active. considering " + + "the [{}] step condition met and continuing to the next step", index.getName(), getKey().getName()); // the user removed the tier routing setting and all the shards are active so we'll cary on return new Result(true, null); } @@ -95,22 +101,18 @@ public Result isConditionMet(Index index, ClusterState clusterState) { int allocationPendingAllShards = getPendingAllocations(index, ALLOCATION_DECIDERS, clusterState); if (allocationPendingAllShards > 0) { - boolean targetTierNodeFound = false; - for (DiscoveryNode node : clusterState.nodes()) { - for (DiscoveryNodeRole role : node.getRoles()) { - if (role.roleName().equals(DATA_ROLE.roleName()) || role.roleName().equals(destinationTier)) { - targetTierNodeFound = true; - break; - } - } + String statusMessage; + if (Strings.hasText(availableDestinationTier)) { + statusMessage = String.format(Locale.ROOT, "[%s] lifecycle action [%s] waiting for [%s] shards to be moved to the [%s] " + + "tier", index.getName(), getKey().getAction(), allocationPendingAllShards, availableDestinationTier); + } else { + statusMessage = String.format(Locale.ROOT, "index [%s] has a preference for tiers [%s], " + + "but no nodes for any of those tiers are available in the cluster", index.getName(), preferredTierConfiguration); } - String statusMessage = String.format(Locale.ROOT, "%s lifecycle action [%s] waiting for [%s] shards to be moved to the [%s] " + - "tier" + (targetTierNodeFound ? "" : " but there are currently no [%s] nodes in the cluster"), - index, getKey().getAction(), allocationPendingAllShards, destinationTier, destinationTier); logger.debug(statusMessage); return new Result(false, new AllocationInfo(idxMeta.getNumberOfReplicas(), allocationPendingAllShards, true, statusMessage)); } else { - logger.debug("[{}] migration of index [{}] to tier [{}] complete", getKey().getAction(), index, destinationTier); + logger.debug("[{}] migration of index [{}] to tier [{}] complete", getKey().getAction(), index, availableDestinationTier); return new Result(true, null); } } 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 4f8784d22ee79..dcf558c0ef1cb 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 @@ -22,6 +22,7 @@ import java.util.Arrays; import java.util.List; import java.util.Objects; +import java.util.stream.Collectors; /** * A {@link LifecycleAction} which enables or disables the automatic migration of data between @@ -31,6 +32,9 @@ public class MigrateAction implements LifecycleAction { public static final String NAME = "migrate"; public static final ParseField ENABLED_FIELD = new ParseField("enabled"); + // Represents an ordered list of data tiers from cold to hot (or slow to fast) + private static final List COLD_TO_HOT_TIERS = List.of(DataTier.DATA_COLD, DataTier.DATA_WARM, DataTier.DATA_HOT); + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>(NAME, a -> new MigrateAction(a[0] == null ? true : (boolean) a[0])); @@ -92,7 +96,7 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { Settings.Builder migrationSettings = Settings.builder(); String dataTierName = "data_" + phase; assert DataTier.validTierName(dataTierName) : "invalid data tier name:" + dataTierName; - migrationSettings.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, dataTierName); + migrationSettings.put(DataTierAllocationDecider.INDEX_ROUTING_PREFER, getPreferredTiersConfiguration(dataTierName)); UpdateSettingsStep updateMigrationSettingStep = new UpdateSettingsStep(migrationKey, migrationRoutedKey, client, migrationSettings.build()); DataTierMigrationRoutedStep migrationRoutedStep = new DataTierMigrationRoutedStep(migrationRoutedKey, nextStepKey); @@ -102,6 +106,23 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { } } + /** + * 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` + * This is usually used in conjunction with {@link DataTierAllocationDecider#INDEX_ROUTING_PREFER_SETTING} + */ + static String getPreferredTiersConfiguration(String targetTier) { + int indexOfTargetTier = COLD_TO_HOT_TIERS.indexOf(targetTier); + if(indexOfTargetTier == -1) { + throw new IllegalArgumentException("invalid data tier [" + targetTier + "]"); + } + String preferredTiers = COLD_TO_HOT_TIERS.stream().skip(indexOfTargetTier).collect(Collectors.joining(",")); + if (Strings.hasText(preferredTiers) == false) { + return targetTier; + } + return preferredTiers; + } + @Override public int hashCode() { return Objects.hash(enabled); diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java index 6518706544aa7..f3e97f7bfa45a 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java @@ -137,9 +137,8 @@ public void testExecuteWithPendingShardsAndTargetRoleNotPresentInCluster() { .build(); DataTierMigrationRoutedStep step = createRandomInstance(); Result expectedResult = new Result(false, new AllocationInfo(0, 1, true, - "[" + index.getName() + "] lifecycle action [" + step.getKey().getAction() + "] waiting for " + - "[1] shards to be moved to the [data_warm] tier but there are currently no [data_warm] nodes in the cluster") - ); + "index [" + index.getName() + "] has a preference for tiers [data_warm], but no nodes for any of those tiers are available " + + "in the cluster")); Result actualResult = step.isConditionMet(index, clusterState); assertThat(actualResult.isComplete(), is(false)); 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 67bdb7f03e795..bdfb00fd99e96 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 @@ -7,12 +7,21 @@ import org.elasticsearch.common.io.stream.Writeable.Reader; 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.List; +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.TimeseriesLifecycleType.COLD_PHASE; 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.hamcrest.CoreMatchers.is; public class MigrateActionTests extends AbstractActionTestCase { @@ -56,4 +65,36 @@ public void testToSteps() { assertEquals(0, steps.size()); } } + + public void testGetPreferredTiersConfiguration() { + assertThat(getPreferredTiersConfiguration(DATA_HOT), is(DATA_HOT)); + assertThat(getPreferredTiersConfiguration(DATA_WARM), is(DATA_WARM + "," + DATA_HOT)); + assertThat(getPreferredTiersConfiguration(DATA_COLD), is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT)); + IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> getPreferredTiersConfiguration("no_tier")); + assertThat(exception.getMessage(), is("invalid data tier [no_tier]")); + } + + public void testMigrateActionsConfiguresTierPreference() { + StepKey nextStepKey = new StepKey(randomAlphaOfLengthBetween(1, 10), randomAlphaOfLengthBetween(1, 10), + randomAlphaOfLengthBetween(1, 10)); + MigrateAction action = new MigrateAction(); + { + List steps = action.toSteps(null, HOT_PHASE, nextStepKey); + UpdateSettingsStep firstStep = (UpdateSettingsStep) steps.get(0); + 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); + 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); + assertThat(DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING.get(firstStep.getSettings()), + is(DATA_COLD + "," + DATA_WARM + "," + DATA_HOT)); + } + } } diff --git a/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/DataTiersMigrationsTests.java b/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/DataTiersMigrationsTests.java index da9d66b78f439..58719a0b68ada 100644 --- a/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/DataTiersMigrationsTests.java +++ b/x-pack/plugin/ilm/src/internalClusterTest/java/org/elasticsearch/xpack/ilm/DataTiersMigrationsTests.java @@ -92,8 +92,19 @@ public static Settings coldNode(final Settings settings) { public void testIndexDataTierMigration() throws Exception { internalCluster().startMasterOnlyNodes(1, Settings.EMPTY); - logger.info("starting hot data node"); + logger.info("starting 2 hot data nodes"); internalCluster().startNode(hotNode(Settings.EMPTY)); + internalCluster().startNode(hotNode(Settings.EMPTY)); + + // it's important we start one node of each tear as otherwise all phases will be allocated on the 2 available hot nodes (as our + // tier preference configuration will not detect any available warm/cold tier node and will fallback to the available hot tier) + // we want ILM to stop in the check-migration step in the warm and cold phase so we can unblock it manually by starting another + // node in the corresponding tier (so that the index replica is allocated) + logger.info("starting a warm data node"); + internalCluster().startNode(warmNode(Settings.EMPTY)); + + logger.info("starting a cold data node"); + internalCluster().startNode(coldNode(Settings.EMPTY)); Phase hotPhase = new Phase("hot", TimeValue.ZERO, Collections.emptyMap()); Phase warmPhase = new Phase("warm", TimeValue.ZERO, Collections.emptyMap()); @@ -104,7 +115,7 @@ public void testIndexDataTierMigration() throws Exception { assertAcked(putLifecycleResponse); Settings settings = Settings.builder().put(indexSettings()).put(SETTING_NUMBER_OF_SHARDS, 1) - .put(SETTING_NUMBER_OF_REPLICAS, 0).put(LifecycleSettings.LIFECYCLE_NAME, policy).build(); + .put(SETTING_NUMBER_OF_REPLICAS, 1).put(LifecycleSettings.LIFECYCLE_NAME, policy).build(); CreateIndexResponse res = client().admin().indices().prepareCreate(managedIndex).setSettings(settings).get(); assertTrue(res.isAcknowledged()); @@ -118,7 +129,7 @@ public void testIndexDataTierMigration() throws Exception { assertThat(indexLifecycleExplainResponse.getStep(), is(DataTierMigrationRoutedStep.NAME)); }); - logger.info("starting warm data node"); + logger.info("starting a warm data node"); internalCluster().startNode(warmNode(Settings.EMPTY)); assertBusy(() -> { ExplainLifecycleRequest explainRequest = new ExplainLifecycleRequest().indices(managedIndex); @@ -130,7 +141,7 @@ public void testIndexDataTierMigration() throws Exception { assertThat(indexLifecycleExplainResponse.getStep(), is(DataTierMigrationRoutedStep.NAME)); }); - logger.info("starting cold data node"); + logger.info("starting a cold data node"); internalCluster().startNode(coldNode(Settings.EMPTY)); // wait for lifecycle to complete in the cold phase after the index has been migrated to the cold node @@ -147,9 +158,15 @@ public void testIndexDataTierMigration() throws Exception { public void testUserOptsOutOfTierMigration() throws Exception { internalCluster().startMasterOnlyNodes(1, Settings.EMPTY); - logger.info("starting hot data node"); + logger.info("starting a hot data node"); internalCluster().startNode(hotNode(Settings.EMPTY)); + logger.info("starting a warm data node"); + internalCluster().startNode(warmNode(Settings.EMPTY)); + + logger.info("starting a cold data node"); + internalCluster().startNode(coldNode(Settings.EMPTY)); + Phase hotPhase = new Phase("hot", TimeValue.ZERO, Collections.emptyMap()); Phase warmPhase = new Phase("warm", TimeValue.ZERO, Collections.emptyMap()); Phase coldPhase = new Phase("cold", TimeValue.ZERO, Collections.emptyMap()); @@ -171,26 +188,14 @@ public void testUserOptsOutOfTierMigration() throws Exception { IndexLifecycleExplainResponse indexLifecycleExplainResponse = explainResponse.getIndexResponses().get(managedIndex); assertThat(indexLifecycleExplainResponse.getPhase(), is("warm")); assertThat(indexLifecycleExplainResponse.getStep(), is(DataTierMigrationRoutedStep.NAME)); - }); + assertReplicaIsUnassigned(); + }, 30, TimeUnit.SECONDS); Settings removeTierRoutingSetting = Settings.builder().putNull(DataTierAllocationDecider.INDEX_ROUTING_PREFER).build(); UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(managedIndex).settings(removeTierRoutingSetting); assertAcked(client().admin().indices().updateSettings(updateSettingsRequest).actionGet()); - assertBusy(() -> { - ExplainLifecycleRequest explainRequest = new ExplainLifecycleRequest().indices(managedIndex); - ExplainLifecycleResponse explainResponse = client().execute(ExplainLifecycleAction.INSTANCE, - explainRequest).get(); - - IndexLifecycleExplainResponse indexLifecycleExplainResponse = explainResponse.getIndexResponses().get(managedIndex); - assertThat(indexLifecycleExplainResponse.getPhase(), is("warm")); - assertThat(indexLifecycleExplainResponse.getStep(), is(DataTierMigrationRoutedStep.NAME)); - assertReplicaIsUnassigned(); - }, 30, TimeUnit.SECONDS); - - internalCluster().startNode(coldNode(Settings.EMPTY)); - - // the index should successfully allocate + // the index should successfully allocate on any nodes ensureGreen(managedIndex); // the index is successfully allocated but the migrate action from the cold phase re-configured the tier migration setting to the @@ -206,7 +211,7 @@ public void testUserOptsOutOfTierMigration() throws Exception { IndexLifecycleExplainResponse indexLifecycleExplainResponse = explainResponse.getIndexResponses().get(managedIndex); assertThat(indexLifecycleExplainResponse.getPhase(), is("cold")); assertThat(indexLifecycleExplainResponse.getStep(), is(DataTierMigrationRoutedStep.NAME)); - }); + }, 30, TimeUnit.SECONDS); // remove the tier routing setting again assertAcked(client().admin().indices().updateSettings(updateSettingsRequest).actionGet()); From 53c3a99023829e2499a0adadad72e19bacbb2332 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Wed, 23 Sep 2020 16:32:33 +0100 Subject: [PATCH 2/9] Drop redundant check --- .../org/elasticsearch/xpack/core/ilm/MigrateAction.java | 6 +----- 1 file changed, 1 insertion(+), 5 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 dcf558c0ef1cb..6497049dd5c5d 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 @@ -116,11 +116,7 @@ static String getPreferredTiersConfiguration(String targetTier) { if(indexOfTargetTier == -1) { throw new IllegalArgumentException("invalid data tier [" + targetTier + "]"); } - String preferredTiers = COLD_TO_HOT_TIERS.stream().skip(indexOfTargetTier).collect(Collectors.joining(",")); - if (Strings.hasText(preferredTiers) == false) { - return targetTier; - } - return preferredTiers; + return COLD_TO_HOT_TIERS.stream().skip(indexOfTargetTier).collect(Collectors.joining(",")); } @Override From 27577db697dcd6f82de7d6e1c662365d0d07286a Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 24 Sep 2020 11:07:52 +0100 Subject: [PATCH 3/9] Extra space on if statement Co-authored-by: Lee Hinman --- .../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 6497049dd5c5d..4f8231921fcb6 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 @@ -113,7 +113,7 @@ public List toSteps(Client client, String phase, StepKey nextStepKey) { */ static String getPreferredTiersConfiguration(String targetTier) { int indexOfTargetTier = COLD_TO_HOT_TIERS.indexOf(targetTier); - if(indexOfTargetTier == -1) { + if (indexOfTargetTier == -1) { throw new IllegalArgumentException("invalid data tier [" + targetTier + "]"); } return COLD_TO_HOT_TIERS.stream().skip(indexOfTargetTier).collect(Collectors.joining(",")); From f1dd5cef40cc124dfe99cc850656321c979da229 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 24 Sep 2020 11:08:30 +0100 Subject: [PATCH 4/9] Include the preference configuration in log message Co-authored-by: Lee Hinman --- .../xpack/core/ilm/DataTierMigrationRoutedStep.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java index 476bc9c77f108..7fc5de0f6e720 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java @@ -112,7 +112,7 @@ public Result isConditionMet(Index index, ClusterState clusterState) { logger.debug(statusMessage); return new Result(false, new AllocationInfo(idxMeta.getNumberOfReplicas(), allocationPendingAllShards, true, statusMessage)); } else { - logger.debug("[{}] migration of index [{}] to tier [{}] complete", getKey().getAction(), index, availableDestinationTier); + logger.debug("[{}] migration of index [{}] to tier [{}] (preference [{}]) complete", getKey().getAction(), index, availableDestinationTier, preferredTierConfiguration); return new Result(true, null); } } From 408df916a0e8530450f056556b29e62bc67fb0f8 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 24 Sep 2020 11:09:05 +0100 Subject: [PATCH 5/9] Update log message Co-authored-by: Lee Hinman --- .../xpack/core/ilm/DataTierMigrationRoutedStep.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java index 7fc5de0f6e720..e4d663b2d5bfc 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java @@ -80,7 +80,7 @@ public Result isConditionMet(Index index, ClusterState clusterState) { getKey().getAction(), index.getName()); } else { if (Strings.hasText(availableDestinationTier)) { - logger.debug("[{}] migration of index [{}] to the [{}] tier cannot progress, as not all shards are active", + logger.debug("[{}] migration of index [{}] to the [{}] tier preference cannot progress, as not all shards are active", getKey().getAction(), index.getName(), preferredTierConfiguration); } else { logger.debug("[{}] migration of index [{}] to the next tier cannot progress as there is no available tier for the " + From a087b3d63a53837c21a106e923286578140a8c83 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 24 Sep 2020 11:13:31 +0100 Subject: [PATCH 6/9] Update log message to include the tier preference config --- .../xpack/core/ilm/DataTierMigrationRoutedStep.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java index e4d663b2d5bfc..0f9eea5bd2e8f 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java @@ -104,7 +104,8 @@ public Result isConditionMet(Index index, ClusterState clusterState) { String statusMessage; if (Strings.hasText(availableDestinationTier)) { statusMessage = String.format(Locale.ROOT, "[%s] lifecycle action [%s] waiting for [%s] shards to be moved to the [%s] " + - "tier", index.getName(), getKey().getAction(), allocationPendingAllShards, availableDestinationTier); + "tier (tier migration preference configuration is [{}])", index.getName(), getKey().getAction(), + allocationPendingAllShards, availableDestinationTier, preferredTierConfiguration); } else { statusMessage = String.format(Locale.ROOT, "index [%s] has a preference for tiers [%s], " + "but no nodes for any of those tiers are available in the cluster", index.getName(), preferredTierConfiguration); From 7e834cc100569421bb3e9dcf17067f16a47c1835 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 24 Sep 2020 11:15:50 +0100 Subject: [PATCH 7/9] Use %s for string templating --- .../xpack/core/ilm/DataTierMigrationRoutedStep.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java index 0f9eea5bd2e8f..eebc95ce0a417 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java @@ -104,8 +104,8 @@ public Result isConditionMet(Index index, ClusterState clusterState) { String statusMessage; if (Strings.hasText(availableDestinationTier)) { statusMessage = String.format(Locale.ROOT, "[%s] lifecycle action [%s] waiting for [%s] shards to be moved to the [%s] " + - "tier (tier migration preference configuration is [{}])", index.getName(), getKey().getAction(), - allocationPendingAllShards, availableDestinationTier, preferredTierConfiguration); + "tier (tier migration preference configuration is [%s])", index.getName(), getKey().getAction(), + allocationPendingAllShards, availableDestinationTier.get(), preferredTierConfiguration); } else { statusMessage = String.format(Locale.ROOT, "index [%s] has a preference for tiers [%s], " + "but no nodes for any of those tiers are available in the cluster", index.getName(), preferredTierConfiguration); From f62104c8a68c35389083567b4c942894cdbe2937 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 24 Sep 2020 11:20:41 +0100 Subject: [PATCH 8/9] Use Optional --- .../core/ilm/DataTierMigrationRoutedStep.java | 26 +++++++++---------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java index eebc95ce0a417..2c35f8d998ec2 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStep.java @@ -22,6 +22,7 @@ import java.util.HashSet; import java.util.List; import java.util.Locale; +import java.util.Optional; import java.util.Set; import static org.elasticsearch.xpack.cluster.routing.allocation.DataTierAllocationDecider.INDEX_ROUTING_PREFER_SETTING; @@ -71,15 +72,15 @@ public Result isConditionMet(Index index, ClusterState clusterState) { return new Result(false, null); } String preferredTierConfiguration = INDEX_ROUTING_PREFER_SETTING.get(idxMeta.getSettings()); - String availableDestinationTier = DataTierAllocationDecider.preferredAvailableTier(preferredTierConfiguration, - clusterState.getNodes()).orElse(""); + Optional availableDestinationTier = DataTierAllocationDecider.preferredAvailableTier(preferredTierConfiguration, + clusterState.getNodes()); if (ActiveShardCount.ALL.enoughShardsActive(clusterState, index.getName()) == false) { if (Strings.isEmpty(preferredTierConfiguration)) { logger.debug("[{}] lifecycle action for index [{}] cannot make progress because not all shards are active", getKey().getAction(), index.getName()); } else { - if (Strings.hasText(availableDestinationTier)) { + if (availableDestinationTier.isPresent()) { logger.debug("[{}] migration of index [{}] to the [{}] tier preference cannot progress, as not all shards are active", getKey().getAction(), index.getName(), preferredTierConfiguration); } else { @@ -101,19 +102,18 @@ public Result isConditionMet(Index index, ClusterState clusterState) { int allocationPendingAllShards = getPendingAllocations(index, ALLOCATION_DECIDERS, clusterState); if (allocationPendingAllShards > 0) { - String statusMessage; - if (Strings.hasText(availableDestinationTier)) { - statusMessage = String.format(Locale.ROOT, "[%s] lifecycle action [%s] waiting for [%s] shards to be moved to the [%s] " + - "tier (tier migration preference configuration is [%s])", index.getName(), getKey().getAction(), - allocationPendingAllShards, availableDestinationTier.get(), preferredTierConfiguration); - } else { - statusMessage = String.format(Locale.ROOT, "index [%s] has a preference for tiers [%s], " + - "but no nodes for any of those tiers are available in the cluster", index.getName(), preferredTierConfiguration); - } + String statusMessage = availableDestinationTier.map( + s -> String.format(Locale.ROOT, "[%s] lifecycle action [%s] waiting for [%s] shards to be moved to the [%s] tier (tier " + + "migration preference configuration is [%s])", index.getName(), getKey().getAction(), allocationPendingAllShards, s, + preferredTierConfiguration) + ).orElseGet( + () -> String.format(Locale.ROOT, "index [%s] has a preference for tiers [%s], but no nodes for any of those tiers are " + + "available in the cluster", index.getName(), preferredTierConfiguration)); logger.debug(statusMessage); return new Result(false, new AllocationInfo(idxMeta.getNumberOfReplicas(), allocationPendingAllShards, true, statusMessage)); } else { - logger.debug("[{}] migration of index [{}] to tier [{}] (preference [{}]) complete", getKey().getAction(), index, availableDestinationTier, preferredTierConfiguration); + logger.debug("[{}] migration of index [{}] to tier [{}] (preference [{}]) complete", + getKey().getAction(), index, availableDestinationTier, preferredTierConfiguration); return new Result(true, null); } } From 12955d5e7e4069e3847a2c89c3627606547eaf47 Mon Sep 17 00:00:00 2001 From: Andrei Dan Date: Thu, 24 Sep 2020 11:24:28 +0100 Subject: [PATCH 9/9] Fix test --- .../xpack/core/ilm/DataTierMigrationRoutedStepTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java index f3e97f7bfa45a..e6f50df6584b7 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/DataTierMigrationRoutedStepTests.java @@ -112,7 +112,7 @@ public void testExecuteWithPendingShards() { DataTierMigrationRoutedStep step = createRandomInstance(); Result expectedResult = new Result(false, new AllocationInfo(0, 1, true, "[" + index.getName() + "] lifecycle action [" + step.getKey().getAction() + "] waiting for " + - "[1] shards to be moved to the [data_warm] tier") + "[1] shards to be moved to the [data_warm] tier (tier migration preference configuration is [data_warm])") ); Result actualResult = step.isConditionMet(index, clusterState);