[improve] Enable hostname specification as a script argument for topology script

bookkeeper-server/src/main/java/org/apache/bookkeeper/net/CachedDNSToSwitchMapping.java

@@ -31,7 +31,7 @@
  *
  */
 public class CachedDNSToSwitchMapping extends AbstractDNSToSwitchMapping {
-  private Map<String, String> cache = new ConcurrentHashMap<String, String>();
+  private Map<String, String> cache = new ConcurrentHashMap<>();
 
   /**
    * The uncached mapping.
@@ -46,7 +46,8 @@ public CachedDNSToSwitchMapping(DNSToSwitchMapping rawMapping) {
     this.rawMapping = rawMapping;
   }
 
-  // we'll use IP Address for these mappings.
+  // we'll use IP Address or Host Name for these mappings.
+  // The default value is the IP address.
   @Override
   public boolean useHostName() {
     return false;
@@ -58,7 +59,7 @@ public boolean useHostName() {
    */
   private List<String> getUncachedHosts(List<String> names) {
     // find out all names without cached resolved location
-    List<String> unCachedHosts = new ArrayList<String>(names.size());
+    List<String> unCachedHosts = new ArrayList<>(names.size());
     for (String name : names) {
       if (cache.get(name) == null) {
         unCachedHosts.add(name);
@@ -91,7 +92,7 @@ private void cacheResolvedHosts(List<String> uncachedHosts,
    *  or null if any of the names are not currently in the cache
    */
   private List<String> getCachedHosts(List<String> names) {
-    List<String> result = new ArrayList<String>(names.size());
+    List<String> result = new ArrayList<>(names.size());
     // Construct the result
     for (String name : names) {
       String networkLocation = cache.get(name);
@@ -106,23 +107,27 @@ private List<String> getCachedHosts(List<String> names) {
 
   @Override
   public List<String> resolve(List<String> names) {
-    // normalize all input names to be in the form of IP addresses
-    names = NetUtils.normalizeHostNames(names);
-
-    List <String> result = new ArrayList<String>(names.size());
+    List <String> result = new ArrayList<>(names.size());
     if (names.isEmpty()) {
       return result;
     }
 
+    if (useHostName()) {
+      // normalize all input names to be in the form of host name
+      names = NetUtils.normalizeToHostNames(names);
+    } else {
+      // normalize all input names to be in the form of IP addresses
+      names = NetUtils.normalizeToIPAddresses(names);
+    }
+
     List<String> uncachedHosts = getUncachedHosts(names);
 
     // Resolve the uncached hosts
     List<String> resolvedHosts = rawMapping.resolve(uncachedHosts);
-    //cache them
+    // cache them
     cacheResolvedHosts(uncachedHosts, resolvedHosts);
-    //now look up the entire list in the cache
+    // now look up the entire list in the cache
     return getCachedHosts(names);
-
   }
 
   /**
@@ -131,7 +136,7 @@ public List<String> resolve(List<String> names) {
    */
   @Override
   public Map<String, String> getSwitchMap() {
-    Map<String, String> switchMap = new HashMap<String, String>(cache);
+    Map<String, String> switchMap = new HashMap<>(cache);
     return switchMap;
   }
 

bookkeeper-server/src/main/java/org/apache/bookkeeper/net/CommonConfigurationKeys.java

@@ -29,6 +29,8 @@ public interface CommonConfigurationKeys {
     String NET_TOPOLOGY_SCRIPT_FILE_NAME_KEY = "networkTopologyScriptFileName";
     // number of arguments that network topology resolve script used
     String NET_TOPOLOGY_SCRIPT_NUMBER_ARGS_KEY = "networkTopologyScriptNumberArgs";
+    // use hostname as an argument for network topology resolve script
+    String NET_TOPOLOGY_SCRIPT_USE_HOSTNAME_ARGS_KEY = "networkTopologyScriptUseHostNameArgs";
     // default value of NET_TOPOLOGY_SCRIPT_NUMBER_ARGS_KEY
     int NET_TOPOLOGY_SCRIPT_NUMBER_ARGS_DEFAULT = 100;
 }

bookkeeper-server/src/main/java/org/apache/bookkeeper/net/NetUtils.java

@@ -40,7 +40,7 @@ public class NetUtils {
      *             either a textual representation its IP address or its host name
      * @return its IP address in the string format
      */
-    public static String normalizeHostName(String name) {
+    public static String normalizeToIPAddress(String name) {
         try {
             return InetAddress.getByName(name).getHostAddress();
         } catch (UnknownHostException e) {
@@ -54,19 +54,51 @@ public static String normalizeHostName(String name) {
      *
      * @param names a collection of string representations of hosts
      * @return a list of corresponding IP addresses in the string format
-     * @see #normalizeHostName(String)
+     * @see #normalizeToIPAddress(String)
      */
-    public static List<String> normalizeHostNames(Collection<String> names) {
-        List<String> hostNames = new ArrayList<String>(names.size());
+    public static List<String> normalizeToIPAddresses(Collection<String> names) {
+        List<String> ipAddresses = new ArrayList<>(names.size());
         for (String name : names) {
-            hostNames.add(normalizeHostName(name));
+            ipAddresses.add(normalizeToIPAddress(name));
+        }
+        return ipAddresses;
+    }
+
+    /**
+     * Given a string representation of an IP address, return its host name
+     * in textual presentation.
+     *
+     * @param name a string representation of an IP Address:
+     *             either a textual representation its IP address or its host name
+     * @return its host name in the string format
+     */
+    public static String normalizeToHostName(String name) {
+        try {
+            return InetAddress.getByName(name).getHostName();
+        } catch (UnknownHostException e) {
+            return name;
+        }
+    }
+
+    /**
+     * Given a collection of string representation of IP addresses, return a list of
+     * corresponding hosts in the textual representation.
+     *
+     * @param names a collection of string representations of IP addresses
+     * @return a list of corresponding hosts in the string format
+     * @see #normalizeToHostName(String)
+     */
+    public static List<String> normalizeToHostNames(Collection<String> names) {
+        List<String> hostNames = new ArrayList<>(names.size());
+        for (String name : names) {
+            hostNames.add(normalizeToHostName(name));
         }
         return hostNames;
     }
 
     public static String resolveNetworkLocation(DNSToSwitchMapping dnsResolver,
                                                 BookieSocketAddress addr) {
-        List<String> names = new ArrayList<String>(1);
+        List<String> names = new ArrayList<>(1);
 
         InetSocketAddress inetSocketAddress = addr.getSocketAddress();
         if (dnsResolver.useHostName()) {

bookkeeper-server/src/main/java/org/apache/bookkeeper/net/ScriptBasedMapping.java

@@ -63,6 +63,11 @@ public final class ScriptBasedMapping extends CachedDNSToSwitchMapping {
      * {@value}.
      */
     static final String SCRIPT_ARG_COUNT_KEY = CommonConfigurationKeys.NET_TOPOLOGY_SCRIPT_NUMBER_ARGS_KEY;
+    /**
+     * Key to specify whether hostname should be used as an argument
+     * {@value}.
+     */
+    static final String SCRIPT_USE_HOSTNAME_KEY = CommonConfigurationKeys.NET_TOPOLOGY_SCRIPT_USE_HOSTNAME_ARGS_KEY;
     /**
      * Text used in the {@link #toString()} method if there is no string
      * {@value}.
@@ -122,13 +127,19 @@ public void setConf(Configuration conf) {
         getRawMapping().setConf(conf);
     }
 
+    @Override
+    public boolean useHostName() {
+        return getRawMapping().useHostName();
+    }
+
     /**
      * This is the uncached script mapping that is fed into the cache managed
      * by the superclass {@link CachedDNSToSwitchMapping}.
      */
     private static final class RawScriptBasedMapping extends AbstractDNSToSwitchMapping {
         private String scriptName;
         private int maxArgs; //max hostnames per call of the script
+        private boolean useHostName;
         private static final Logger LOG = LoggerFactory.getLogger(RawScriptBasedMapping.class);
 
         /*
@@ -149,6 +160,7 @@ protected void validateConf() {
                 if (StringUtils.isNotBlank(scriptNameConfValue)) {
                     scriptName = scriptNameConfValue;
                     maxArgs = conf.getInt(SCRIPT_ARG_COUNT_KEY, DEFAULT_ARG_COUNT);
+                    useHostName = conf.getBoolean(SCRIPT_USE_HOSTNAME_KEY, false);
                 } else {
                     scriptName = null;
                     maxArgs = 0;
@@ -290,5 +302,10 @@ public void reloadCachedMappings() {
             // Nothing to do here, since RawScriptBasedMapping has no cache, and
             // does not inherit from CachedDNSToSwitchMapping
         }
+
+        @Override
+        public boolean useHostName() {
+            return useHostName;
+        }
     }
 }

bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestRackawareEnsemblePlacementPolicyUsingScript.java

@@ -26,9 +26,14 @@
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.times;
 
 import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import io.netty.util.HashedWheelTimer;
+import java.net.InetAddress;
 import java.util.ArrayList;
 import java.util.HashSet;
 import java.util.List;
@@ -41,13 +46,16 @@
 import org.apache.bookkeeper.net.BookieSocketAddress;
 import org.apache.bookkeeper.net.CommonConfigurationKeys;
 import org.apache.bookkeeper.net.DNSToSwitchMapping;
+import org.apache.bookkeeper.net.NetUtils;
 import org.apache.bookkeeper.net.ScriptBasedMapping;
 import org.apache.bookkeeper.stats.NullStatsLogger;
 import org.apache.bookkeeper.util.Shell;
 import org.junit.After;
 import org.junit.Assume;
 import org.junit.Before;
 import org.junit.Test;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -462,6 +470,62 @@ public void testIfValidateConfFails() throws Exception {
         }
     }
 
+    @Test
+    public void testUseHostnameArgsOption() throws Exception {
+        ignoreTestIfItIsWindowsOS();
+        repp.uninitalize();
+
+        // Mock NetUtils.
+        try (MockedStatic<NetUtils> netUtils = Mockito.mockStatic(NetUtils.class)) {
+            netUtils.when(() -> NetUtils.resolveNetworkLocation(
+                    any(DNSToSwitchMapping.class), any(BookieSocketAddress.class))).thenCallRealMethod();
+            netUtils.when(() -> NetUtils.normalizeToHostNames(any())).thenCallRealMethod();
+            netUtils.when(() -> NetUtils.normalizeToHostName(anyString())).thenCallRealMethod();
+
+            netUtils.when(() -> NetUtils.normalizeToHostName(
+                    eq(InetAddress.getLocalHost().getHostAddress()))).thenReturn("bookie1");
+            netUtils.when(() -> NetUtils.normalizeToHostName(eq("127.0.0.4"))).thenReturn("bookie22");
+
+            // Initialize RackawareEnsemblePlacementPolicy.
+            ClientConfiguration newConf = new ClientConfiguration();
+            newConf.setProperty(REPP_DNS_RESOLVER_CLASS, ScriptBasedMapping.class.getName());
+            newConf.setProperty(CommonConfigurationKeys.NET_TOPOLOGY_SCRIPT_FILE_NAME_KEY,
+                    "src/test/resources/networkmappingscript.sh");
+            newConf.setProperty(CommonConfigurationKeys.NET_TOPOLOGY_SCRIPT_USE_HOSTNAME_ARGS_KEY, true);
+            timer = new HashedWheelTimer(
+                    new ThreadFactoryBuilder().setNameFormat("TestTimer-%d").build(),
+                    conf.getTimeoutTimerTickDurationMs(), TimeUnit.MILLISECONDS,
+                    conf.getTimeoutTimerNumTicks());
+            repp = new RackawareEnsemblePlacementPolicy();
+            repp.initialize(newConf, Optional.<DNSToSwitchMapping>empty(), timer,
+                    DISABLE_ALL, NullStatsLogger.INSTANCE, BookieSocketAddress.LEGACY_BOOKIEID_RESOLVER);
+
+            // Join Bookie2, Bookie22, Bookie3, and Bookie33.
+            BookieSocketAddress addr1 = new BookieSocketAddress("bookie2", 3181);   // /2 rack
+            BookieSocketAddress addr2 = new BookieSocketAddress("127.0.0.4", 3181); // /2 rack
+            BookieSocketAddress addr3 = new BookieSocketAddress("bookie3", 3181);   // /3 rack
+            BookieSocketAddress addr4 = new BookieSocketAddress("bookie33", 3181);  // /3 rack
+            Set<BookieId> addrs = new HashSet<>();
+            addrs.add(addr1.toBookieId());
+            addrs.add(addr2.toBookieId());
+            addrs.add(addr3.toBookieId());
+            addrs.add(addr4.toBookieId());
+            repp.onClusterChanged(addrs, new HashSet<>());
+
+            // Remove Bookie2.
+            addrs.remove(addr1.toBookieId());
+            repp.onClusterChanged(addrs, new HashSet<>());
+
+            BookieId replacedBookie = repp.replaceBookie(1, 1, 1, null, new ArrayList<>(),
+                    addr1.toBookieId(), new HashSet<>()).getResult();
+            assertEquals(addr2.toBookieId(), replacedBookie);
+
+            netUtils.verify(() -> NetUtils.normalizeToHostName(
+                    eq(InetAddress.getLocalHost().getHostAddress())), times(1));
+            netUtils.verify(() -> NetUtils.normalizeToHostName(eq("127.0.0.4")), times(1));
+        }
+    }
+
     private int getNumCoveredWriteQuorums(List<BookieId> ensemble, int writeQuorumSize)
             throws Exception {
         int ensembleSize = ensemble.size();

conf/bk_server.conf

@@ -1023,6 +1023,9 @@ statsProviderClass=org.apache.bookkeeper.stats.prometheus.PrometheusMetricsProvi
 # The max number of args used in the script provided at `networkTopologyScriptFileName`
 # networkTopologyScriptNumberArgs=100
 
+# Whether to use hostname instead of IP address as an argument for the script provided at `networkTopologyScriptFileName`
+# networkTopologyScriptUseHostNameArgs=false
+
 # minimum number of racks per write quorum. RackawareEnsemblePlacementPolicy will try to
 # get bookies from atleast 'minNumRacksPerWriteQuorum' racks for a writeQuorum.
 # minNumRacksPerWriteQuorum=2

site3/website/docs/reference/config.md

@@ -322,6 +322,7 @@ The table below lists parameters that you can set to configure bookies. All conf
 | reppDnsResolverClass | The DNS resolver class used for resolving network locations for bookies. The setting is used<br />when using either RackawareEnsemblePlacementPolicy and RegionAwareEnsemblePlacementPolicy.<br /> | org.apache.bookkeeper.net.ScriptBasedMapping | 
 | networkTopologyScriptFileName | The bash script used by `ScriptBasedMapping` DNS resolver for resolving bookies' network locations.<br /> |  | 
 | networkTopologyScriptNumberArgs | The max number of args used in the script provided at `networkTopologyScriptFileName`.<br /> |  | 
+| networkTopologyScriptUseHostNameArgs | Whether to use hostname instead of IP address as an argument for the script provided at `networkTopologyScriptFileName`.<br /> | false | 
 | minNumRacksPerWriteQuorum | minimum number of racks per write quorum. RackawareEnsemblePlacementPolicy will try to get bookies from atleast 'minNumRacksPerWriteQuorum' racks for a writeQuorum.<br /> |  | 
 | enforceMinNumRacksPerWriteQuorum | 'enforceMinNumRacksPerWriteQuorum' enforces RackawareEnsemblePlacementPolicy to pick bookies from 'minNumRacksPerWriteQuorum' racks for a writeQuorum. If it cann't find bookie then it would throw BKNotEnoughBookiesException instead of picking random one.<br /> |  | 
 | ignoreLocalNodeInPlacementPolicy | 'ignoreLocalNodeInPlacementPolicy' specifies whether to ignore localnode in the internal logic of placement policy. If it is not possible or useful to use Bookkeeper client node's (or AutoReplicator) rack/region info. for placement policy then it is better to ignore localnode instead of false alarming with log lines and metrics.<br /> |  |