Skip to content

Commit a57778c

Browse files
jasontedorjasontedor
committed
Fix roles parsing in client nodes sniffer (#52888)
We mades roles pluggable, but never updated the client to account for this. This means that when speaking to a modern cluster, application logs are spammed with warning messages around unrecognized roles. This commit addresses this by accounting for the fact that roles can extend beyond master/data/ingest now.
1 parent 4bd6333 commit a57778c

File tree

10 files changed

+346
-58
lines changed

10 files changed

+346
-58
lines changed

‎client/rest/src/main/java/org/elasticsearch/client/Node.java‎

Lines changed: 15 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,13 @@
1919

2020
package org.elasticsearch.client;
2121

22+
import org.apache.http.HttpHost;
23+
2224
import java.util.List;
2325
import java.util.Map;
2426
import java.util.Objects;
2527
import java.util.Set;
26-
27-
import org.apache.http.HttpHost;
28+
import java.util.TreeSet;
2829

2930
/**
3031
* Metadata about an {@link HttpHost} running Elasticsearch.
@@ -175,42 +176,35 @@ public int hashCode() {
175176
* Role information about an Elasticsearch process.
176177
*/
177178
public static final class Roles {
178-
private final boolean masterEligible;
179-
private final boolean data;
180-
private final boolean ingest;
181-
182-
public Roles(boolean masterEligible, boolean data, boolean ingest) {
183-
this.masterEligible = masterEligible;
184-
this.data = data;
185-
this.ingest = ingest;
179+
180+
private final Set<String> roles;
181+
182+
public Roles(final Set<String> roles) {
183+
this.roles = new TreeSet<>(roles);
186184
}
187185

188186
/**
189187
* Teturns whether or not the node <strong>could</strong> be elected master.
190188
*/
191189
public boolean isMasterEligible() {
192-
return masterEligible;
190+
return roles.contains("master");
193191
}
194192
/**
195193
* Teturns whether or not the node stores data.
196194
*/
197195
public boolean isData() {
198-
return data;
196+
return roles.contains("data");
199197
}
200198
/**
201199
* Teturns whether or not the node runs ingest pipelines.
202200
*/
203201
public boolean isIngest() {
204-
return ingest;
202+
return roles.contains("ingest");
205203
}
206204

207205
@Override
208206
public String toString() {
209-
StringBuilder result = new StringBuilder(3);
210-
if (masterEligible) result.append('m');
211-
if (data) result.append('d');
212-
if (ingest) result.append('i');
213-
return result.toString();
207+
return String.join(",", roles);
214208
}
215209

216210
@Override
@@ -219,14 +213,13 @@ public boolean equals(Object obj) {
219213
return false;
220214
}
221215
Roles other = (Roles) obj;
222-
return masterEligible == other.masterEligible
223-
&& data == other.data
224-
&& ingest == other.ingest;
216+
return roles.equals(other.roles);
225217
}
226218

227219
@Override
228220
public int hashCode() {
229-
return Objects.hash(masterEligible, data, ingest);
221+
return roles.hashCode();
230222
}
223+
231224
}
232225
}

‎client/rest/src/test/java/org/elasticsearch/client/HasAttributeNodeSelectorTests.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import java.util.Collections;
2828
import java.util.List;
2929
import java.util.Map;
30+
import java.util.Set;
31+
import java.util.TreeSet;
3032

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

5355
private static Node dummyNode(Map<String, List<String>> attributes) {
56+
final Set<String> roles = new TreeSet<>();
57+
if (randomBoolean()) {
58+
roles.add("master");
59+
}
60+
if (randomBoolean()) {
61+
roles.add("data");
62+
}
63+
if (randomBoolean()) {
64+
roles.add("ingest");
65+
}
5466
return new Node(new HttpHost("dummy"), Collections.<HttpHost>emptySet(),
5567
randomAsciiAlphanumOfLength(5), randomAsciiAlphanumOfLength(5),
56-
new Roles(randomBoolean(), randomBoolean(), randomBoolean()),
68+
new Roles(roles),
5769
attributes);
5870
}
5971
}

‎client/rest/src/test/java/org/elasticsearch/client/NodeSelectorTests.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@
2525
import java.util.ArrayList;
2626
import java.util.Collections;
2727
import java.util.List;
28+
import java.util.Set;
29+
import java.util.TreeSet;
2830

2931
import static org.junit.Assert.assertEquals;
3032

@@ -64,9 +66,19 @@ public void testNotMasterOnly() {
6466
}
6567

6668
private static Node dummyNode(boolean master, boolean data, boolean ingest) {
69+
final Set<String> roles = new TreeSet<>();
70+
if (master) {
71+
roles.add("master");
72+
}
73+
if (data) {
74+
roles.add("data");
75+
}
76+
if (ingest) {
77+
roles.add("ingest");
78+
}
6779
return new Node(new HttpHost("dummy"), Collections.<HttpHost>emptySet(),
6880
randomAsciiAlphanumOfLength(5), randomAsciiAlphanumOfLength(5),
69-
new Roles(master, data, ingest),
81+
new Roles(roles),
7082
Collections.<String, List<String>>emptyMap());
7183
}
7284
}

‎client/rest/src/test/java/org/elasticsearch/client/NodeTests.java‎

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@
2323
import org.elasticsearch.client.Node.Roles;
2424

2525
import java.util.Arrays;
26+
import java.util.Collections;
2627
import java.util.HashMap;
2728
import java.util.HashSet;
2829
import java.util.List;
2930
import java.util.Map;
31+
import java.util.TreeSet;
3032

3133
import static java.util.Collections.singleton;
3234
import static java.util.Collections.singletonList;
@@ -43,19 +45,19 @@ public void testToString() {
4345
assertEquals("[host=http://1]", new Node(new HttpHost("1")).toString());
4446
assertEquals("[host=http://1, attributes={foo=[bar], baz=[bort, zoom]}]",
4547
new Node(new HttpHost("1"), null, null, null, null, attributes).toString());
46-
assertEquals("[host=http://1, roles=mdi]", new Node(new HttpHost("1"),
47-
null, null, null, new Roles(true, true, true), null).toString());
48+
assertEquals("[host=http://1, roles=data,ingest,master]", new Node(new HttpHost("1"),
49+
null, null, null, new Roles(new TreeSet<>(Arrays.asList("master", "data", "ingest"))), null).toString());
4850
assertEquals("[host=http://1, version=ver]", new Node(new HttpHost("1"),
4951
null, null, "ver", null, null).toString());
5052
assertEquals("[host=http://1, name=nam]", new Node(new HttpHost("1"),
5153
null, "nam", null, null, null).toString());
5254
assertEquals("[host=http://1, bound=[http://1, http://2]]", new Node(new HttpHost("1"),
5355
new HashSet<>(Arrays.asList(new HttpHost("1"), new HttpHost("2"))), null, null, null, null).toString());
5456
assertEquals(
55-
"[host=http://1, bound=[http://1, http://2], name=nam, version=ver, roles=m, attributes={foo=[bar], baz=[bort, zoom]}]",
57+
"[host=http://1, bound=[http://1, http://2], "
58+
+ "name=nam, version=ver, roles=master, attributes={foo=[bar], baz=[bort, zoom]}]",
5659
new Node(new HttpHost("1"), new HashSet<>(Arrays.asList(new HttpHost("1"), new HttpHost("2"))),
57-
"nam", "ver", new Roles(true, false, false), attributes).toString());
58-
60+
"nam", "ver", new Roles(Collections.singleton("master")), attributes).toString());
5961
}
6062

6163
public void testEqualsAndHashCode() {
@@ -64,7 +66,7 @@ public void testEqualsAndHashCode() {
6466
randomBoolean() ? null : singleton(host),
6567
randomBoolean() ? null : randomAsciiAlphanumOfLength(5),
6668
randomBoolean() ? null : randomAsciiAlphanumOfLength(5),
67-
randomBoolean() ? null : new Roles(true, true, true),
69+
randomBoolean() ? null : new Roles(new TreeSet<>(Arrays.asList("master", "data", "ingest"))),
6870
randomBoolean() ? null : singletonMap("foo", singletonList("bar")));
6971
assertFalse(node.equals(null));
7072
assertTrue(node.equals(node));
@@ -82,7 +84,7 @@ public void testEqualsAndHashCode() {
8284
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(),
8385
node.getVersion() + "changed", node.getRoles(), node.getAttributes())));
8486
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(),
85-
node.getVersion(), new Roles(false, false, false), node.getAttributes())));
87+
node.getVersion(), new Roles(Collections.emptySet()), node.getAttributes())));
8688
assertFalse(node.equals(new Node(host, node.getBoundHosts(), node.getName(),
8789
node.getVersion(), node.getRoles(), singletonMap("bort", singletonList("bing")))));
8890
}

‎client/rest/src/test/java/org/elasticsearch/client/PreferHasAttributeNodeSelectorTests.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
import java.util.Collections;
2828
import java.util.List;
2929
import java.util.Map;
30+
import java.util.Set;
31+
import java.util.TreeSet;
3032

3133
import static java.util.Collections.singletonList;
3234
import static java.util.Collections.singletonMap;
@@ -61,9 +63,19 @@ public void testNotFoundPreferHasAttribute() {
6163
}
6264

6365
private static Node dummyNode(Map<String, List<String>> attributes) {
66+
final Set<String> roles = new TreeSet<>();
67+
if (randomBoolean()) {
68+
roles.add("master");
69+
}
70+
if (randomBoolean()) {
71+
roles.add("data");
72+
}
73+
if (randomBoolean()) {
74+
roles.add("ingest");
75+
}
6476
return new Node(new HttpHost("dummy"), Collections.<HttpHost>emptySet(),
6577
randomAsciiAlphanumOfLength(5), randomAsciiAlphanumOfLength(5),
66-
new Roles(randomBoolean(), randomBoolean(), randomBoolean()),
78+
new Roles(roles),
6779
attributes);
6880
}
6981
}

‎client/rest/src/test/java/org/elasticsearch/client/RestClientMultipleHostsTests.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@
2727

2828
import java.io.IOException;
2929
import java.util.ArrayList;
30+
import java.util.Arrays;
3031
import java.util.Collections;
3132
import java.util.HashSet;
3233
import java.util.Iterator;
3334
import java.util.List;
3435
import java.util.Set;
36+
import java.util.TreeSet;
3537
import java.util.concurrent.ExecutorService;
3638
import java.util.concurrent.Executors;
3739

@@ -271,7 +273,9 @@ public void testSetNodes() throws Exception {
271273
RestClient restClient = createRestClient(NodeSelector.SKIP_DEDICATED_MASTERS);
272274
List<Node> newNodes = new ArrayList<>(nodes.size());
273275
for (int i = 0; i < nodes.size(); i++) {
274-
Node.Roles roles = i == 0 ? new Node.Roles(false, true, true) : new Node.Roles(true, false, false);
276+
Node.Roles roles = i == 0 ?
277+
new Node.Roles(new TreeSet<>(Arrays.asList("data", "ingest"))) :
278+
new Node.Roles(new TreeSet<>(Arrays.asList("master")));
275279
newNodes.add(new Node(nodes.get(i).getHost(), null, null, null, roles, null));
276280
}
277281
restClient.setNodes(newNodes);

‎client/sniffer/src/main/java/org/elasticsearch/client/sniff/ElasticsearchNodesSniffer.java‎

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,10 +27,10 @@
2727
import org.apache.http.HttpEntity;
2828
import org.apache.http.HttpHost;
2929
import org.elasticsearch.client.Node;
30+
import org.elasticsearch.client.Node.Roles;
3031
import org.elasticsearch.client.Request;
3132
import org.elasticsearch.client.Response;
3233
import org.elasticsearch.client.RestClient;
33-
import org.elasticsearch.client.Node.Roles;
3434

3535
import java.io.IOException;
3636
import java.io.InputStream;
@@ -42,6 +42,7 @@
4242
import java.util.Map;
4343
import java.util.Objects;
4444
import java.util.Set;
45+
import java.util.TreeSet;
4546
import java.util.concurrent.TimeUnit;
4647

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

154155
boolean sawRoles = false;
155-
boolean master = false;
156-
boolean data = false;
157-
boolean ingest = false;
156+
final Set<String> roles = new TreeSet<>();
158157

159158
String fieldName = null;
160159
while (parser.nextToken() != JsonToken.END_OBJECT) {
@@ -207,19 +206,7 @@ private static Node readNode(String nodeId, JsonParser parser, Scheme scheme) th
207206
if ("roles".equals(fieldName)) {
208207
sawRoles = true;
209208
while (parser.nextToken() != JsonToken.END_ARRAY) {
210-
switch (parser.getText()) {
211-
case "master":
212-
master = true;
213-
break;
214-
case "data":
215-
data = true;
216-
break;
217-
case "ingest":
218-
ingest = true;
219-
break;
220-
default:
221-
logger.warn("unknown role [" + parser.getText() + "] on node [" + nodeId + "]");
222-
}
209+
roles.add(parser.getText());
223210
}
224211
} else {
225212
parser.skipChildren();
@@ -268,15 +255,19 @@ private static Node readNode(String nodeId, JsonParser parser, Scheme scheme) th
268255
boolean clientAttribute = v2RoleAttributeValue(realAttributes, "client", false);
269256
Boolean masterAttribute = v2RoleAttributeValue(realAttributes, "master", null);
270257
Boolean dataAttribute = v2RoleAttributeValue(realAttributes, "data", null);
271-
master = masterAttribute == null ? false == clientAttribute : masterAttribute;
272-
data = dataAttribute == null ? false == clientAttribute : dataAttribute;
258+
if ((masterAttribute == null && false == clientAttribute) || masterAttribute) {
259+
roles.add("master");
260+
}
261+
if ((dataAttribute == null && false == clientAttribute) || dataAttribute) {
262+
roles.add("data");
263+
}
273264
} else {
274265
assert sawRoles : "didn't see roles for [" + nodeId + "]";
275266
}
276267
assert boundHosts.contains(publishedHost) :
277268
"[" + nodeId + "] doesn't make sense! publishedHost should be in boundHosts";
278269
logger.trace("adding node [" + nodeId + "]");
279-
return new Node(publishedHost, boundHosts, name, version, new Roles(master, data, ingest),
270+
return new Node(publishedHost, boundHosts, name, version, new Roles(roles),
280271
unmodifiableMap(realAttributes));
281272
}
282273

0 commit comments

Comments
 (0)