-
Notifications
You must be signed in to change notification settings - Fork 25.2k
LLClient: Support host selection #30523
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.
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
Changes from 2 commits
99c8a33
81dd654
73d710f
7eb276b
9cda8c9
f05a7ca
d4c56a3
0e6c479
e13171d
7339b26
2ae7c27
d551dcb
5e0b20b
1245ed3
9504841
2bc05b3
8a781de
0a56119
159ad56
baf572c
1f00b95
4652073
a6a5b46
7d6ee4f
25280ef
5eca290
0ab1b30
a9e682f
e29723a
07004c2
f030c9b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,235 @@ | ||
/* | ||
* Licensed to Elasticsearch under one or more contributor | ||
* license agreements. See the NOTICE file distributed with | ||
* this work for additional information regarding copyright | ||
* ownership. Elasticsearch licenses this file to you under | ||
* the Apache License, Version 2.0 (the "License"); you may | ||
* not use this file except in compliance with the License. | ||
* You may obtain a copy of the License at | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
|
||
package org.elasticsearch.client; | ||
|
||
import static java.util.Collections.unmodifiableSet; | ||
|
||
import java.util.HashSet; | ||
import java.util.Objects; | ||
import java.util.Set; | ||
|
||
import org.apache.http.HttpHost; | ||
|
||
/** | ||
* Metadata about an {@link HttpHost} running Elasticsearch. | ||
*/ | ||
public class Node { | ||
/** | ||
* Address that this host claims is its primary contact point. | ||
*/ | ||
private final HttpHost host; | ||
/** | ||
* Addresses on which the host is listening. These are useful to have | ||
* around because they allow you to find a host based on any address it | ||
* is listening on. | ||
*/ | ||
private final Set<HttpHost> boundHosts; | ||
/** | ||
* Name of the node as configured by the {@code node.name} attribute. | ||
*/ | ||
private final String name; | ||
/** | ||
* Version of Elasticsearch that the node is running or {@code null} | ||
* if we don't know the version. | ||
*/ | ||
private final String version; | ||
/** | ||
* Roles that the Elasticsearch process on the host has or {@code null} | ||
* if we don't know what roles the node has. | ||
*/ | ||
private final Roles roles; | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to add node attributes to this in a followup. Folks can tag the elasticsearch node with the rack/row/availability zone that it is in and then use a |
||
/** | ||
* Create a {@linkplain Node} with metadata. All parameters except | ||
* {@code host} are nullable and implementations of {@link NodeSelector} | ||
* need to decide what to do in their absence. | ||
*/ | ||
public Node(HttpHost host, Set<HttpHost> boundHosts, String name, String version, Roles roles) { | ||
if (host == null) { | ||
throw new IllegalArgumentException("host cannot be null"); | ||
} | ||
this.host = host; | ||
this.boundHosts = boundHosts; | ||
this.name = name; | ||
this.version = version; | ||
this.roles = roles; | ||
} | ||
|
||
/** | ||
* Create a {@linkplain Node} without any metadata. | ||
*/ | ||
public Node(HttpHost host) { | ||
this(host, null, null, null, null); | ||
} | ||
|
||
/** | ||
* Make a copy of this {@link Node} but replacing its | ||
* {@link #getHost() host}. Use this when the sniffing implementation | ||
* returns a {@link #getHost() host} that is not useful to the client. | ||
*/ | ||
public Node withHost(HttpHost host) { | ||
/* | ||
* If the new host isn't in the bound hosts list we add it so the | ||
* result looks sane. | ||
*/ | ||
Set<HttpHost> boundHosts = this.boundHosts; | ||
if (false == boundHosts.contains(host)) { | ||
boundHosts = new HashSet<>(boundHosts); | ||
boundHosts.add(host); | ||
boundHosts = unmodifiableSet(boundHosts); | ||
} | ||
return new Node(host, boundHosts, name, version, roles); | ||
} | ||
|
||
/** | ||
* Contact information for the host. | ||
*/ | ||
public HttpHost getHost() { | ||
return host; | ||
} | ||
|
||
/** | ||
* Addresses on which the host is listening. These are useful to have | ||
* around because they allow you to find a host based on any address it | ||
* is listening on. | ||
*/ | ||
public Set<HttpHost> getBoundHosts() { | ||
return boundHosts; | ||
} | ||
|
||
/** | ||
* @return the name | ||
*/ | ||
public String getName() { | ||
return name; | ||
} | ||
|
||
/** | ||
* Version of Elasticsearch that the node is running or {@code null} | ||
* if we don't know the version. | ||
*/ | ||
public String getVersion() { | ||
return version; | ||
} | ||
|
||
/** | ||
* Roles that the Elasticsearch process on the host has or {@code null} | ||
* if we don't know what roles the node has. | ||
*/ | ||
public Roles getRoles() { | ||
return roles; | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
StringBuilder b = new StringBuilder(); | ||
b.append("[host=").append(host); | ||
if (boundHosts != null) { | ||
b.append(", bound=").append(boundHosts); | ||
} | ||
if (name != null) { | ||
b.append(", name=").append(name); | ||
} | ||
if (version != null) { | ||
b.append(", version=").append(version); | ||
} | ||
if (roles != null) { | ||
b.append(", roles=").append(roles); | ||
} | ||
return b.append(']').toString(); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object obj) { | ||
if (obj == null || obj.getClass() != getClass()) { | ||
return false; | ||
} | ||
Node other = (Node) obj; | ||
return host.equals(other.host) | ||
&& Objects.equals(boundHosts, other.boundHosts) | ||
&& Objects.equals(version, other.version) | ||
&& Objects.equals(name, other.name) | ||
&& Objects.equals(roles, other.roles); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this may sound controversial but in my mind the answer to the "when are two nodes equal?" question is "when they have the same host and port". There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think things that want to compare nodes based on the host and port should do There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shall we then drop these methods in this case, I think it confuses me to have equals working this way if most of the times we want to compare the hosts :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sorry I just suggested that we re-implement this. Do what you prefer, I am fine with both at this point given that depending on the day I suggest to have equals or move it to tests :) |
||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(host, boundHosts, name, version, roles); | ||
} | ||
|
||
/** | ||
* 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; | ||
} | ||
|
||
/** | ||
* The node <strong>could</strong> be elected master. | ||
*/ | ||
public boolean isMasterEligible() { | ||
return masterEligible; | ||
} | ||
/** | ||
* The node stores data. | ||
*/ | ||
public boolean isData() { | ||
return data; | ||
} | ||
/** | ||
* The node runs ingest pipelines. | ||
*/ | ||
public boolean isIngest() { | ||
return 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(); | ||
} | ||
|
||
@Override | ||
public boolean equals(Object obj) { | ||
if (obj == null || obj.getClass() != getClass()) { | ||
return false; | ||
} | ||
Roles other = (Roles) obj; | ||
return masterEligible == other.masterEligible | ||
&& data == other.data | ||
&& ingest == other.ingest; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(masterEligible, data, ingest); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose these changes are gone due to a bad merge. I think this was a good improvement and I don't see a replacement for it and the corresponding tests in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Kind of. In my old PR
DeadHostState
didn't have ashallBeRetried
because it looks at thegetDeadUntilNanos
directly so it can sort all the dead nodes. That was part of the thing that allowed us to get rid of the looping. I pulled on the string from there and got here. I agree about the missing test - I need to rebuild it.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would be nice to use the fact that DeadHostState is now Comparable and the shallBeRetried method was convenient too I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still think the same here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I see what you are getting at. We can keep
shallbeRetried
if we keep theDeadHostState
and sort on it. That'd be fine.