Minimize reverse DNS lookups

1 files changed, 99 insertions, 61 deletions

diff --git a/vespajlib/src/main/java/com/yahoo/net/HostName.java b/vespajlib/src/main/java/com/yahoo/net/HostName.java
index 449688f335b..3c87b6c112c 100644
--- a/vespajlib/src/main/java/com/yahoo/net/HostName.java
+++ b/vespajlib/src/main/java/com/yahoo/net/HostName.java
@@ -4,16 +4,17 @@ package com.yahoo.net;
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.net.Inet4Address;
 import java.net.InetAddress;
 import java.net.NetworkInterface;
-import java.net.SocketException;
+import java.net.UnknownHostException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
 /**
  * Utilities for getting the hostname of the system running the JVM.
@@ -30,7 +31,8 @@ public class HostName {
 
     /**
      * Return a fully qualified hostname that resolves to an IP address on a network interface.
-     * Normally this is the same as the 'hostname' command, but on dev machines on WiFi,
+     * Normally this is the same as the 'hostname' command, but if that's not reachable a hostname
+     * for a reachable IP address is returned which is reachable , but on dev machines on WiFi,
      * that IP isn't configured so we prefer a WiFi network interface IP address which is both reachable and
      * has a DNS entry.
      *
@@ -40,7 +42,7 @@ public class HostName {
     public static synchronized String getLocalhost() {
         if (cachedHostName == null) {
             try {
-                cachedHostName = getPreferredAddress().canonicalHostName;
+                cachedHostName = getPreferredHostName();
             } catch (Exception e) {
                 throw new RuntimeException("Failed to find a preferred hostname", e);
             }
@@ -48,35 +50,70 @@ public class HostName {
         return cachedHostName;
     }
 
-    private static Address getPreferredAddress() throws Exception {
-        List<Address> addresses = getReachableNetworkInterfaceAddresses();
-
-        // Prefer address matching the system hostname
+    private static String getPreferredHostName() throws Exception {
+        // Prefer the system hostname
         String systemHostName = getSystemHostName();
-        List<Address> systemAddresses = addresses.stream()
-                .filter(address -> Objects.equals(address.canonicalHostName, systemHostName))
-                .collect(Collectors.toList());
-        if (systemAddresses.size() >= 1) {
-            return systemAddresses.iterator().next();
+        if (isReachable(systemHostName)) {
+            return systemHostName;
+        }
+
+        // Try to find an IP address that resolves in DNS, starting with IPv4 addresses as an optimization.
+
+        List<InetAddress> reachableNonLocalIp4Addresses = new ArrayList<>();
+        List<InetAddress> reachableNonLocalIp6Addresses = new ArrayList<>();
+        for (NetworkInterface networkInterface : Collections.list(NetworkInterface.getNetworkInterfaces())) {
+            for (InetAddress ipAddress : Collections.list(networkInterface.getInetAddresses())) {
+                if (!isReachable(ipAddress)) {
+                    continue;
+                }
+
+                if (ipAddress instanceof Inet4Address) {
+                    reachableNonLocalIp4Addresses.add(ipAddress);
+                } else {
+                    reachableNonLocalIp6Addresses.add(ipAddress);
+                }
+            }
         }
 
-        // Otherwise, prefer non-local address.
-        List<Address> nonLocalAddresses = addresses.stream()
-                .filter(address -> !address.ipAddress.isAnyLocalAddress())
-                .collect(Collectors.toList());
-        if (nonLocalAddresses.size() >= 1) {
-            return nonLocalAddresses.iterator().next();
+        Optional<String> reachableHostName = getAnyHostNameInDns(reachableNonLocalIp4Addresses);
+        if (reachableHostName.isPresent()) {
+            return reachableHostName.get();
         }
 
-        // Otherwise, pick a local address.
-        List<Address> localAddresses = addresses.stream()
-                .filter(address -> address.ipAddress.isAnyLocalAddress())
-                .collect(Collectors.toList());
-        if (localAddresses.size() >= 1) {
-            return localAddresses.iterator().next();
+        reachableHostName = getAnyHostNameInDns(reachableNonLocalIp6Addresses);
+        if (reachableHostName.isPresent()) {
+            return reachableHostName.get();
+        }
+
+        // Use textual representation of IP address since we failed to find a canonical DNS name above.
+
+        if (!reachableNonLocalIp4Addresses.isEmpty()) {
+            return reachableNonLocalIp4Addresses.get(0).getHostName();
+        } else if (!reachableNonLocalIp6Addresses.isEmpty()) {
+            return reachableNonLocalIp6Addresses.get(0).getHostName();
         }
 
-        throw new RuntimeException("Failed to find any addresses on the network interfaces that resolves to a DNS name");
+        // Fall back to localhost.
+
+        return InetAddress.getLocalHost().getCanonicalHostName();
+    }
+
+    private static Optional<String> getAnyHostNameInDns(List<InetAddress> ipAddresses) {
+        for (InetAddress ipAddress : ipAddresses) {
+            // Caveat: This call typically takes seconds on a Mac, and with 5 or so addresses
+            // it is important to avoid calling this too often. That's why we do it here.
+            // We should actually have called this first, then gotten the hostname's InetAddress
+            // address (which may not match ipAddress), and used it for the above reachability test.
+            String hostname = ipAddress.getCanonicalHostName();
+            if (Objects.equals(hostname, ipAddress.getHostAddress())) {
+                // getCanonicalHostName() failed to get the fully qualified domain name
+                continue;
+            }
+
+            return Optional.of(hostname);
+        }
+
+        return Optional.empty();
     }
 
     // public for testing purposes (all testing machines should have a hostname
@@ -92,49 +129,50 @@ public class HostName {
         return hostname;
     }
 
-    private static class Address {
-
-        public final InetAddress ipAddress;
-        public final String canonicalHostName;
+    private static Optional<String> getHostNameIfReachable(InetAddress ipAddress) {
+        if (!isReachable(ipAddress)) {
+            return Optional.empty();
+        }
 
-        public Address(InetAddress ipAddress, String canonicalHostName) {
-            this.ipAddress = ipAddress;
-            this.canonicalHostName = canonicalHostName;
+        // Caveat: This call typically takes seconds on a Mac, and with 5 or so addresses
+        // it is important to avoid calling this too often. That's why we do it here.
+        // We should actually have called this first, then gotten the hostname's InetAddress
+        // address (which may not match ipAddress), and used it for the above reachability test.
+        String hostname = ipAddress.getCanonicalHostName();
+        if (Objects.equals(hostname, ipAddress.getHostAddress())) {
+            // getCanonicalHostName() failed to get the fully qualified domain name
+            return Optional.empty();
         }
 
+        return Optional.of(hostname);
     }
 
-    private static List<Address> getReachableNetworkInterfaceAddresses() throws SocketException {
-        List<Address> addresses = new ArrayList<>();
-
-        for (NetworkInterface networkInterface : Collections.list(NetworkInterface.getNetworkInterfaces())) {
-            for (InetAddress ipAddress : Collections.list(networkInterface.getInetAddresses())) {
-                String hostname = ipAddress.getCanonicalHostName();
-                if (Objects.equals(hostname, ipAddress.getHostAddress())) {
-                    // getCanonicalHostName() failed to get the fully qualified domain name
-                    continue;
-                }
-
-                try {
-                    // ping says ~50ms on my Fedora Lenovo, but that seems a lot for pinging oneself  - hakon
-                    int timeoutMs = 100;
-                    if ( ! ipAddress.isReachable(timeoutMs)) {
-                        // The network interface may be down, ignore address
-                        logger.log(Level.INFO, ipAddress.toString() + 
-                                               " is unreachable w/" + timeoutMs + "ms timeout, ignoring address");
-                        continue;
-                    }
-                } catch (IOException e) {
-                    // Why would this be different from !isReachable ?
-                    logger.log(Level.INFO, "Failed testing reachability of " + ipAddress + ", ignoring address", e);
-                    continue;
-                }
+    private static boolean isReachable(String hostname) {
+        try {
+            InetAddress ipAddress = InetAddress.getByName(hostname);
+            return isReachable(ipAddress);
+        } catch (UnknownHostException e) {
+            return false;
+        }
+    }
 
-                addresses.add(new Address(ipAddress, hostname));
+    private static boolean isReachable(InetAddress ipAddress) {
+        try {
+            // ping says ~50ms on my Fedora Lenovo, but that seems a lot for pinging oneself  - hakon
+            int timeoutMs = 100;
+            if ( ! ipAddress.isReachable(timeoutMs)) {
+                // The network interface may be down, ignore address
+                logger.log(Level.INFO, ipAddress.toString() +
+                        " is unreachable w/" + timeoutMs + "ms timeout, ignoring address");
+                return false;
             }
-        }
 
-        return addresses;
+            return true;
+        } catch (IOException e) {
+            // Why would this be different from !isReachable ?
+            logger.log(Level.INFO, "Failed testing reachability of " + ipAddress + ", ignoring address", e);
+            return false;
+        }
     }
 
 }