This is an automated email from the ASF dual-hosted git repository.
andor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git
The following commit(s) were added to refs/heads/master by this push:
new 229721e98 ZOOKEEPER-3100: ZooKeeper client times out due to random
choice of resolved addresses
229721e98 is described below
commit 229721e98d562f1d3a74e9f8212155483dce269f
Author: Andor Molnár <[email protected]>
AuthorDate: Wed Dec 3 16:07:09 2025 -0600
ZOOKEEPER-3100: ZooKeeper client times out due to random choice of resolved
addresses
Reviewers: kezhuw
Author: anmolnar
Closes #2338 from anmolnar/ZOOKEEPER-3100
---
.../resources/markdown/zookeeperProgrammers.md | 16 +++++++++
.../main/java/org/apache/zookeeper/ZooKeeper.java | 2 +-
.../zookeeper/client/StaticHostProvider.java | 32 ++++++++++++++++-
.../apache/zookeeper/client/ZKClientConfig.java | 15 ++++++++
.../zookeeper/test/StaticHostProviderTest.java | 40 ++++++++++++++++++++++
5 files changed, 103 insertions(+), 2 deletions(-)
diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperProgrammers.md
b/zookeeper-docs/src/main/resources/markdown/zookeeperProgrammers.md
index 620f0f508..59008215c 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperProgrammers.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperProgrammers.md
@@ -1381,6 +1381,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
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
index 8831c0622..b3b5af454 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooKeeper.java
+++ b/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;
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/client/StaticHostProvider.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/client/StaticHostProvider.java
index bfd771943..e07754c35 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/client/StaticHostProvider.java
+++
b/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,7 +170,9 @@ private InetSocketAddress resolve(InetSocketAddress
address) {
if (resolvedAddresses.isEmpty()) {
return address;
}
- Collections.shuffle(resolvedAddresses);
+ 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);
diff --git
a/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java
index 8ef00ac43..5472838bd 100644
---
a/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java
+++
b/zookeeper-server/src/main/java/org/apache/zookeeper/client/ZKClientConfig.java
@@ -62,6 +62,13 @@ public class ZKClientConfig extends ZKConfig {
public static final long ZOOKEEPER_REQUEST_TIMEOUT_DEFAULT = 0;
public static final String ZK_SASL_CLIENT_ALLOW_REVERSE_DNS =
"zookeeper.sasl.client.allowReverseDnsLookup";
public static final boolean ZK_SASL_CLIENT_ALLOW_REVERSE_DNS_DEFAULT =
false;
+ /**
+ * 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();
@@ -137,6 +144,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
diff --git
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/StaticHostProviderTest.java
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/StaticHostProviderTest.java
index 2289847d4..7ab207519 100644
---
a/zookeeper-server/src/test/java/org/apache/zookeeper/test/StaticHostProviderTest.java
+++
b/zookeeper-server/src/test/java/org/apache/zookeeper/test/StaticHostProviderTest.java
@@ -30,9 +30,12 @@
import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
+import java.net.Inet4Address;
+import java.net.Inet6Address;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
@@ -115,6 +118,7 @@ public void testNextDoesNotSleepForZero() {
assertTrue(5 > stop - start);
}
+
@Test
public void testEmptyServerAddressesList() {
assertThrows(IllegalArgumentException.class, () -> {
@@ -888,6 +892,42 @@ public void testReResolvingLocalhost() {
+ hostProvider.size() + " (after), " + sizeBefore + "
(before)");
}
+ @Test
+ public void testResolverKeepsResolutionOrderPreferV6() {
+ // Arrange
+ Collection<InetSocketAddress> servers = new ArrayList<>();
+ servers.add(InetSocketAddress.createUnresolved("test-server-name",
1111));
+ InetAddress[] resolvedAddresses = new InetAddress[] {
+ mock(Inet6Address.class),
+ mock(Inet4Address.class)
+ };
+ StaticHostProvider staticHostProvider = new
StaticHostProvider(servers, name -> resolvedAddresses);
+
+ // Act & Assert
+ for (int i = 0; i < 100; i++) {
+ InetSocketAddress next = staticHostProvider.next(0);
+ assertSame(next.getAddress(), resolvedAddresses[0], "Default
resolver should always return the first address");
+ }
+ }
+
+ @Test
+ public void testResolverKeepsResolutionOrderPreferV4() {
+ // Arrange
+ Collection<InetSocketAddress> servers = new ArrayList<>();
+ servers.add(InetSocketAddress.createUnresolved("test-server-name",
1111));
+ InetAddress[] resolvedAddresses = new InetAddress[] {
+ mock(Inet4Address.class),
+ mock(Inet6Address.class)
+ };
+ StaticHostProvider staticHostProvider = new
StaticHostProvider(servers, name -> resolvedAddresses);
+
+ // Act & Assert
+ for (int i = 0; i < 100; i++) {
+ InetSocketAddress next = staticHostProvider.next(0);
+ assertSame(next.getAddress(), resolvedAddresses[0], "Default
resolver should always return the first address");
+ }
+ }
+
private StaticHostProvider getHostProviderUnresolved(byte size) {
return new StaticHostProvider(getUnresolvedServerAddresses(size),
r.nextLong());
}
