ZOOKEEPER-3100. Add feature flag with false default value

Author: anmolnar

zookeeper-docs/src/main/resources/markdown/zookeeperProgrammers.md

@@ -1376,6 +1376,22 @@ and [SASL authentication for ZooKeeper](https://cwiki.apache.org/confluence/disp
 * *zookeeper.kinit* :
     Specifies path to kinit binary. Default is "/usr/bin/kinit".
 
+* *zookeeper.shuffleDnsResponse* :
+  **New in 3.10.0:**
+  Specifies whether ZooKeeper client should randomly pick an IP address from the DNS lookup query result when resolving 
+  server addresses or not. This is a feature flag in order to keep the old behavior of the default DNS resolver in 
+  `StaticHostProvider`, because we disabled it by default. The reason behind that is shuffling the response of DNS queries 
+  shadows JVM network property `java.net.preferIPv6Addresses` (default: false). This property controls whether JVM's built-in 
+  resolver should prioritize v4 (false value) or v6 (true value) addresses when putting together the list of IP addresses 
+  in the result. In other words the above Java system property was completely ineffective in the case of ZooKeeper server host 
+  resolution protocol and this must have been fixed. In a dual stack environment one might want to prefer one stack over 
+  the other which, in case of Java processes, should be controlled by JVM network properties and ZooKeeper must honor 
+  these settings. Since the old behavior has been with us since day zero, we introduced this feature flag in case you 
+  need it. Such case could be when you have a buggy DNS server which responds IP addresses always in the same order and 
+  you want to randomize that.
+  Default: false
+
+
 <a name="C+Binding"></a>
 
 ### C Binding

zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java

@@ -1140,7 +1140,7 @@ public ZooKeeper(ZooKeeperOptions options) throws IOException {
         if (options.getHostProvider() != null) {
             hostProvider = options.getHostProvider().apply(connectStringParser.getServerAddresses());
         } else {
-            hostProvider = new StaticHostProvider(connectStringParser.getServerAddresses());
+            hostProvider = new StaticHostProvider(connectStringParser.getServerAddresses(), clientConfig);
         }
         this.hostProvider = hostProvider;
 

zookeeper-server/src/main/java/org/apache/zookeeper/client/StaticHostProvider.java

@@ -50,6 +50,8 @@ public interface Resolver {
 
     private static final Logger LOG = LoggerFactory.getLogger(StaticHostProvider.class);
 
+    private ZKClientConfig clientConfig = null;
+
     private List<InetSocketAddress> serverAddresses = new ArrayList<>(5);
 
     private Random sourceOfRandomness;
@@ -90,6 +92,26 @@ public InetAddress[] getAllByName(String name) throws UnknownHostException {
         });
     }
 
+    /**
+     * Constructs a SimpleHostSet with ZKClientConfig.
+     *
+     * Introduced this new overload in 3.10.0 in order to take advantage of some newly introduced feature flags. Like
+     * the shuffle (old) / not to shuffle (new) behavior of DNS resolution.
+     *
+     * @param serverAddresses
+     *            possibly unresolved ZooKeeper server addresses
+     * @param clientConfig
+     *            ZooKeeper client configuration
+     */
+    public StaticHostProvider(Collection<InetSocketAddress> serverAddresses, ZKClientConfig clientConfig) {
+        init(serverAddresses, System.currentTimeMillis() ^ this.hashCode(), new Resolver() {
+            @Override
+            public InetAddress[] getAllByName(String name) throws UnknownHostException {
+                return InetAddress.getAllByName(name);
+            }
+        }, clientConfig);
+    }
+
     /**
      * Constructs a SimpleHostSet.
      *
@@ -125,6 +147,12 @@ public InetAddress[] getAllByName(String name) throws UnknownHostException {
     }
 
     private void init(Collection<InetSocketAddress> serverAddresses, long randomnessSeed, Resolver resolver) {
+        init(serverAddresses, randomnessSeed, resolver, null);
+    }
+
+    private void init(Collection<InetSocketAddress> serverAddresses, long randomnessSeed, Resolver resolver,
+                      ZKClientConfig clientConfig) {
+        this.clientConfig = clientConfig == null ? new ZKClientConfig() : clientConfig;
         this.sourceOfRandomness = new Random(randomnessSeed);
         this.resolver = resolver;
         if (serverAddresses.isEmpty()) {
@@ -142,6 +170,9 @@ private InetSocketAddress resolve(InetSocketAddress address) {
             if (resolvedAddresses.isEmpty()) {
                 return address;
             }
+            if (clientConfig.isShuffleDnsResponseEnabled()) {
+                Collections.shuffle(resolvedAddresses);
+            }
             return new InetSocketAddress(resolvedAddresses.get(0), address.getPort());
         } catch (UnknownHostException e) {
             LOG.error("Unable to resolve address: {}", address.toString(), e);

zookeeper-server/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java

@@ -60,6 +60,13 @@ public class ZKClientConfig extends ZKConfig {
      * Feature is disabled by default.
      */
     public static final long ZOOKEEPER_REQUEST_TIMEOUT_DEFAULT = 0;
+    /**
+     * True value preserves the old behavior is to shuffle the addresses in DNS response regardless of address type.
+     * This could help with buggy DNS servers which always return addresses in the same order, but at the same time
+     * ignores the JVM option java.net.preferIPv6Addresses.
+     */
+    public static final String ZOOKEEPER_SHUFFLE_DNS_RESPONSE = "zookeeper.shuffleDnsResponse";
+    public static final boolean ZOOKEEPER_SHUFFLE_DNS_RESPONSE_DEFAULT = false;
 
     public ZKClientConfig() {
         super();
@@ -134,6 +141,14 @@ public boolean isSaslClientEnabled() {
         return Boolean.valueOf(getProperty(ENABLE_CLIENT_SASL_KEY, ENABLE_CLIENT_SASL_DEFAULT));
     }
 
+    /**
+     * Return true if we need to use the old behavior of default DNS resolver and always shuffle the IP addresses
+     * in the DNS lookup response in order to pick a completely random IP address.
+     */
+    public boolean isShuffleDnsResponseEnabled() {
+        return getBoolean(ZOOKEEPER_SHUFFLE_DNS_RESPONSE, ZOOKEEPER_SHUFFLE_DNS_RESPONSE_DEFAULT);
+    }
+
     /**
      * Get the value of the <code>key</code> property as an <code>long</code>.
      * If property is not set, the provided <code>defaultValue</code> is