From 2e5625df8110920a901ab129b061f98c0cd12955 Mon Sep 17 00:00:00 2001 From: David Turner Date: Tue, 23 Feb 2021 17:28:54 +0000 Subject: [PATCH] Alloc. awareness should ignore nodes without attr (#69374) Today we count `null` (i.e. missing) as a valid attribute value in allocation awareness, even though allocation awareness forbids the allocation of shards to such a node. Prior to #69334 this didn't matter, a data node without allocation attributes was pointless. However, #69334 means we now can allocate shards to such a node: for instance, there is no need for nodes holding only enrich indices to have allocation attributes. Therefore we should stop counting `null` as one of the attribute values. --- .../cluster/routing/RoutingNodes.java | 4 ++- .../allocation/AwarenessAllocationTests.java | 33 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingNodes.java b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingNodes.java index d399b324263c4..88aaff2f85381 100644 --- a/server/src/main/java/org/elasticsearch/cluster/routing/RoutingNodes.java +++ b/server/src/main/java/org/elasticsearch/cluster/routing/RoutingNodes.java @@ -237,7 +237,9 @@ public ObjectIntHashMap nodesPerAttributesCounts(String attributeName) { nodesPerAttributesCounts = new ObjectIntHashMap<>(); for (RoutingNode routingNode : this) { String attrValue = routingNode.node().getAttributes().get(attributeName); - nodesPerAttributesCounts.addTo(attrValue, 1); + if (attrValue != null) { + nodesPerAttributesCounts.addTo(attrValue, 1); + } } nodesPerAttributeNames.put(attributeName, nodesPerAttributesCounts); return nodesPerAttributesCounts; diff --git a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java index 7a7702c4d7671..c984dcf796816 100644 --- a/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/routing/allocation/AwarenessAllocationTests.java @@ -30,6 +30,7 @@ import java.util.HashMap; import java.util.Map; +import static java.util.Collections.emptyMap; import static java.util.Collections.singletonMap; import static org.elasticsearch.cluster.routing.ShardRoutingState.INITIALIZING; import static org.elasticsearch.cluster.routing.ShardRoutingState.RELOCATING; @@ -904,4 +905,36 @@ public void testDisabledByAutoExpandReplicas() { assertThat(clusterState.getRoutingNodes().shardsWithState(UNASSIGNED), empty()); } + + public void testNodesWithoutAttributeAreIgnored() { + final Settings settings = Settings.builder() + .put(AwarenessAllocationDecider.CLUSTER_ROUTING_ALLOCATION_AWARENESS_ATTRIBUTE_SETTING.getKey(), "zone") + .build(); + + final AllocationService strategy = createAllocationService(settings); + + final Metadata metadata = Metadata.builder() + .put(IndexMetadata.builder("test").settings(settings(Version.CURRENT) + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, 2))) + .build(); + + final ClusterState clusterState = applyStartedShardsUntilNoChange( + ClusterState.builder(ClusterName.CLUSTER_NAME_SETTING.get(Settings.EMPTY)) + .metadata(metadata) + .routingTable(RoutingTable.builder() + .addAsNew(metadata.index("test")) + .build()) + .nodes(DiscoveryNodes.builder() + .add(newNode("A-0", singletonMap("zone", "a"))) + .add(newNode("A-1", singletonMap("zone", "a"))) + .add(newNode("B-0", singletonMap("zone", "b"))) + .add(newNode("B-1", singletonMap("zone", "b"))) + .add(newNode("X-0", emptyMap())) + ).build(), strategy); + + assertThat(clusterState.getRoutingNodes().shardsWithState(UNASSIGNED), empty()); + assertTrue(clusterState.getRoutingNodes().node("X-0").isEmpty()); + } + }