From 70d134799f685b78c19e5e369ad837f23f6d4de7 Mon Sep 17 00:00:00 2001 From: Lee Hinman Date: Tue, 6 Nov 2018 17:26:04 -0700 Subject: [PATCH] Remove ALL shard check in CheckShrinkReadyStep Since it's still possible to shrink an index when replicas are unassigned, we should not check that all copies are available when performing the shrink, since we set the allocation requirement for a single node. Resolves #35321 --- .../indexlifecycle/CheckShrinkReadyStep.java | 7 --- .../CheckShrinkReadyStepTests.java | 44 ++++++++++++++++--- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStep.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStep.java index bf29a595c192c..ae6f2f22a95d0 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStep.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStep.java @@ -8,7 +8,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.elasticsearch.action.support.ActiveShardCount; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.routing.IndexRoutingTable; @@ -53,12 +52,6 @@ public Result isConditionMet(Index index, ClusterState clusterState) { // How many shards the node should have int expectedShardCount = idxMeta.getNumberOfShards(); - if (ActiveShardCount.ALL.enoughShardsActive(clusterState, index.getName()) == false) { - logger.debug("[{}] shrink action for [{}] cannot make progress because not all shards are active", - getKey().getAction(), index.getName()); - return new Result(false, new CheckShrinkReadyStep.Info("", expectedShardCount, -1)); - } - // The id of the node the shards should be on final String idShardsShouldBeOn = idxMeta.getSettings().get(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_PREFIX + "._id"); if (idShardsShouldBeOn == null) { diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStepTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStepTests.java index f7a6687dd987c..579ee1630e225 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStepTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/indexlifecycle/CheckShrinkReadyStepTests.java @@ -221,7 +221,7 @@ public void testExecuteAllocateNotCompleteOnlyOneCopyAllocated() throws Exceptio new ClusterStateWaitStep.Result(false, new CheckShrinkReadyStep.Info("node1", 2, 1))); } - public void testExecuteAllocateUnassigned() throws Exception { + public void testExecuteAllocateReplicaUnassigned() { Index index = new Index(randomAlphaOfLengthBetween(1, 20), randomAlphaOfLengthBetween(1, 20)); Map requires = AllocateActionTests.randomMap(1, 5); Settings.Builder existingSettings = Settings.builder() @@ -239,12 +239,12 @@ public void testExecuteAllocateUnassigned() throws Exception { IndexRoutingTable.Builder indexRoutingTable = IndexRoutingTable.builder(index) .addShard(TestShardRouting.newShardRouting(new ShardId(index, 0), "node1", true, ShardRoutingState.STARTED)) - .addShard(TestShardRouting.newShardRouting(new ShardId(index, 1), null, null, true, ShardRoutingState.UNASSIGNED, + .addShard(TestShardRouting.newShardRouting(new ShardId(index, 0), null, null, false, ShardRoutingState.UNASSIGNED, new UnassignedInfo(randomFrom(UnassignedInfo.Reason.values()), "the shard is intentionally unassigned"))); CheckShrinkReadyStep step = createRandomInstance(); - assertAllocateStatus(index, 2, 0, step, existingSettings, node1Settings, node2Settings, indexRoutingTable, - new ClusterStateWaitStep.Result(false, new CheckShrinkReadyStep.Info("", 2, -1))); + assertAllocateStatus(index, 1, 1, step, existingSettings, node1Settings, node2Settings, indexRoutingTable, + new ClusterStateWaitStep.Result(true, null)); } /** @@ -267,7 +267,9 @@ public void testExecuteAllocateUnassigned() throws Exception { public void testExecuteReplicasNotAllocatedOnSingleNode() { Index index = new Index(randomAlphaOfLengthBetween(1, 20), randomAlphaOfLengthBetween(1, 20)); Map requires = Collections.singletonMap("_id", "node1"); - Settings.Builder existingSettings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT.id) + Settings.Builder existingSettings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT.id) + .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_PREFIX + "._id", "node1") .put(IndexMetaData.SETTING_INDEX_UUID, index.getUUID()); Settings.Builder expectedSettings = Settings.builder(); Settings.Builder node1Settings = Settings.builder(); @@ -278,12 +280,40 @@ public void testExecuteReplicasNotAllocatedOnSingleNode() { IndexRoutingTable.Builder indexRoutingTable = IndexRoutingTable.builder(index) .addShard(TestShardRouting.newShardRouting(new ShardId(index, 0), "node1", true, ShardRoutingState.STARTED)) + .addShard(TestShardRouting.newShardRouting(new ShardId(index, 1), "node1", false, ShardRoutingState.STARTED)) + .addShard(TestShardRouting.newShardRouting(new ShardId(index, 1), "node2", true, ShardRoutingState.STARTED)) .addShard(TestShardRouting.newShardRouting(new ShardId(index, 0), null, null, false, ShardRoutingState.UNASSIGNED, new UnassignedInfo(UnassignedInfo.Reason.REPLICA_ADDED, "no attempt"))); CheckShrinkReadyStep step = createRandomInstance(); - assertAllocateStatus(index, 1, 1, step, existingSettings, node1Settings, node2Settings, indexRoutingTable, - new ClusterStateWaitStep.Result(false, new CheckShrinkReadyStep.Info("", 1, -1))); + assertAllocateStatus(index, 2, 1, step, existingSettings, node1Settings, node2Settings, indexRoutingTable, + new ClusterStateWaitStep.Result(true, null)); + } + + public void testExecuteReplicasButCopiesNotPresent() { + Index index = new Index(randomAlphaOfLengthBetween(1, 20), randomAlphaOfLengthBetween(1, 20)); + Map requires = Collections.singletonMap("_id", "node1"); + Settings.Builder existingSettings = Settings.builder() + .put(IndexMetaData.SETTING_VERSION_CREATED, Version.CURRENT.id) + .put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_PREFIX + "._id", "node1") + .put(IndexMetaData.SETTING_INDEX_UUID, index.getUUID()); + Settings.Builder expectedSettings = Settings.builder(); + Settings.Builder node1Settings = Settings.builder(); + Settings.Builder node2Settings = Settings.builder(); + requires.forEach((k, v) -> { + expectedSettings.put(IndexMetaData.INDEX_ROUTING_REQUIRE_GROUP_SETTING.getKey() + k, v); + }); + + IndexRoutingTable.Builder indexRoutingTable = IndexRoutingTable.builder(index) + .addShard(TestShardRouting.newShardRouting(new ShardId(index, 0), "node1", true, ShardRoutingState.STARTED)) + .addShard(TestShardRouting.newShardRouting(new ShardId(index, 1), "node2", false, ShardRoutingState.STARTED)) + .addShard(TestShardRouting.newShardRouting(new ShardId(index, 1), "node3", true, ShardRoutingState.STARTED)) + .addShard(TestShardRouting.newShardRouting(new ShardId(index, 0), null, null, false, ShardRoutingState.UNASSIGNED, + new UnassignedInfo(UnassignedInfo.Reason.REPLICA_ADDED, "no attempt"))); + + CheckShrinkReadyStep step = createRandomInstance(); + assertAllocateStatus(index, 2, 1, step, existingSettings, node1Settings, node2Settings, indexRoutingTable, + new ClusterStateWaitStep.Result(false, new CheckShrinkReadyStep.Info("node1", 2, 1))); } public void testExecuteIndexMissing() throws Exception {