From 950c4363bfe45a676e74ac68eaaec3da0ad499b3 Mon Sep 17 00:00:00 2001 From: Jason Tedor Date: Fri, 9 Mar 2018 18:23:16 -0500 Subject: [PATCH] Remove special handling for _all in nodes info Today when requesting _all we return all nodes regardless of what other node qualifiers are in the request. This is contrary to how the remainder of the API behaves which acts as additive and subtractive based on the qualifiers and their ordering. It is also contrary to how the wildcard * behaves. This commit removes the special handling for _all so that it behaves identical to the wildcard *. Relates #28971 --- .../cluster/node/DiscoveryNodes.java | 27 ++++---------- .../cluster/node/DiscoveryNodesTests.java | 37 +++++++++++++++++++ 2 files changed, 44 insertions(+), 20 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java index 057d37d5999a2..5522e37f71a18 100644 --- a/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java +++ b/server/src/main/java/org/elasticsearch/cluster/node/DiscoveryNodes.java @@ -40,6 +40,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.stream.StreamSupport; /** * This class holds all {@link DiscoveryNode} in the cluster and provides convenience methods to @@ -232,10 +233,6 @@ public DiscoveryNode findByAddress(TransportAddress address) { return null; } - public boolean isAllNodes(String... nodesIds) { - return nodesIds == null || nodesIds.length == 0 || (nodesIds.length == 1 && nodesIds[0].equals("_all")); - } - /** * Returns the version of the node with the oldest version in the cluster that is not a client node * @@ -304,13 +301,8 @@ public DiscoveryNode resolveNode(String node) { * or a generic node attribute name in which case value will be treated as a wildcard and matched against the node attribute values. */ public String[] resolveNodes(String... nodes) { - if (isAllNodes(nodes)) { - int index = 0; - nodes = new String[this.nodes.size()]; - for (DiscoveryNode node : this) { - nodes[index++] = node.getId(); - } - return nodes; + if (nodes == null || nodes.length == 0) { + return StreamSupport.stream(this.spliterator(), false).map(DiscoveryNode::getId).toArray(String[]::new); } else { ObjectHashSet resolvedNodesIds = new ObjectHashSet<>(nodes.length); for (String nodeId : nodes) { @@ -327,16 +319,11 @@ public String[] resolveNodes(String... nodes) { } else if (nodeExists(nodeId)) { resolvedNodesIds.add(nodeId); } else { - // not a node id, try and search by name - for (DiscoveryNode node : this) { - if (Regex.simpleMatch(nodeId, node.getName())) { - resolvedNodesIds.add(node.getId()); - } - } for (DiscoveryNode node : this) { - if (Regex.simpleMatch(nodeId, node.getHostAddress())) { - resolvedNodesIds.add(node.getId()); - } else if (Regex.simpleMatch(nodeId, node.getHostName())) { + if ("_all".equals(nodeId) + || Regex.simpleMatch(nodeId, node.getName()) + || Regex.simpleMatch(nodeId, node.getHostAddress()) + || Regex.simpleMatch(nodeId, node.getHostName())) { resolvedNodesIds.add(node.getId()); } } diff --git a/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java b/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java index 37cc11da8b7b6..80f2401ed7735 100644 --- a/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java +++ b/server/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodesTests.java @@ -35,9 +35,11 @@ import java.util.Set; import java.util.concurrent.atomic.AtomicInteger; import java.util.stream.Collectors; +import java.util.stream.StreamSupport; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.Matchers.arrayContainingInAnyOrder; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.nullValue; @@ -70,6 +72,41 @@ public void testResolveNodeByAttribute() { } } + public void testAll() { + final DiscoveryNodes discoveryNodes = buildDiscoveryNodes(); + + final String[] allNodes = + StreamSupport.stream(discoveryNodes.spliterator(), false).map(DiscoveryNode::getId).toArray(String[]::new); + assertThat(discoveryNodes.resolveNodes(), arrayContainingInAnyOrder(allNodes)); + assertThat(discoveryNodes.resolveNodes(new String[0]), arrayContainingInAnyOrder(allNodes)); + assertThat(discoveryNodes.resolveNodes("_all"), arrayContainingInAnyOrder(allNodes)); + + final String[] nonMasterNodes = + StreamSupport.stream(discoveryNodes.getNodes().values().spliterator(), false) + .map(n -> n.value) + .filter(n -> n.isMasterNode() == false) + .map(DiscoveryNode::getId) + .toArray(String[]::new); + assertThat(discoveryNodes.resolveNodes("_all", "master:false"), arrayContainingInAnyOrder(nonMasterNodes)); + + assertThat(discoveryNodes.resolveNodes("master:false", "_all"), arrayContainingInAnyOrder(allNodes)); + } + + public void testCoordinatorOnlyNodes() { + final DiscoveryNodes discoveryNodes = buildDiscoveryNodes(); + + final String[] coordinatorOnlyNodes = + StreamSupport.stream(discoveryNodes.getNodes().values().spliterator(), false) + .map(n -> n.value) + .filter(n -> n.isDataNode() == false && n.isIngestNode() == false && n.isMasterNode() == false) + .map(DiscoveryNode::getId) + .toArray(String[]::new); + + assertThat( + discoveryNodes.resolveNodes("_all", "data:false", "ingest:false", "master:false"), + arrayContainingInAnyOrder(coordinatorOnlyNodes)); + } + public void testResolveNodesIds() { DiscoveryNodes discoveryNodes = buildDiscoveryNodes();