From 429e517476d95e50ecb9d73d58e5013bd2158e7d Mon Sep 17 00:00:00 2001 From: Jay Modi Date: Fri, 2 Dec 2016 12:21:53 -0500 Subject: [PATCH] Do not lose host information when pinging In #21828, serialization of the host string was added to preserve this information when a TransportAddress gets serialized. However, there is still a case where this did not always work. In UnicastZenPings, DiscoveryNode instances are created for the ping hosts with the minimum compatibility version, which is currently less than the version required to preserve the host information. This means that when a node is received from a PingResponse that the host information is no longer set correctly on the InetSocketAddress contained in the DiscoveryNode. This commit adds a workaround for this situation by allowing the host string to be passed into the TransportAddress constructor that takes a StreamInput and using that as the host for the InetAddress that is created during deserialization. --- .../elasticsearch/cluster/node/DiscoveryNode.java | 9 ++++++++- .../common/transport/TransportAddress.java | 14 ++++++++++++-- .../cluster/node/DiscoveryNodeTests.java | 5 ++++- 3 files changed, 24 insertions(+), 4 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java index a8e59006af7..a8290d317e8 100644 --- a/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java +++ b/core/src/main/java/org/elasticsearch/cluster/node/DiscoveryNode.java @@ -216,7 +216,14 @@ public class DiscoveryNode implements Writeable, ToXContent { this.ephemeralId = in.readString().intern(); this.hostName = in.readString().intern(); this.hostAddress = in.readString().intern(); - this.address = new TransportAddress(in); + if (in.getVersion().onOrAfter(Version.V_5_0_3_UNRELEASED)) { + this.address = new TransportAddress(in); + } else { + // we need to do this to preserve the host information during pinging and joining of a master. Since the version of the + // DiscoveryNode is set to Version#minimumCompatibilityVersion(), the host information gets lost as we do not serialize the + // hostString for the address + this.address = new TransportAddress(in, hostName); + } int size = in.readVInt(); this.attributes = new HashMap<>(size); for (int i = 0; i < size; i++) { diff --git a/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java b/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java index 4881398823d..b6c93e389f4 100644 --- a/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java +++ b/core/src/main/java/org/elasticsearch/common/transport/TransportAddress.java @@ -20,6 +20,7 @@ package org.elasticsearch.common.transport; import org.elasticsearch.Version; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.io.stream.Writeable; @@ -69,6 +70,14 @@ public final class TransportAddress implements Writeable { * Read from a stream. */ public TransportAddress(StreamInput in) throws IOException { + this(in, null); + } + + /** + * Read from a stream and use the {@code hostString} when creating the InetAddress if the input comes from a version prior + * {@link Version#V_5_0_3_UNRELEASED} as the hostString was not serialized + */ + public TransportAddress(StreamInput in, @Nullable String hostString) throws IOException { if (in.getVersion().before(Version.V_6_0_0_alpha1_UNRELEASED)) { // bwc layer for 5.x where we had more than one transport address final short i = in.readShort(); if(i != 1) { // we fail hard to ensure nobody tries to use some custom transport address impl even if that is difficult to add @@ -80,10 +89,11 @@ public final class TransportAddress implements Writeable { in.readFully(a); final InetAddress inetAddress; if (in.getVersion().onOrAfter(Version.V_5_0_3_UNRELEASED)) { - String host = in.readString(); + String host = in.readString(); // the host string was serialized so we can ignore the passed in version inetAddress = InetAddress.getByAddress(host, a); } else { - inetAddress = InetAddress.getByAddress(a); + // prior to this version, we did not serialize the host string so we used the passed in version + inetAddress = InetAddress.getByAddress(hostString, a); } int port = in.readInt(); this.address = new InetSocketAddress(inetAddress, port); diff --git a/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeTests.java b/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeTests.java index 5178c5f3fc8..548f9d407cc 100644 --- a/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeTests.java +++ b/core/src/test/java/org/elasticsearch/cluster/node/DiscoveryNodeTests.java @@ -72,9 +72,12 @@ public class DiscoveryNodeTests extends ESTestCase { in.setVersion(Version.V_5_0_0); DiscoveryNode serialized = new DiscoveryNode(in); assertEquals(transportAddress.address().getHostString(), serialized.getHostName()); - assertNotEquals(transportAddress.address().getHostString(), serialized.getAddress().address().getHostString()); + assertEquals(transportAddress.address().getHostString(), serialized.getAddress().address().getHostString()); assertEquals(transportAddress.getAddress(), serialized.getHostAddress()); assertEquals(transportAddress.getAddress(), serialized.getAddress().getAddress()); assertEquals(transportAddress.getPort(), serialized.getAddress().getPort()); + assertFalse("if the minimum compatibility version moves past 5.0.3, remove the special casing in DiscoverNode(StreamInput) and " + + "the TransportAddress(StreamInput, String) constructor", + Version.CURRENT.minimumCompatibilityVersion().onOrAfter(Version.V_5_0_3_UNRELEASED)); } }