Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix roles parsing in client nodes sniffer #52888

Merged
merged 7 commits into from
Feb 27, 2020
Merged

Fix roles parsing in client nodes sniffer #52888

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0b9f258
Fix roles parsing in client nodes sniffer
jasontedor Feb 27, 2020
e3a76b3
Merge branch 'master' into fix-roles-parsing-in-llrc
elasticmachine Feb 27, 2020
b97eca1
Fix compilation
jasontedor Feb 27, 2020
124ac22
Fix tests
jasontedor Feb 27, 2020
44ed410
Fix compilation
jasontedor Feb 27, 2020
01ce6c6
Fix test
jasontedor Feb 27, 2020
1700773
Fix line length
jasontedor Feb 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
37 changes: 15 additions & 22 deletions client/rest/src/main/java/org/elasticsearch/client/Node.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@

package org.elasticsearch.client;

import org.apache.http.HttpHost;

import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.Set;

import org.apache.http.HttpHost;
import java.util.TreeSet;

/**
* Metadata about an {@link HttpHost} running Elasticsearch.
Expand Down Expand Up @@ -175,42 +176,35 @@ public int hashCode() {
* Role information about an Elasticsearch process.
*/
public static final class Roles {
private final boolean masterEligible;
private final boolean data;
private final boolean ingest;

public Roles(boolean masterEligible, boolean data, boolean ingest) {
this.masterEligible = masterEligible;
this.data = data;
this.ingest = ingest;

private final Set<String> roles;

public Roles(final Set<String> roles) {
this.roles = new TreeSet<>(roles);
}

/**
* Teturns whether or not the node <strong>could</strong> be elected master.
*/
public boolean isMasterEligible() {
return masterEligible;
return roles.contains("master");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, should we make these constants since they're hardcoded as strings all over the place? (in separate work, not this PR)

}
/**
* Teturns whether or not the node stores data.
*/
public boolean isData() {
return data;
return roles.contains("data");
}
/**
* Teturns whether or not the node runs ingest pipelines.
*/
public boolean isIngest() {
return ingest;
return roles.contains("ingest");
}

@Override
public String toString() {
StringBuilder result = new StringBuilder(3);
if (masterEligible) result.append('m');
if (data) result.append('d');
if (ingest) result.append('i');
return result.toString();
return String.join(",", roles);
}

@Override
Expand All @@ -219,14 +213,13 @@ public boolean equals(Object obj) {
return false;
}
Roles other = (Roles) obj;
return masterEligible == other.masterEligible
&& data == other.data
&& ingest == other.ingest;
return roles.equals(other.roles);
}

@Override
public int hashCode() {
return Objects.hash(masterEligible, data, ingest);
return roles.hashCode();
}

}
}
14 changes: 13 additions & 1 deletion client/rest/src/test/java/org/elasticsearch/client/HasAttributeNodeSelectorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;

import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
Expand All @@ -51,9 +53,19 @@ public void testHasAttribute() {
}

private static Node dummyNode(Map<String, List<String>> attributes) {
final Set<String> roles = new TreeSet<>();
if (randomBoolean()) {
roles.add("master");
}
if (randomBoolean()) {
roles.add("data");
}
if (randomBoolean()) {
roles.add("ingest");
}
return new Node(new HttpHost("dummy"), Collections.<HttpHost>emptySet(),
randomAsciiAlphanumOfLength(5), randomAsciiAlphanumOfLength(5),
new Roles(randomBoolean(), randomBoolean(), randomBoolean()),
new Roles(roles),
attributes);
}
}
14 changes: 13 additions & 1 deletion client/rest/src/test/java/org/elasticsearch/client/NodeSelectorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;

import static org.junit.Assert.assertEquals;

Expand Down Expand Up @@ -64,9 +66,19 @@ public void testNotMasterOnly() {
}

private static Node dummyNode(boolean master, boolean data, boolean ingest) {
final Set<String> roles = new TreeSet<>();
if (master) {
roles.add("master");
}
if (data) {
roles.add("data");
}
if (ingest) {
roles.add("ingest");
}
return new Node(new HttpHost("dummy"), Collections.<HttpHost>emptySet(),
randomAsciiAlphanumOfLength(5), randomAsciiAlphanumOfLength(5),
new Roles(master, data, ingest),
new Roles(roles),
Collections.<String, List<String>>emptyMap());
}
}
16 changes: 9 additions & 7 deletions client/rest/src/test/java/org/elasticsearch/client/NodeTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,12 @@
import org.elasticsearch.client.Node.Roles;

import java.util.Arrays;
import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.TreeSet;

import static java.util.Collections.singleton;
import static java.util.Collections.singletonList;
Expand All @@ -43,19 +45,19 @@ public void testToString() {
assertEquals("[host=http://1]", new Node(new HttpHost("1")).toString());
assertEquals("[host=http://1, attributes={foo=[bar], baz=[bort, zoom]}]",
new Node(new HttpHost("1"), null, null, null, null, attributes).toString());
assertEquals("[host=http://1, roles=mdi]", new Node(new HttpHost("1"),
null, null, null, new Roles(true, true, true), null).toString());
assertEquals("[host=http://1, roles=data,ingest,master]", new Node(new HttpHost("1"),
null, null, null, new Roles(new TreeSet<>(Arrays.asList("master", "data", "ingest"))), null).toString());
assertEquals("[host=http://1, version=ver]", new Node(new HttpHost("1"),
null, null, "ver", null, null).toString());
assertEquals("[host=http://1, name=nam]", new Node(new HttpHost("1"),
null, "nam", null, null, null).toString());
assertEquals("[host=http://1, bound=[http://1, http://2]]", new Node(new HttpHost("1"),
new HashSet<>(Arrays.asList(new HttpHost("1"), new HttpHost("2"))), null, null, null, null).toString());
assertEquals(
"[host=http://1, bound=[http://1, http://2], name=nam, version=ver, roles=m, attributes={foo=[bar], baz=[bort, zoom]}]",
"[host=http://1, bound=[http://1, http://2], "
+ "name=nam, version=ver, roles=master, attributes={foo=[bar], baz=[bort, zoom]}]",
new Node(new HttpHost("1"), new HashSet<>(Arrays.asList(new HttpHost("1"), new HttpHost("2"))),
"nam", "ver", new Roles(true, false, false), attributes).toString());

"nam", "ver", new Roles(Collections.singleton("master")), attributes).toString());
}

public void testEqualsAndHashCode() {
Expand All @@ -64,7 +66,7 @@ public void testEqualsAndHashCode() {
randomBoolean() ? null : singleton(host),
randomBoolean() ? null : randomAsciiAlphanumOfLength(5),
randomBoolean() ? null : randomAsciiAlphanumOfLength(5),
randomBoolean() ? null : new Roles(true, true, true),
randomBoolean() ? null : new Roles(new TreeSet<>(Arrays.asList("master", "data", "ingest"))),
randomBoolean() ? null : singletonMap("foo", singletonList("bar")));
assertFalse(node.equals(null));
assertTrue(node.equals(node));
Expand All @@ -82,7 +84,7 @@ public void testEqualsAndHashCode() {
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(),
node.getVersion() + "changed", node.getRoles(), node.getAttributes())));
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(),
node.getVersion(), new Roles(false, false, false), node.getAttributes())));
node.getVersion(), new Roles(Collections.emptySet()), node.getAttributes())));
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(),
node.getVersion(), node.getRoles(), singletonMap("bort", singletonList("bing")))));
}
Expand Down
14 changes: 13 additions & 1 deletion client/rest/src/test/java/org/elasticsearch/client/PreferHasAttributeNodeSelectorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.TreeSet;

import static java.util.Collections.singletonList;
import static java.util.Collections.singletonMap;
Expand Down Expand Up @@ -61,9 +63,19 @@ public void testNotFoundPreferHasAttribute() {
}

private static Node dummyNode(Map<String, List<String>> attributes) {
final Set<String> roles = new TreeSet<>();
if (randomBoolean()) {
roles.add("master");
}
if (randomBoolean()) {
roles.add("data");
}
if (randomBoolean()) {
roles.add("ingest");
}
return new Node(new HttpHost("dummy"), Collections.<HttpHost>emptySet(),
randomAsciiAlphanumOfLength(5), randomAsciiAlphanumOfLength(5),
new Roles(randomBoolean(), randomBoolean(), randomBoolean()),
new Roles(roles),
attributes);
}
}
6 changes: 5 additions & 1 deletion client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,13 @@

import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

Expand Down Expand Up @@ -271,7 +273,9 @@ public void testSetNodes() throws Exception {
RestClient restClient = createRestClient(NodeSelector.SKIP_DEDICATED_MASTERS);
List<Node> newNodes = new ArrayList<>(nodes.size());
for (int i = 0; i < nodes.size(); i++) {
Node.Roles roles = i == 0 ? new Node.Roles(false, true, true) : new Node.Roles(true, false, false);
Node.Roles roles = i == 0 ?
new Node.Roles(new TreeSet<>(Arrays.asList("data", "ingest"))) :
new Node.Roles(new TreeSet<>(Arrays.asList("master")));
newNodes.add(new Node(nodes.get(i).getHost(), null, null, null, roles, null));
}
restClient.setNodes(newNodes);
Expand Down
31 changes: 11 additions & 20 deletions client/sniffer/src/main/java/org/elasticsearch/client/sniff/ElasticsearchNodesSniffer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@
import org.apache.http.HttpEntity;
import org.apache.http.HttpHost;
import org.elasticsearch.client.Node;
import org.elasticsearch.client.Node.Roles;
import org.elasticsearch.client.Request;
import org.elasticsearch.client.Response;
import org.elasticsearch.client.RestClient;
import org.elasticsearch.client.Node.Roles;

import java.io.IOException;
import java.io.InputStream;
Expand All @@ -42,6 +42,7 @@
import java.util.Map;
import java.util.Objects;
import java.util.Set;
import java.util.TreeSet;
import java.util.concurrent.TimeUnit;

import static java.util.Collections.singletonList;
Expand Down Expand Up @@ -152,9 +153,7 @@ private static Node readNode(String nodeId, JsonParser parser, Scheme scheme) th
final Map<String, String> protoAttributes = new HashMap<String, String>();

boolean sawRoles = false;
boolean master = false;
boolean data = false;
boolean ingest = false;
final Set<String> roles = new TreeSet<>();

String fieldName = null;
while (parser.nextToken() != JsonToken.END_OBJECT) {
Expand Down Expand Up @@ -207,19 +206,7 @@ private static Node readNode(String nodeId, JsonParser parser, Scheme scheme) th
if ("roles".equals(fieldName)) {
sawRoles = true;
while (parser.nextToken() != JsonToken.END_ARRAY) {
switch (parser.getText()) {
case "master":
master = true;
break;
case "data":
data = true;
break;
case "ingest":
ingest = true;
break;
default:
logger.warn("unknown role [" + parser.getText() + "] on node [" + nodeId + "]");
}
roles.add(parser.getText());
}
} else {
parser.skipChildren();
Expand Down Expand Up @@ -268,15 +255,19 @@ private static Node readNode(String nodeId, JsonParser parser, Scheme scheme) th
boolean clientAttribute = v2RoleAttributeValue(realAttributes, "client", false);
Boolean masterAttribute = v2RoleAttributeValue(realAttributes, "master", null);
Boolean dataAttribute = v2RoleAttributeValue(realAttributes, "data", null);
master = masterAttribute == null ? false == clientAttribute : masterAttribute;
data = dataAttribute == null ? false == clientAttribute : dataAttribute;
if ((masterAttribute == null && false == clientAttribute) || masterAttribute) {
roles.add("master");
}
if ((dataAttribute == null && false == clientAttribute) || dataAttribute) {
roles.add("data");
}
} else {
assert sawRoles : "didn't see roles for [" + nodeId + "]";
}
assert boundHosts.contains(publishedHost) :
"[" + nodeId + "] doesn't make sense! publishedHost should be in boundHosts";
logger.trace("adding node [" + nodeId + "]");
return new Node(publishedHost, boundHosts, name, version, new Roles(master, data, ingest),
return new Node(publishedHost, boundHosts, name, version, new Roles(roles),
unmodifiableMap(realAttributes));
}

Expand Down