Skip to content

Commit

Permalink
Remove special handling for _all in nodes info
Browse files Browse the repository at this point in the history
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 elastic#28971
  • Loading branch information
jasontedor authored Mar 9, 2018
1 parent 97b513e commit 950c436
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
*
Expand Down Expand Up @@ -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<String> resolvedNodesIds = new ObjectHashSet<>(nodes.length);
for (String nodeId : nodes) {
Expand All @@ -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());
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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();

Expand Down

0 comments on commit 950c436

Please sign in to comment.