From 62bee44c36131efde8e48ca05abc4542ebacd48d Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 19 Aug 2021 15:19:55 -0500 Subject: [PATCH 1/4] Ensuring that the ShrinkAction does not hang if total shards per node is too low --- .../reference/ilm/actions/ilm-shrink.asciidoc | 4 +++ .../core/ilm/SetSingleNodeAllocateStep.java | 5 ++-- .../xpack/ilm/actions/ShrinkActionIT.java | 30 +++++++++++++++++++ 3 files changed, 37 insertions(+), 2 deletions(-) diff --git a/docs/reference/ilm/actions/ilm-shrink.asciidoc b/docs/reference/ilm/actions/ilm-shrink.asciidoc index e29b975771aac..321e89d110fd9 100644 --- a/docs/reference/ilm/actions/ilm-shrink.asciidoc +++ b/docs/reference/ilm/actions/ilm-shrink.asciidoc @@ -17,6 +17,10 @@ stream. You cannot perform the `shrink` action on a write index. To use the `shrink` action in the `hot` phase, the `rollover` action *must* be present. If no rollover action is configured, {ilm-init} will reject the policy. +The shrink action will set the index's index.routing.allocation.total_shards_per_node +setting to -1 to ensure that all shards of the index can be copied to a single node. +This setting will persist on the index even after the step completes. + [IMPORTANT] If the shrink action is used on a <>, policy execution waits until the leader index rolls over (or is < indexDocument(client(), index)); } + + /* + * This test verifies that we can still shrink an index even if the total number of shards in the index is greater than + * index.routing.allocation.total_shards_per_node. + */ + public void testTotalShardsPerNodeTooLow() throws Exception { + int numShards = 4; + int divisor = randomFrom(2, 4); + int expectedFinalShards = numShards / divisor; + createIndexWithSettings(client(), index, alias, Settings.builder().put(SETTING_NUMBER_OF_SHARDS, numShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 0) + .put(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), numShards - 2)); + createNewSingletonPolicy(client(), policy, "warm", new ShrinkAction(expectedFinalShards, null)); + updatePolicy(client(), index, policy); + + String shrunkenIndexName = waitAndGetShrinkIndexName(client(), index); + assertBusy(() -> assertTrue(indexExists(shrunkenIndexName)), 60, TimeUnit.SECONDS); + assertBusy(() -> assertTrue(aliasExists(shrunkenIndexName, index))); + assertBusy(() -> assertThat(getStepKeyForIndex(client(), shrunkenIndexName), + equalTo(PhaseCompleteStep.finalStep("warm").getKey()))); + assertBusy(() -> { + Map settings = getOnlyIndexSettings(client(), shrunkenIndexName); + assertThat(settings.get(SETTING_NUMBER_OF_SHARDS), equalTo(String.valueOf(expectedFinalShards))); + assertThat(settings.get(IndexMetadata.INDEX_BLOCKS_WRITE_SETTING.getKey()), equalTo("true")); + assertThat(settings.get(IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id"), nullValue()); + assertThat(settings.get(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey()), equalTo("-1")); + }); + expectThrows(ResponseException.class, () -> indexDocument(client(), index)); + } } From dd889abb921f355280e819bf3dc812dff3ddd299 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 19 Aug 2021 17:33:59 -0500 Subject: [PATCH 2/4] taking code review into account, and fixing a unit test --- .../xpack/core/ilm/SetSingleNodeAllocateStep.java | 3 ++- .../xpack/core/ilm/SetSingleNodeAllocateStepTests.java | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStep.java index 154ad704a1927..934c345be9c4d 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStep.java @@ -97,7 +97,7 @@ public void performAction(IndexMetadata indexMetadata, ClusterState clusterState if (nodeId.isPresent()) { Settings settings = Settings.builder() .put(IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id", nodeId.get()) - .put(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey(), -1).build(); + .putNull(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey()).build(); UpdateSettingsRequest updateSettingsRequest = new UpdateSettingsRequest(indexName) .masterNodeTimeout(TimeValue.MAX_VALUE) .settings(settings); @@ -115,4 +115,5 @@ public void performAction(IndexMetadata indexMetadata, ClusterState clusterState listener.onFailure(new IndexNotFoundException(indexMetadata.getIndex())); } } + } diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStepTests.java index 578f327139a5a..4927e8857413d 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ilm/SetSingleNodeAllocateStepTests.java @@ -85,7 +85,7 @@ public static void assertSettingsRequestContainsValueFrom(UpdateSettingsRequest assertArrayEquals(expectedIndices, request.indices()); assertThat(request.settings().get(settingsKey), anyOf(acceptableValues.stream().map(e -> equalTo(e)).collect(Collectors.toList()))); if (assertOnlyKeyInSettings) { - assertEquals(1, request.settings().size()); + assertEquals(2, request.settings().size()); } } From 0db24d54efc7e54a5e6e9777abaac99067478f7b Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Thu, 19 Aug 2021 17:39:50 -0500 Subject: [PATCH 3/4] fixing IT test --- .../org/elasticsearch/xpack/ilm/actions/ShrinkActionIT.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/ShrinkActionIT.java b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/ShrinkActionIT.java index 974bc772bc5b8..bdbc6094c4c2c 100644 --- a/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/ShrinkActionIT.java +++ b/x-pack/plugin/ilm/qa/multi-node/src/javaRestTest/java/org/elasticsearch/xpack/ilm/actions/ShrinkActionIT.java @@ -328,7 +328,7 @@ public void testTotalShardsPerNodeTooLow() throws Exception { assertThat(settings.get(SETTING_NUMBER_OF_SHARDS), equalTo(String.valueOf(expectedFinalShards))); assertThat(settings.get(IndexMetadata.INDEX_BLOCKS_WRITE_SETTING.getKey()), equalTo("true")); assertThat(settings.get(IndexMetadata.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + "_id"), nullValue()); - assertThat(settings.get(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey()), equalTo("-1")); + assertNull(settings.get(ShardsLimitAllocationDecider.INDEX_TOTAL_SHARDS_PER_NODE_SETTING.getKey())); }); expectThrows(ResponseException.class, () -> indexDocument(client(), index)); } From 7b1db38af00d6b861761500f34696988b35e3d81 Mon Sep 17 00:00:00 2001 From: Keith Massey Date: Fri, 20 Aug 2021 08:39:49 -0500 Subject: [PATCH 4/4] updating wording of docs --- docs/reference/ilm/actions/ilm-shrink.asciidoc | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/reference/ilm/actions/ilm-shrink.asciidoc b/docs/reference/ilm/actions/ilm-shrink.asciidoc index 321e89d110fd9..727124a18ea43 100644 --- a/docs/reference/ilm/actions/ilm-shrink.asciidoc +++ b/docs/reference/ilm/actions/ilm-shrink.asciidoc @@ -17,9 +17,10 @@ stream. You cannot perform the `shrink` action on a write index. To use the `shrink` action in the `hot` phase, the `rollover` action *must* be present. If no rollover action is configured, {ilm-init} will reject the policy. -The shrink action will set the index's index.routing.allocation.total_shards_per_node -setting to -1 to ensure that all shards of the index can be copied to a single node. -This setting will persist on the index even after the step completes. +The shrink action will unset the index's index.routing.allocation.total_shards_per_node +setting, meaning that there will be no limit. This is to ensure that all shards of the +index can be copied to a single node. This setting change will persist on the index +even after the step completes. [IMPORTANT] If the shrink action is used on a <>, policy