From 33029bbd3a98eacab56054f8aeae6cc6b184bc2d Mon Sep 17 00:00:00 2001 From: Gaurav614 Date: Mon, 17 Jun 2019 20:11:19 +0530 Subject: [PATCH 1/5] Bugfix of issue #41073 Addition of test case that creates the scenario when there are no data nodes in Cluster and user tries for index Creation. Changing the status of primary shards that are unassigned to AllocationStatus.Deciders_NO when there are no data nodes helps in solving this issue --- .../allocator/BalancedShardsAllocator.java | 33 +++- .../health/NoDataNodesHealthTests.java | 153 ++++++++++++++++++ 2 files changed, 185 insertions(+), 1 deletion(-) create mode 100644 server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java index 6af6e6696e033..17d110a2f6478 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java @@ -29,6 +29,7 @@ import org.elasticsearch.cluster.routing.RoutingNodes; import org.elasticsearch.cluster.routing.ShardRouting; import org.elasticsearch.cluster.routing.ShardRoutingState; +import org.elasticsearch.cluster.routing.UnassignedInfo; import org.elasticsearch.cluster.routing.UnassignedInfo.AllocationStatus; import org.elasticsearch.cluster.routing.allocation.AllocateUnassignedDecision; import org.elasticsearch.cluster.routing.allocation.AllocationDecision; @@ -115,7 +116,8 @@ private void setThreshold(float threshold) { @Override public void allocate(RoutingAllocation allocation) { if (allocation.routingNodes().size() == 0) { - /* with no nodes this is pointless */ + // If no data node then set AllocationStatus to DECIDERS_NO + setAllocationStatus(allocation); return; } final Balancer balancer = new Balancer(logger, allocation, weightFunction, threshold); @@ -141,6 +143,35 @@ public ShardAllocationDecision decideShardAllocation(final ShardRouting shard, f return new ShardAllocationDecision(allocateUnassignedDecision, moveDecision); } + /** + * This method is called when there are no data nodes in the cluster. + * + * Newly created unassigned primary shards, with no prior allocation attempts + * are classified as yellow instead of red by cluster health. + * This function explicitly sets their allocation status to DECIDERS_NO, to + * indicate red indices with unassigned shards. + */ + private void setAllocationStatus(RoutingAllocation allocation){ + RoutingNodes routingNodes = allocation.routingNodes(); + RoutingNodes.UnassignedShards unassignedShards = routingNodes.unassigned(); + if (unassignedShards.isEmpty()) { + return; + } + RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = unassignedShards.iterator(); + while (unassignedIterator.hasNext()) { + ShardRouting shard = unassignedIterator.next(); + UnassignedInfo shardInfo = shard.unassignedInfo(); + if (shard.primary() && shardInfo.getLastAllocationStatus() == AllocationStatus.NO_ATTEMPT) { + UnassignedInfo newInfo = new UnassignedInfo(shardInfo.getReason(), shardInfo.getMessage(), shardInfo.getFailure(), + shardInfo.getNumFailedAllocations(), shardInfo.getUnassignedTimeInNanos(), + shardInfo.getUnassignedTimeInMillis(), shardInfo.isDelayed(), + AllocationStatus.DECIDERS_NO); + unassignedIterator.updateUnassigned(newInfo, shard.recoverySource(), allocation.changes()); + } + } + } + + /** * Returns the currently configured delta threshold */ diff --git a/server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java b/server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java new file mode 100644 index 0000000000000..cffaa21c4a5f3 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java @@ -0,0 +1,153 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.cluster.health; + +import org.elasticsearch.Version; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ESAllocationTestCase; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.routing.RoutingNodes; +import org.elasticsearch.cluster.routing.RoutingTable; +import org.elasticsearch.cluster.routing.ShardRouting; +import org.elasticsearch.cluster.routing.UnassignedInfo; +import org.elasticsearch.common.util.set.Sets; + +import java.util.Collections; + +import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; +import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; + +/** + * The test case checks for the scenario when there is no data node in the cluster and only + * master is active. At this moment when index creation is tried, the cluster health status should + * change to RED + */ +public class NoDataNodesHealthTests extends ESAllocationTestCase { + /** + * This method specifically creates a cluster with no data nodes + * and a single master node + */ + private ClusterState setUpClusterWithNoDataNodes() { + + DiscoveryNodes node = DiscoveryNodes.builder().add(newNode("node_m", Collections.singleton(DiscoveryNode.Role.MASTER))).build(); + MetaData metaData = MetaData.builder() + .put(IndexMetaData.builder("TestIndex") + .settings(settings(Version.CURRENT)) + .numberOfShards(randomIntBetween(1, 3)) + .numberOfReplicas(randomIntBetween(0, 2))) + .build(); + RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("TestIndex")).build(); + ClusterState state = ClusterState.builder(new ClusterName("test_cluster")) + .nodes(node) + .metaData(metaData) + .routingTable(routingTable) + .build(); + MockAllocationService service = createAllocationService(); + state = service.reroute(state, "reroute"); + return state; + } + + public void testClusterHealthWithNoDataNodes() { + ClusterState state = setUpClusterWithNoDataNodes(); + int dataNodes = state.nodes().getDataNodes().size(); + int masterNodes = state.nodes().getMasterNodes().size(); + assertTrue(dataNodes == 0); + assertTrue(masterNodes > 0); + ClusterHealthStatus clusterHealthStatus = new ClusterStateHealth(state).getStatus(); + assertEquals(ClusterHealthStatus.RED, clusterHealthStatus); + } + + /** + * The method test for scenario where we have a cluster with indices and data nodes + * and then all data nodes gets terminated now new index is created then + * all indices and cluster health should be red, but last allocation attempts of new index shards v/s + * old index shards should be different. + */ + public void testAllocationStatusForTerminatedNodes() { + //creates one master and two data nodes + DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder().add(newNode("node_m", Collections.singleton(DiscoveryNode.Role.MASTER))) + .add(newNode("node_d1", Collections.singleton(DiscoveryNode.Role.DATA))) + .add(newNode("node_d2", Collections.singleton(DiscoveryNode.Role.DATA))); + MetaData metaData = MetaData.builder() + .put(IndexMetaData.builder("TestIndex") + .settings(settings(Version.CURRENT)) + .numberOfShards(2) + .numberOfReplicas(0)) + .build(); + RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("TestIndex")).build(); + ClusterState state = ClusterState.builder(new ClusterName("test_cluster")) + .nodes(nodeBuilder.build()) + .metaData(metaData) + .routingTable(routingTable) + .build(); + MockAllocationService allocationService = createAllocationService(); + //perform allocation of TestIndex + state = allocationService.reroute(state, "Test_allocation"); + state = allocationService.applyStartedShards(state, state.getRoutingNodes().shardsWithState(INITIALIZING)); + IndexMetaData.Builder idxMetaBuilder = IndexMetaData.builder(state.metaData().index("TestIndex")); + for (final ShardRouting shards : state.getRoutingTable().index("TestIndex").shardsWithState(STARTED)) { + idxMetaBuilder.putInSyncAllocationIds(shards.getId(), Sets.newHashSet(shards.allocationId().getId())); + } + state = ClusterState.builder(state).metaData(MetaData.builder(state.metaData()).put(idxMetaBuilder)).build(); + //asserting the cluster is in green after TestIndex Creation + assertEquals(ClusterHealthStatus.GREEN, new ClusterStateHealth(state).getStatus()); + //Terminating data nodes + state = ClusterState.builder(state) + .nodes(DiscoveryNodes.builder(state.getNodes()) + .remove("node_d1").remove("node_d2").build()) + .build(); + //Removing dead nodes from cluster with a cluster reroute + state = allocationService.deassociateDeadNodes(state, true, "Test_allocation"); + //asserting that a cluster state goes Red after data nodes goes terminated + assertEquals(ClusterHealthStatus.RED, new ClusterStateHealth(state).getStatus()); + //Creating NewTestIndex meta deta + metaData = MetaData.builder(state.metaData()) + .put(IndexMetaData.builder("NewTestIndex") + .settings(settings(Version.CURRENT)) + .numberOfShards(2) + .numberOfReplicas(0)) + .build(); + //changed cluster state + state = ClusterState.builder(state) + .metaData(metaData) + .routingTable(RoutingTable.builder(state.getRoutingTable()).addAsNew(metaData.index("NewTestIndex")).build()).build(); + //allocation after newly created index + state = allocationService.reroute(state, "no data nodes"); + assertEquals(ClusterHealthStatus.RED, new ClusterStateHealth(state).getStatus()); + RoutingNodes routingNodes = state.getRoutingNodes(); + RoutingNodes.UnassignedShards unassignedShards = routingNodes.unassigned(); + assertFalse(unassignedShards.isEmpty()); + RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = unassignedShards.iterator(); + while (unassignedIterator.hasNext()) { + ShardRouting shard = unassignedIterator.next(); + UnassignedInfo shardInfo = shard.unassignedInfo(); + /* asserting that the TestIndex shards have different AllocationStatus than DECIDERS_NO + and NewTestIndex status is DECIDERS_NO */ + if (shard.getIndexName().equals("TestIndex")) { + assertNotEquals(shardInfo.getLastAllocationStatus(), UnassignedInfo.AllocationStatus.DECIDERS_NO); + } else if (shard.getIndexName().equals("NewTestIndex")) { + assertEquals(shardInfo.getLastAllocationStatus(), UnassignedInfo.AllocationStatus.DECIDERS_NO); + } + } + } +} From bd1edbf1438ef65a73f78344fae1a245b6194b07 Mon Sep 17 00:00:00 2001 From: Gaurav614 Date: Thu, 25 Jul 2019 23:36:07 +0530 Subject: [PATCH 2/5] Modified Test case and Documentation --- .../allocator/BalancedShardsAllocator.java | 31 ++-- .../health/ClusterHealthAllocationTests.java | 102 ++++++++++++ .../health/NoDataNodesHealthTests.java | 153 ------------------ 3 files changed, 112 insertions(+), 174 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java delete mode 100644 server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java index 17d110a2f6478..78d39ba9ba1cb 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java @@ -117,7 +117,7 @@ private void setThreshold(float threshold) { public void allocate(RoutingAllocation allocation) { if (allocation.routingNodes().size() == 0) { // If no data node then set AllocationStatus to DECIDERS_NO - setAllocationStatus(allocation); + failedAllocationOfNewPrimaries(allocation); return; } final Balancer balancer = new Balancer(logger, allocation, weightFunction, threshold); @@ -143,30 +143,19 @@ public ShardAllocationDecision decideShardAllocation(final ShardRouting shard, f return new ShardAllocationDecision(allocateUnassignedDecision, moveDecision); } - /** - * This method is called when there are no data nodes in the cluster. - * - * Newly created unassigned primary shards, with no prior allocation attempts - * are classified as yellow instead of red by cluster health. - * This function explicitly sets their allocation status to DECIDERS_NO, to - * indicate red indices with unassigned shards. - */ - private void setAllocationStatus(RoutingAllocation allocation){ + private void failedAllocationOfNewPrimaries(RoutingAllocation allocation){ RoutingNodes routingNodes = allocation.routingNodes(); + assert routingNodes.size() == 0 : routingNodes; RoutingNodes.UnassignedShards unassignedShards = routingNodes.unassigned(); - if (unassignedShards.isEmpty()) { - return; - } RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = unassignedShards.iterator(); while (unassignedIterator.hasNext()) { - ShardRouting shard = unassignedIterator.next(); - UnassignedInfo shardInfo = shard.unassignedInfo(); - if (shard.primary() && shardInfo.getLastAllocationStatus() == AllocationStatus.NO_ATTEMPT) { - UnassignedInfo newInfo = new UnassignedInfo(shardInfo.getReason(), shardInfo.getMessage(), shardInfo.getFailure(), - shardInfo.getNumFailedAllocations(), shardInfo.getUnassignedTimeInNanos(), - shardInfo.getUnassignedTimeInMillis(), shardInfo.isDelayed(), - AllocationStatus.DECIDERS_NO); - unassignedIterator.updateUnassigned(newInfo, shard.recoverySource(), allocation.changes()); + ShardRouting shardRouting = unassignedIterator.next(); + UnassignedInfo unassignedInfo = shardRouting.unassignedInfo(); + if (shardRouting.primary() && unassignedInfo.getLastAllocationStatus() == AllocationStatus.NO_ATTEMPT) { + unassignedIterator.updateUnassigned(new UnassignedInfo(unassignedInfo.getReason(), unassignedInfo.getMessage(), unassignedInfo.getFailure(), + unassignedInfo.getNumFailedAllocations(), unassignedInfo.getUnassignedTimeInNanos(), + unassignedInfo.getUnassignedTimeInMillis(), unassignedInfo.isDelayed(), + AllocationStatus.DECIDERS_NO), shardRouting.recoverySource(), allocation.changes()); } } } diff --git a/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java b/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java new file mode 100644 index 0000000000000..82f72eb8836ab --- /dev/null +++ b/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java @@ -0,0 +1,102 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.elasticsearch.cluster.health; + +import org.elasticsearch.Version; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ESAllocationTestCase; +import org.elasticsearch.cluster.metadata.IndexMetaData; +import org.elasticsearch.cluster.metadata.MetaData; +import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodes; +import org.elasticsearch.cluster.routing.RoutingTable; + +import java.util.Collections; + +import static org.elasticsearch.cluster.routing.ShardRoutingState.*; + +public class ClusterHealthAllocationTests extends ESAllocationTestCase { + public void testClusterHealth() { + ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); + clusterState = addNode(clusterState, "node_m", true); + assertEquals(ClusterHealthStatus.GREEN, getClusterHealthStatus(clusterState)); + MetaData metaData = MetaData.builder() + .put(IndexMetaData.builder("test") + .settings(settings(Version.CURRENT)) + .numberOfShards(2) + .numberOfReplicas(1)) + .build(); + RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("test")).build(); + clusterState = ClusterState.builder(clusterState).metaData(metaData).routingTable(routingTable).build(); + MockAllocationService allocation = createAllocationService(); + clusterState = allocation.reroute(clusterState, "reroute"); + assertTrue(clusterState.nodes().getDataNodes().size() == 0); + assertTrue(clusterState.nodes().getMasterNodes().size() == 1); + assertEquals(ClusterHealthStatus.RED, getClusterHealthStatus(clusterState)); + clusterState = addNode(clusterState, "node_d1", false); + assertTrue(clusterState.nodes().getDataNodes().size() == 1); + clusterState = allocation.reroute(clusterState, "reroute"); + // starting primaries + clusterState = allocation.applyStartedShards(clusterState, clusterState.getRoutingNodes().shardsWithState(INITIALIZING)); + assertEquals(ClusterHealthStatus.YELLOW, getClusterHealthStatus(clusterState)); + clusterState = addNode(clusterState, "node_d2", false); + clusterState = allocation.reroute(clusterState, "reroute"); + // starting replicas + clusterState = allocation.applyStartedShards(clusterState, clusterState.getRoutingNodes().shardsWithState(INITIALIZING)); + assertEquals(ClusterHealthStatus.GREEN, getClusterHealthStatus(clusterState)); + clusterState = removeNode(clusterState, "node_d1"); + assertEquals(ClusterHealthStatus.YELLOW, getClusterHealthStatus(clusterState)); + clusterState = removeNode(clusterState, "node_d2"); + assertEquals(ClusterHealthStatus.RED, getClusterHealthStatus(clusterState)); + routingTable = RoutingTable.builder(routingTable).remove("test").build(); + metaData = MetaData.builder(clusterState.metaData()).remove("test").build(); + clusterState = ClusterState.builder(clusterState).routingTable(routingTable).metaData(metaData).build(); + assertTrue(clusterState.nodes().getDataNodes().size() == 0); + assertTrue(clusterState.nodes().getMasterNodes().size() == 1); + assertEquals(ClusterHealthStatus.GREEN, getClusterHealthStatus(clusterState)); + } + + private ClusterState addNode(ClusterState clusterState, String nodeName, boolean isMaster) { + + DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.getNodes()); + if (isMaster) { + nodeBuilder = nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNode.Role.MASTER))); + } else { + nodeBuilder = nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNode.Role.DATA))); + } + clusterState = ClusterState.builder(clusterState) + .nodes(nodeBuilder.build()) + .build(); + return clusterState; + } + + private ClusterState removeNode(ClusterState clusterState, String nodeName) { + clusterState = ClusterState.builder(clusterState) + .nodes(DiscoveryNodes.builder(clusterState.getNodes()) + .remove(nodeName).build()) + .build(); + clusterState = createAllocationService().deassociateDeadNodes(clusterState, true, "reroute"); + return clusterState; + } + + private ClusterHealthStatus getClusterHealthStatus(ClusterState clusterState) { + return new ClusterStateHealth(clusterState).getStatus(); + } +} diff --git a/server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java b/server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java deleted file mode 100644 index cffaa21c4a5f3..0000000000000 --- a/server/src/test/java/org/elasticsearch/cluster/health/NoDataNodesHealthTests.java +++ /dev/null @@ -1,153 +0,0 @@ -/* - * Licensed to Elasticsearch under one or more contributor - * license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright - * ownership. Elasticsearch licenses this file to you under - * the Apache License, Version 2.0 (the "License"); you may - * not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -package org.elasticsearch.cluster.health; - -import org.elasticsearch.Version; -import org.elasticsearch.cluster.ClusterName; -import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.cluster.ESAllocationTestCase; -import org.elasticsearch.cluster.metadata.IndexMetaData; -import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.node.DiscoveryNode; -import org.elasticsearch.cluster.node.DiscoveryNodes; -import org.elasticsearch.cluster.routing.RoutingNodes; -import org.elasticsearch.cluster.routing.RoutingTable; -import org.elasticsearch.cluster.routing.ShardRouting; -import org.elasticsearch.cluster.routing.UnassignedInfo; -import org.elasticsearch.common.util.set.Sets; - -import java.util.Collections; - -import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; -import static org.elasticsearch.cluster.routing.ShardRoutingState.STARTED; - -/** - * The test case checks for the scenario when there is no data node in the cluster and only - * master is active. At this moment when index creation is tried, the cluster health status should - * change to RED - */ -public class NoDataNodesHealthTests extends ESAllocationTestCase { - /** - * This method specifically creates a cluster with no data nodes - * and a single master node - */ - private ClusterState setUpClusterWithNoDataNodes() { - - DiscoveryNodes node = DiscoveryNodes.builder().add(newNode("node_m", Collections.singleton(DiscoveryNode.Role.MASTER))).build(); - MetaData metaData = MetaData.builder() - .put(IndexMetaData.builder("TestIndex") - .settings(settings(Version.CURRENT)) - .numberOfShards(randomIntBetween(1, 3)) - .numberOfReplicas(randomIntBetween(0, 2))) - .build(); - RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("TestIndex")).build(); - ClusterState state = ClusterState.builder(new ClusterName("test_cluster")) - .nodes(node) - .metaData(metaData) - .routingTable(routingTable) - .build(); - MockAllocationService service = createAllocationService(); - state = service.reroute(state, "reroute"); - return state; - } - - public void testClusterHealthWithNoDataNodes() { - ClusterState state = setUpClusterWithNoDataNodes(); - int dataNodes = state.nodes().getDataNodes().size(); - int masterNodes = state.nodes().getMasterNodes().size(); - assertTrue(dataNodes == 0); - assertTrue(masterNodes > 0); - ClusterHealthStatus clusterHealthStatus = new ClusterStateHealth(state).getStatus(); - assertEquals(ClusterHealthStatus.RED, clusterHealthStatus); - } - - /** - * The method test for scenario where we have a cluster with indices and data nodes - * and then all data nodes gets terminated now new index is created then - * all indices and cluster health should be red, but last allocation attempts of new index shards v/s - * old index shards should be different. - */ - public void testAllocationStatusForTerminatedNodes() { - //creates one master and two data nodes - DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder().add(newNode("node_m", Collections.singleton(DiscoveryNode.Role.MASTER))) - .add(newNode("node_d1", Collections.singleton(DiscoveryNode.Role.DATA))) - .add(newNode("node_d2", Collections.singleton(DiscoveryNode.Role.DATA))); - MetaData metaData = MetaData.builder() - .put(IndexMetaData.builder("TestIndex") - .settings(settings(Version.CURRENT)) - .numberOfShards(2) - .numberOfReplicas(0)) - .build(); - RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("TestIndex")).build(); - ClusterState state = ClusterState.builder(new ClusterName("test_cluster")) - .nodes(nodeBuilder.build()) - .metaData(metaData) - .routingTable(routingTable) - .build(); - MockAllocationService allocationService = createAllocationService(); - //perform allocation of TestIndex - state = allocationService.reroute(state, "Test_allocation"); - state = allocationService.applyStartedShards(state, state.getRoutingNodes().shardsWithState(INITIALIZING)); - IndexMetaData.Builder idxMetaBuilder = IndexMetaData.builder(state.metaData().index("TestIndex")); - for (final ShardRouting shards : state.getRoutingTable().index("TestIndex").shardsWithState(STARTED)) { - idxMetaBuilder.putInSyncAllocationIds(shards.getId(), Sets.newHashSet(shards.allocationId().getId())); - } - state = ClusterState.builder(state).metaData(MetaData.builder(state.metaData()).put(idxMetaBuilder)).build(); - //asserting the cluster is in green after TestIndex Creation - assertEquals(ClusterHealthStatus.GREEN, new ClusterStateHealth(state).getStatus()); - //Terminating data nodes - state = ClusterState.builder(state) - .nodes(DiscoveryNodes.builder(state.getNodes()) - .remove("node_d1").remove("node_d2").build()) - .build(); - //Removing dead nodes from cluster with a cluster reroute - state = allocationService.deassociateDeadNodes(state, true, "Test_allocation"); - //asserting that a cluster state goes Red after data nodes goes terminated - assertEquals(ClusterHealthStatus.RED, new ClusterStateHealth(state).getStatus()); - //Creating NewTestIndex meta deta - metaData = MetaData.builder(state.metaData()) - .put(IndexMetaData.builder("NewTestIndex") - .settings(settings(Version.CURRENT)) - .numberOfShards(2) - .numberOfReplicas(0)) - .build(); - //changed cluster state - state = ClusterState.builder(state) - .metaData(metaData) - .routingTable(RoutingTable.builder(state.getRoutingTable()).addAsNew(metaData.index("NewTestIndex")).build()).build(); - //allocation after newly created index - state = allocationService.reroute(state, "no data nodes"); - assertEquals(ClusterHealthStatus.RED, new ClusterStateHealth(state).getStatus()); - RoutingNodes routingNodes = state.getRoutingNodes(); - RoutingNodes.UnassignedShards unassignedShards = routingNodes.unassigned(); - assertFalse(unassignedShards.isEmpty()); - RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = unassignedShards.iterator(); - while (unassignedIterator.hasNext()) { - ShardRouting shard = unassignedIterator.next(); - UnassignedInfo shardInfo = shard.unassignedInfo(); - /* asserting that the TestIndex shards have different AllocationStatus than DECIDERS_NO - and NewTestIndex status is DECIDERS_NO */ - if (shard.getIndexName().equals("TestIndex")) { - assertNotEquals(shardInfo.getLastAllocationStatus(), UnassignedInfo.AllocationStatus.DECIDERS_NO); - } else if (shard.getIndexName().equals("NewTestIndex")) { - assertEquals(shardInfo.getLastAllocationStatus(), UnassignedInfo.AllocationStatus.DECIDERS_NO); - } - } - } -} From 82c1e289869b0dc3b8497b71ef17002170f8ce30 Mon Sep 17 00:00:00 2001 From: Gaurav614 Date: Sat, 21 Sep 2019 13:46:28 +0530 Subject: [PATCH 3/5] Resolved PR comments --- .../allocator/BalancedShardsAllocator.java | 14 +++-- .../health/ClusterHealthAllocationTests.java | 51 +++++++++---------- 2 files changed, 30 insertions(+), 35 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java index 78d39ba9ba1cb..2bdb21999ec45 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java @@ -115,9 +115,8 @@ private void setThreshold(float threshold) { @Override public void allocate(RoutingAllocation allocation) { - if (allocation.routingNodes().size() == 0) { - // If no data node then set AllocationStatus to DECIDERS_NO - failedAllocationOfNewPrimaries(allocation); + if (allocation.routingNodes().size() == 0){ + failAllocationOfNewPrimaries(allocation); return; } final Balancer balancer = new Balancer(logger, allocation, weightFunction, threshold); @@ -143,14 +142,13 @@ public ShardAllocationDecision decideShardAllocation(final ShardRouting shard, f return new ShardAllocationDecision(allocateUnassignedDecision, moveDecision); } - private void failedAllocationOfNewPrimaries(RoutingAllocation allocation){ + private void failAllocationOfNewPrimaries(RoutingAllocation allocation){ RoutingNodes routingNodes = allocation.routingNodes(); assert routingNodes.size() == 0 : routingNodes; - RoutingNodes.UnassignedShards unassignedShards = routingNodes.unassigned(); - RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = unassignedShards.iterator(); + final RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = routingNodes.unassigned().iterator(); while (unassignedIterator.hasNext()) { - ShardRouting shardRouting = unassignedIterator.next(); - UnassignedInfo unassignedInfo = shardRouting.unassignedInfo(); + final ShardRouting shardRouting = unassignedIterator.next(); + final UnassignedInfo unassignedInfo = shardRouting.unassignedInfo(); if (shardRouting.primary() && unassignedInfo.getLastAllocationStatus() == AllocationStatus.NO_ATTEMPT) { unassignedIterator.updateUnassigned(new UnassignedInfo(unassignedInfo.getReason(), unassignedInfo.getMessage(), unassignedInfo.getFailure(), unassignedInfo.getNumFailedAllocations(), unassignedInfo.getUnassignedTimeInNanos(), diff --git a/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java b/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java index 82f72eb8836ab..148e2023492e1 100644 --- a/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java @@ -24,19 +24,21 @@ import org.elasticsearch.cluster.ESAllocationTestCase; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.cluster.metadata.MetaData; -import org.elasticsearch.cluster.node.DiscoveryNode; +import org.elasticsearch.cluster.node.DiscoveryNodeRole; import org.elasticsearch.cluster.node.DiscoveryNodes; import org.elasticsearch.cluster.routing.RoutingTable; +import org.elasticsearch.cluster.routing.allocation.AllocationService; import java.util.Collections; -import static org.elasticsearch.cluster.routing.ShardRoutingState.*; - public class ClusterHealthAllocationTests extends ESAllocationTestCase { public void testClusterHealth() { ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); - clusterState = addNode(clusterState, "node_m", true); + if (randomBoolean()) { + clusterState = addNode(clusterState, "node_m", true); + } assertEquals(ClusterHealthStatus.GREEN, getClusterHealthStatus(clusterState)); + MetaData metaData = MetaData.builder() .put(IndexMetaData.builder("test") .settings(settings(Version.CURRENT)) @@ -46,30 +48,29 @@ public void testClusterHealth() { RoutingTable routingTable = RoutingTable.builder().addAsNew(metaData.index("test")).build(); clusterState = ClusterState.builder(clusterState).metaData(metaData).routingTable(routingTable).build(); MockAllocationService allocation = createAllocationService(); - clusterState = allocation.reroute(clusterState, "reroute"); - assertTrue(clusterState.nodes().getDataNodes().size() == 0); - assertTrue(clusterState.nodes().getMasterNodes().size() == 1); + clusterState = applyStartedShardsUntilNoChange(clusterState, allocation); + assertEquals(0, clusterState.nodes().getDataNodes().size()); assertEquals(ClusterHealthStatus.RED, getClusterHealthStatus(clusterState)); + clusterState = addNode(clusterState, "node_d1", false); - assertTrue(clusterState.nodes().getDataNodes().size() == 1); - clusterState = allocation.reroute(clusterState, "reroute"); - // starting primaries - clusterState = allocation.applyStartedShards(clusterState, clusterState.getRoutingNodes().shardsWithState(INITIALIZING)); + assertEquals(1, clusterState.nodes().getDataNodes().size()); + clusterState = applyStartedShardsUntilNoChange(clusterState, allocation); assertEquals(ClusterHealthStatus.YELLOW, getClusterHealthStatus(clusterState)); + clusterState = addNode(clusterState, "node_d2", false); - clusterState = allocation.reroute(clusterState, "reroute"); - // starting replicas - clusterState = allocation.applyStartedShards(clusterState, clusterState.getRoutingNodes().shardsWithState(INITIALIZING)); + clusterState = applyStartedShardsUntilNoChange(clusterState, allocation); assertEquals(ClusterHealthStatus.GREEN, getClusterHealthStatus(clusterState)); - clusterState = removeNode(clusterState, "node_d1"); + + clusterState = removeNode(clusterState, "node_d1",allocation); assertEquals(ClusterHealthStatus.YELLOW, getClusterHealthStatus(clusterState)); - clusterState = removeNode(clusterState, "node_d2"); + + clusterState = removeNode(clusterState, "node_d2",allocation); assertEquals(ClusterHealthStatus.RED, getClusterHealthStatus(clusterState)); + routingTable = RoutingTable.builder(routingTable).remove("test").build(); metaData = MetaData.builder(clusterState.metaData()).remove("test").build(); clusterState = ClusterState.builder(clusterState).routingTable(routingTable).metaData(metaData).build(); - assertTrue(clusterState.nodes().getDataNodes().size() == 0); - assertTrue(clusterState.nodes().getMasterNodes().size() == 1); + assertEquals(0, clusterState.nodes().getDataNodes().size()); assertEquals(ClusterHealthStatus.GREEN, getClusterHealthStatus(clusterState)); } @@ -77,23 +78,19 @@ private ClusterState addNode(ClusterState clusterState, String nodeName, boolean DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.getNodes()); if (isMaster) { - nodeBuilder = nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNode.Role.MASTER))); + nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNodeRole.MASTER_ROLE))); } else { - nodeBuilder = nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNode.Role.DATA))); + nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNodeRole.DATA_ROLE))); } - clusterState = ClusterState.builder(clusterState) - .nodes(nodeBuilder.build()) - .build(); - return clusterState; + return ClusterState.builder(clusterState).nodes(nodeBuilder).build(); } - private ClusterState removeNode(ClusterState clusterState, String nodeName) { + private ClusterState removeNode(ClusterState clusterState, String nodeName, AllocationService allocationService) { clusterState = ClusterState.builder(clusterState) .nodes(DiscoveryNodes.builder(clusterState.getNodes()) .remove(nodeName).build()) .build(); - clusterState = createAllocationService().deassociateDeadNodes(clusterState, true, "reroute"); - return clusterState; + return allocationService.disassociateDeadNodes(clusterState, true, "reroute"); } private ClusterHealthStatus getClusterHealthStatus(ClusterState clusterState) { From 40d410dff643dab306ea71cfc97e5555c935be6c Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 30 Sep 2019 11:55:32 +0100 Subject: [PATCH 4/5] Whitespace --- .../allocator/BalancedShardsAllocator.java | 13 ++++++------ .../health/ClusterHealthAllocationTests.java | 20 +++++++------------ 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java index 2bdb21999ec45..a1549c5e217a4 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/allocation/allocator/BalancedShardsAllocator.java @@ -115,7 +115,7 @@ private void setThreshold(float threshold) { @Override public void allocate(RoutingAllocation allocation) { - if (allocation.routingNodes().size() == 0){ + if (allocation.routingNodes().size() == 0) { failAllocationOfNewPrimaries(allocation); return; } @@ -142,7 +142,7 @@ public ShardAllocationDecision decideShardAllocation(final ShardRouting shard, f return new ShardAllocationDecision(allocateUnassignedDecision, moveDecision); } - private void failAllocationOfNewPrimaries(RoutingAllocation allocation){ + private void failAllocationOfNewPrimaries(RoutingAllocation allocation) { RoutingNodes routingNodes = allocation.routingNodes(); assert routingNodes.size() == 0 : routingNodes; final RoutingNodes.UnassignedShards.UnassignedIterator unassignedIterator = routingNodes.unassigned().iterator(); @@ -150,15 +150,14 @@ private void failAllocationOfNewPrimaries(RoutingAllocation allocation){ final ShardRouting shardRouting = unassignedIterator.next(); final UnassignedInfo unassignedInfo = shardRouting.unassignedInfo(); if (shardRouting.primary() && unassignedInfo.getLastAllocationStatus() == AllocationStatus.NO_ATTEMPT) { - unassignedIterator.updateUnassigned(new UnassignedInfo(unassignedInfo.getReason(), unassignedInfo.getMessage(), unassignedInfo.getFailure(), - unassignedInfo.getNumFailedAllocations(), unassignedInfo.getUnassignedTimeInNanos(), - unassignedInfo.getUnassignedTimeInMillis(), unassignedInfo.isDelayed(), - AllocationStatus.DECIDERS_NO), shardRouting.recoverySource(), allocation.changes()); + unassignedIterator.updateUnassigned(new UnassignedInfo(unassignedInfo.getReason(), unassignedInfo.getMessage(), + unassignedInfo.getFailure(), unassignedInfo.getNumFailedAllocations(), unassignedInfo.getUnassignedTimeInNanos(), + unassignedInfo.getUnassignedTimeInMillis(), unassignedInfo.isDelayed(), AllocationStatus.DECIDERS_NO), + shardRouting.recoverySource(), allocation.changes()); } } } - /** * Returns the currently configured delta threshold */ diff --git a/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java b/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java index 148e2023492e1..52adcd503dad0 100644 --- a/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/health/ClusterHealthAllocationTests.java @@ -32,6 +32,7 @@ import java.util.Collections; public class ClusterHealthAllocationTests extends ESAllocationTestCase { + public void testClusterHealth() { ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); if (randomBoolean()) { @@ -61,10 +62,10 @@ public void testClusterHealth() { clusterState = applyStartedShardsUntilNoChange(clusterState, allocation); assertEquals(ClusterHealthStatus.GREEN, getClusterHealthStatus(clusterState)); - clusterState = removeNode(clusterState, "node_d1",allocation); + clusterState = removeNode(clusterState, "node_d1", allocation); assertEquals(ClusterHealthStatus.YELLOW, getClusterHealthStatus(clusterState)); - clusterState = removeNode(clusterState, "node_d2",allocation); + clusterState = removeNode(clusterState, "node_d2", allocation); assertEquals(ClusterHealthStatus.RED, getClusterHealthStatus(clusterState)); routingTable = RoutingTable.builder(routingTable).remove("test").build(); @@ -75,25 +76,18 @@ public void testClusterHealth() { } private ClusterState addNode(ClusterState clusterState, String nodeName, boolean isMaster) { - DiscoveryNodes.Builder nodeBuilder = DiscoveryNodes.builder(clusterState.getNodes()); - if (isMaster) { - nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNodeRole.MASTER_ROLE))); - } else { - nodeBuilder.add(newNode(nodeName, Collections.singleton(DiscoveryNodeRole.DATA_ROLE))); - } + nodeBuilder.add(newNode(nodeName, Collections.singleton(isMaster ? DiscoveryNodeRole.MASTER_ROLE : DiscoveryNodeRole.DATA_ROLE))); return ClusterState.builder(clusterState).nodes(nodeBuilder).build(); } private ClusterState removeNode(ClusterState clusterState, String nodeName, AllocationService allocationService) { - clusterState = ClusterState.builder(clusterState) - .nodes(DiscoveryNodes.builder(clusterState.getNodes()) - .remove(nodeName).build()) - .build(); - return allocationService.disassociateDeadNodes(clusterState, true, "reroute"); + return allocationService.disassociateDeadNodes(ClusterState.builder(clusterState) + .nodes(DiscoveryNodes.builder(clusterState.getNodes()).remove(nodeName)).build(), true, "reroute"); } private ClusterHealthStatus getClusterHealthStatus(ClusterState clusterState) { return new ClusterStateHealth(clusterState).getStatus(); } + } From 3c57592a1f2cf05f9ed5ab40140b9c01b7cdae52 Mon Sep 17 00:00:00 2001 From: David Turner Date: Mon, 30 Sep 2019 12:44:59 +0100 Subject: [PATCH 5/5] TODOs are done! --- .../java/org/elasticsearch/cluster/SimpleDataNodesIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/cluster/SimpleDataNodesIT.java b/server/src/test/java/org/elasticsearch/cluster/SimpleDataNodesIT.java index 82744aa86f93b..1bd6439c2fc5d 100644 --- a/server/src/test/java/org/elasticsearch/cluster/SimpleDataNodesIT.java +++ b/server/src/test/java/org/elasticsearch/cluster/SimpleDataNodesIT.java @@ -85,7 +85,7 @@ public void testShardsAllocatedAfterDataNodesStart() { final ClusterHealthResponse healthResponse1 = client().admin().cluster().prepareHealth() .setWaitForEvents(Priority.LANGUID).execute().actionGet(); assertThat(healthResponse1.isTimedOut(), equalTo(false)); - assertThat(healthResponse1.getStatus(), equalTo(ClusterHealthStatus.YELLOW)); // TODO should be RED, see #41073 + assertThat(healthResponse1.getStatus(), equalTo(ClusterHealthStatus.RED)); assertThat(healthResponse1.getActiveShards(), equalTo(0)); internalCluster().startNode(Settings.builder().put(Node.NODE_DATA_SETTING.getKey(), true).build()); @@ -104,7 +104,7 @@ public void testAutoExpandReplicasAdjustedWhenDataNodeJoins() { final ClusterHealthResponse healthResponse1 = client().admin().cluster().prepareHealth() .setWaitForEvents(Priority.LANGUID).execute().actionGet(); assertThat(healthResponse1.isTimedOut(), equalTo(false)); - assertThat(healthResponse1.getStatus(), equalTo(ClusterHealthStatus.YELLOW)); // TODO should be RED, see #41073 + assertThat(healthResponse1.getStatus(), equalTo(ClusterHealthStatus.RED)); assertThat(healthResponse1.getActiveShards(), equalTo(0)); internalCluster().startNode();