-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Persistent Node Ids #17987
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
Persistent Node Ids #17987
Changes from all commits
bd5693f
31bc94e
512c4f4
199e7e5
af1ecee
536a377
ea8931e
4ae867d
570ff47
5cc4236
07e5558
2ac579d
56d68d5
881b24f
7e96a08
7968736
bb6f062
76f1a1d
fce829d
afb14c3
697419c
4090290
2a7daf1
39fc924
e57232f
49b871a
7f95af0
7dbe1bf
4f382b9
13c993c
258fbad
10bbddb
a1ff308
0174ea5
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 |
|---|---|---|
|
|
@@ -20,7 +20,6 @@ | |
| package org.elasticsearch.cluster.node; | ||
|
|
||
| import org.elasticsearch.Version; | ||
| import org.elasticsearch.common.Strings; | ||
| import org.elasticsearch.common.io.stream.StreamInput; | ||
| import org.elasticsearch.common.io.stream.StreamOutput; | ||
| import org.elasticsearch.common.io.stream.Writeable; | ||
|
|
@@ -64,7 +63,15 @@ public static boolean isLocalNode(Settings settings) { | |
| } | ||
|
|
||
| public static boolean nodeRequiresLocalStorage(Settings settings) { | ||
| return Node.NODE_DATA_SETTING.get(settings) || Node.NODE_MASTER_SETTING.get(settings); | ||
| boolean localStorageEnable = Node.NODE_LOCAL_STORAGE_SETTING.get(settings); | ||
| if (localStorageEnable == false && | ||
| (Node.NODE_DATA_SETTING.get(settings) || | ||
| Node.NODE_MASTER_SETTING.get(settings)) | ||
| ) { | ||
| // TODO: make this a proper setting validation logic, requiring multi-settings validation | ||
| throw new IllegalArgumentException("storage can not be disabled for master and data nodes"); | ||
| } | ||
| return localStorageEnable; | ||
| } | ||
|
|
||
| public static boolean isMasterNode(Settings settings) { | ||
|
|
@@ -81,6 +88,7 @@ public static boolean isIngestNode(Settings settings) { | |
|
|
||
| private final String nodeName; | ||
| private final String nodeId; | ||
| private final String ephemeralId; | ||
| private final String hostName; | ||
| private final String hostAddress; | ||
| private final TransportAddress address; | ||
|
|
@@ -97,14 +105,15 @@ public static boolean isIngestNode(Settings settings) { | |
| * and updated. | ||
| * </p> | ||
| * | ||
| * @param nodeId the nodes unique id. | ||
| * @param address the nodes transport address | ||
| * @param attributes node attributes | ||
| * @param roles node roles | ||
| * @param version the version of the node. | ||
| * @param id the nodes unique (ephemeral and persistent) node id | ||
| * @param address the nodes transport address | ||
| * @param attributes node attributes | ||
| * @param roles node roles | ||
| * @param version the version of the node | ||
| */ | ||
| public DiscoveryNode(String nodeId, TransportAddress address, Map<String, String> attributes, Set<Role> roles, Version version) { | ||
| this("", nodeId, address.getHost(), address.getAddress(), address, attributes, roles, version); | ||
| public DiscoveryNode(String id, TransportAddress address, Map<String, String> attributes, Set<Role> roles, | ||
| Version version) { | ||
| this("", id, id, address, attributes, roles, version); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -116,16 +125,17 @@ public DiscoveryNode(String nodeId, TransportAddress address, Map<String, String | |
| * and updated. | ||
| * </p> | ||
| * | ||
| * @param nodeName the nodes name | ||
| * @param nodeId the nodes unique id. | ||
| * @param address the nodes transport address | ||
| * @param attributes node attributes | ||
| * @param roles node roles | ||
| * @param version the version of the node. | ||
| * @param nodeName the nodes name | ||
| * @param nodeId the nodes unique persistent id | ||
| * @param ephemeralId the nodes unique ephemeral id | ||
| * @param address the nodes transport address | ||
| * @param attributes node attributes | ||
| * @param roles node roles | ||
| * @param version the version of the node | ||
| */ | ||
| public DiscoveryNode(String nodeName, String nodeId, TransportAddress address, Map<String, String> attributes, | ||
| Set<Role> roles, Version version) { | ||
| this(nodeName, nodeId, address.getHost(), address.getAddress(), address, attributes, roles, version); | ||
| public DiscoveryNode(String nodeName, String nodeId, String ephemeralId, TransportAddress address, | ||
| Map<String, String> attributes, Set<Role> roles, Version version) { | ||
| this(nodeName, nodeId, ephemeralId, address.getHost(), address.getAddress(), address, attributes, roles, version); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -137,23 +147,24 @@ public DiscoveryNode(String nodeName, String nodeId, TransportAddress address, M | |
| * and updated. | ||
| * </p> | ||
| * | ||
| * @param nodeName the nodes name | ||
| * @param nodeId the nodes unique id. | ||
| * @param hostName the nodes hostname | ||
| * @param hostAddress the nodes host address | ||
| * @param address the nodes transport address | ||
| * @param attributes node attributes | ||
| * @param roles node roles | ||
| * @param version the version of the node. | ||
| * @param nodeName the nodes name | ||
| * @param nodeId the nodes unique persistent id | ||
| * @param ephemeralId the nodes unique ephemeral id | ||
| * @param hostAddress the nodes host address | ||
| * @param address the nodes transport address | ||
| * @param attributes node attributes | ||
| * @param roles node roles | ||
| * @param version the version of the node | ||
| */ | ||
| public DiscoveryNode(String nodeName, String nodeId, String hostName, String hostAddress, TransportAddress address, | ||
| Map<String, String> attributes, Set<Role> roles, Version version) { | ||
| public DiscoveryNode(String nodeName, String nodeId, String ephemeralId, String hostName, String hostAddress, | ||
| TransportAddress address, Map<String, String> attributes, Set<Role> roles, Version version) { | ||
| if (nodeName != null) { | ||
| this.nodeName = nodeName.intern(); | ||
| } else { | ||
| this.nodeName = ""; | ||
| } | ||
| this.nodeId = nodeId.intern(); | ||
| this.ephemeralId = ephemeralId.intern(); | ||
| this.hostName = hostName.intern(); | ||
| this.hostAddress = hostAddress.intern(); | ||
| this.address = address; | ||
|
|
@@ -184,6 +195,7 @@ public DiscoveryNode(String nodeName, String nodeId, String hostName, String hos | |
| public DiscoveryNode(StreamInput in) throws IOException { | ||
| this.nodeName = in.readString().intern(); | ||
| this.nodeId = in.readString().intern(); | ||
| this.ephemeralId = in.readString().intern(); | ||
| this.hostName = in.readString().intern(); | ||
| this.hostAddress = in.readString().intern(); | ||
| this.address = TransportAddressSerializers.addressFromStream(in); | ||
|
|
@@ -208,6 +220,7 @@ public DiscoveryNode(StreamInput in) throws IOException { | |
| public void writeTo(StreamOutput out) throws IOException { | ||
| out.writeString(nodeName); | ||
| out.writeString(nodeId); | ||
| out.writeString(ephemeralId); | ||
| out.writeString(hostName); | ||
| out.writeString(hostAddress); | ||
| addressToStream(out, address); | ||
|
|
@@ -237,6 +250,13 @@ public String getId() { | |
| return nodeId; | ||
| } | ||
|
|
||
| /** | ||
| * The unique ephemeral id of the node. | ||
|
Contributor
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. can we add some docs as to what we mean with ephemeral - most notably when it changes, and it's implication to identity |
||
| */ | ||
| public String getEphemeralId() { | ||
| return ephemeralId; | ||
| } | ||
|
|
||
| /** | ||
| * The name of the node. | ||
| */ | ||
|
|
@@ -293,17 +313,49 @@ public String getHostAddress() { | |
| } | ||
|
|
||
| @Override | ||
| public boolean equals(Object obj) { | ||
| if (!(obj instanceof DiscoveryNode)) { | ||
| public boolean equals(Object o) { | ||
| if (this == o) { | ||
| return true; | ||
| } | ||
| if (o == null || getClass() != o.getClass()) { | ||
| return false; | ||
| } | ||
|
|
||
| DiscoveryNode that = (DiscoveryNode) o; | ||
|
|
||
| if (!nodeId.equals(that.nodeId)) { | ||
| return false; | ||
| } | ||
| if (!ephemeralId.equals(that.ephemeralId)) { | ||
| return false; | ||
| } | ||
| if (!nodeName.equals(that.nodeName)) { | ||
| return false; | ||
| } | ||
| if (!hostName.equals(that.hostName)) { | ||
| return false; | ||
| } | ||
| if (!hostAddress.equals(that.hostAddress)) { | ||
| return false; | ||
| } | ||
| if (!address.equals(that.address)) { | ||
| return false; | ||
| } | ||
| if (!attributes.equals(that.attributes)) { | ||
| return false; | ||
| } | ||
| if (!version.equals(that.version)) { | ||
| return false; | ||
| } | ||
| return roles.equals(that.roles); | ||
|
|
||
| DiscoveryNode other = (DiscoveryNode) obj; | ||
| return this.nodeId.equals(other.nodeId); | ||
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { | ||
| // we only need to hash the id because it's highly unlikely that two nodes | ||
| // in our system will have the same id but be different | ||
| // This is done so that this class can be used efficiently as a key in a map | ||
| return nodeId.hashCode(); | ||
| } | ||
|
|
||
|
|
@@ -313,15 +365,10 @@ public String toString() { | |
| if (nodeName.length() > 0) { | ||
| sb.append('{').append(nodeName).append('}'); | ||
| } | ||
| if (nodeId != null) { | ||
| sb.append('{').append(nodeId).append('}'); | ||
| } | ||
| if (Strings.hasLength(hostName)) { | ||
| sb.append('{').append(hostName).append('}'); | ||
| } | ||
| if (address != null) { | ||
| sb.append('{').append(address).append('}'); | ||
| } | ||
| sb.append('{').append(nodeId).append('}'); | ||
|
Contributor
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. why did you change this?
Contributor
Author
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. nodeId is never null |
||
| sb.append('{').append(ephemeralId).append('}'); | ||
| sb.append('{').append(hostName).append('}'); | ||
| sb.append('{').append(address).append('}'); | ||
| if (!attributes.isEmpty()) { | ||
| sb.append(attributes); | ||
| } | ||
|
|
@@ -332,6 +379,7 @@ public String toString() { | |
| public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { | ||
| builder.startObject(getId()); | ||
| builder.field("name", getName()); | ||
| builder.field("ephemeral_id", getEphemeralId()); | ||
| builder.field("transport_address", getAddress().toString()); | ||
|
|
||
| builder.startObject("attributes"); | ||
|
|
||
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.
hmm. I think this is tricky having no id string. How about using the supplied id as the nodeId and generate a random ephemeral one?
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 don't understand what you mean here. This constructor is mainly used by tests where we don't care much about the actual values and construction of temporary nodes for pinging. I don't think we should just randomly generate a ephemeral id here. Better would be to get rid of this constructor altogether...