HADOOP-18628. IPC Server Connection should log host name before returning VersionMismatch error (#5385)

Contributed by Viraj Jasani

hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/ipc/Server.java

@@ -1871,11 +1871,26 @@ public abstract class Server {
     private long lastContact;
     private int dataLength;
     private Socket socket;
+
     // Cache the remote host & port info so that even if the socket is 
     // disconnected, we can say where it used to connect to.
-    private String hostAddress;
+
-    private int remotePort;
+    /**
-    private InetAddress addr;
+     * Client Host IP address from where the socket connection is being established to the Server.
+     */
+    private final String hostAddress;
+    /**
+     * Client remote port used for the given socket connection.
+     */
+    private final int remotePort;
+    /**
+     * Address to which the socket is connected to.
+     */
+    private final InetAddress addr;
+    /**
+     * Client Host address from where the socket connection is being established to the Server.
+     */
+    private final String hostName;
     
     IpcConnectionContextProto connectionContext;
     String protocolName;
@@ -1919,8 +1934,12 @@ public abstract class Server {
       this.isOnAuxiliaryPort = isOnAuxiliaryPort;
       if (addr == null) {
         this.hostAddress = "*Unknown*";
+        this.hostName = this.hostAddress;
       } else {
+        // host IP address
         this.hostAddress = addr.getHostAddress();
+        // host name for the IP address
+        this.hostName = addr.getHostName();
       }
       this.remotePort = socket.getPort();
       this.responseQueue = new LinkedList<RpcCall>();
@@ -1936,7 +1955,7 @@ public abstract class Server {
 
     @Override
     public String toString() {
-      return getHostAddress() + ":" + remotePort; 
+      return hostName + ":" + remotePort + " / " + hostAddress + ":" + remotePort;
     }
 
     boolean setShouldClose() {
@@ -2338,19 +2357,18 @@ public abstract class Server {
             return -1;
           }
 
-          if(!RpcConstants.HEADER.equals(dataLengthBuffer)) {
+          if (!RpcConstants.HEADER.equals(dataLengthBuffer)) {
-            LOG.warn("Incorrect RPC Header length from {}:{} "
+            LOG.warn("Incorrect RPC Header length from {}:{} / {}:{}. Expected: {}. Actual: {}",
-                + "expected length: {} got length: {}",
+                hostName, remotePort, hostAddress, remotePort, RpcConstants.HEADER,
-                hostAddress, remotePort, RpcConstants.HEADER, dataLengthBuffer);
+                dataLengthBuffer);
             setupBadVersionResponse(version);
             return -1;
           }
           if (version != CURRENT_VERSION) {
             //Warning is ok since this is not supposed to happen.
-            LOG.warn("Version mismatch from " +
+            LOG.warn("Version mismatch from {}:{} / {}:{}. "
-                     hostAddress + ":" + remotePort +
+                    + "Expected version: {}. Actual version: {} ", hostName,
-                     " got version " + version + 
+                remotePort, hostAddress, remotePort, CURRENT_VERSION, version);
-                     " expected version " + CURRENT_VERSION);
             setupBadVersionResponse(version);
             return -1;
           }

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestIPC.java

@@ -1170,6 +1170,10 @@ public class TestIPC {
 
     call(client, addr, serviceClass, conf);
     Connection connection = server.getConnections()[0];
+    LOG.info("Connection is from: {}", connection);
+    assertEquals(
+        "Connection string representation should include both IP address and Host name", 2,
+        connection.toString().split(" / ").length);
     int serviceClass2 = connection.getServiceClass();
     assertFalse(noChanged ^ serviceClass == serviceClass2);
     client.stop();

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/ipc/TestRPC.java

@@ -1827,6 +1827,11 @@ public class TestRPC extends TestRpcBase {
           // if it wasn't fatal, verify there's only one open connection.
           Connection[] conns = server.getConnections();
           assertEquals(reqName, 1, conns.length);
+          String connectionInfo = conns[0].toString();
+          LOG.info("Connection is from: {}", connectionInfo);
+          assertEquals(
+              "Connection string representation should include both IP address and Host name", 2,
+              connectionInfo.split(" / ").length);
           // verify whether the connection should have been reused.
           if (isDisconnected) {
             assertNotSame(reqName, lastConn, conns[0]);