You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@zookeeper.apache.org by ha...@apache.org on 2018/07/16 03:45:50 UTC
zookeeper git commit: ZOOKEEPER-2184: Zookeeper Client should
re-resolve hosts when connection attempts fail
Repository: zookeeper
Updated Branches:
refs/heads/master 28de451aa -> 0a311873d
ZOOKEEPER-2184: Zookeeper Client should re-resolve hosts when connection attempts fail
This is the master/3.5 port of #451
Author: Andor Molnar <an...@cloudera.com>
Author: Andor Molnar <an...@apache.org>
Reviewers: Michael Han <ha...@apache.org>, Flavio Junqueira <fp...@apache.org>, Edward Ribeiro <ed...@gmail.com>, Mark Fenes <mf...@cloudera.com>, Abraham Fine <af...@apache.org>
Closes #534 from anmolnar/ZOOKEEPER-2184_master
Project: http://git-wip-us.apache.org/repos/asf/zookeeper/repo
Commit: http://git-wip-us.apache.org/repos/asf/zookeeper/commit/0a311873
Tree: http://git-wip-us.apache.org/repos/asf/zookeeper/tree/0a311873
Diff: http://git-wip-us.apache.org/repos/asf/zookeeper/diff/0a311873
Branch: refs/heads/master
Commit: 0a311873deb1847703c9b62716c626ce43d4ba48
Parents: 28de451
Author: Andor Molnar <an...@cloudera.com>
Authored: Sun Jul 15 20:45:45 2018 -0700
Committer: Michael Han <ha...@apache.org>
Committed: Sun Jul 15 20:45:45 2018 -0700
----------------------------------------------------------------------
build.xml | 2 +-
.../apache/zookeeper/client/HostProvider.java | 5 +-
.../zookeeper/client/StaticHostProvider.java | 121 +++--
.../zookeeper/test/StaticHostProviderTest.java | 479 +++++++++++++++++--
4 files changed, 527 insertions(+), 80 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/0a311873/build.xml
----------------------------------------------------------------------
diff --git a/build.xml b/build.xml
index b8aa5b3..464e155 100644
--- a/build.xml
+++ b/build.xml
@@ -39,7 +39,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
<property name="netty.version" value="3.10.6.Final"/>
<property name="junit.version" value="4.12"/>
- <property name="mockito.version" value="1.8.2"/>
+ <property name="mockito.version" value="1.8.5"/>
<property name="checkstyle.version" value="6.13"/>
<property name="commons-collections.version" value="3.2.2"/>
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/0a311873/src/java/main/org/apache/zookeeper/client/HostProvider.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/client/HostProvider.java b/src/java/main/org/apache/zookeeper/client/HostProvider.java
index ec3ca97..caeedcc 100644
--- a/src/java/main/org/apache/zookeeper/client/HostProvider.java
+++ b/src/java/main/org/apache/zookeeper/client/HostProvider.java
@@ -34,8 +34,9 @@ import java.util.Collection;
*
* * The size() of a HostProvider may never be zero.
*
- * A HostProvider must return resolved InetSocketAddress instances on next(),
- * but it's up to the HostProvider, when it wants to do the resolving.
+ * A HostProvider must return resolved InetSocketAddress instances on next() if the next address is resolvable.
+ * In that case, it's up to the HostProvider, whether it returns the next resolvable address in the list or return
+ * the next one as UnResolved.
*
* Different HostProvider could be imagined:
*
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/0a311873/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
----------------------------------------------------------------------
diff --git a/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java b/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
index cb53936..0602103 100644
--- a/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
+++ b/src/java/main/org/apache/zookeeper/client/StaticHostProvider.java
@@ -22,6 +22,7 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
@@ -32,11 +33,19 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * Most simple HostProvider, resolves only on instantiation.
- *
+ * Most simple HostProvider, resolves on every next() call.
+ *
+ * Please be aware that although this class doesn't do any DNS caching, there're multiple levels of caching already
+ * present across the stack like in JVM, OS level, hardware, etc. The best we could do here is to get the most recent
+ * address from the underlying system which is considered up-to-date.
+ *
*/
@InterfaceAudience.Public
public final class StaticHostProvider implements HostProvider {
+ public interface Resolver {
+ InetAddress[] getAllByName(String name) throws UnknownHostException;
+ }
+
private static final Logger LOG = LoggerFactory
.getLogger(StaticHostProvider.class);
@@ -64,6 +73,8 @@ public final class StaticHostProvider implements HostProvider {
private float pOld, pNew;
+ private Resolver resolver;
+
/**
* Constructs a SimpleHostSet.
*
@@ -73,15 +84,29 @@ public final class StaticHostProvider implements HostProvider {
* if serverAddresses is empty or resolves to an empty list
*/
public StaticHostProvider(Collection<InetSocketAddress> serverAddresses) {
- sourceOfRandomness = new Random(System.currentTimeMillis() ^ this.hashCode());
+ init(serverAddresses,
+ System.currentTimeMillis() ^ this.hashCode(),
+ new Resolver() {
+ @Override
+ public InetAddress[] getAllByName(String name) throws UnknownHostException {
+ return InetAddress.getAllByName(name);
+ }
+ });
+ }
- this.serverAddresses = resolveAndShuffle(serverAddresses);
- if (this.serverAddresses.isEmpty()) {
- throw new IllegalArgumentException(
- "A HostProvider may not be empty!");
- }
- currentIndex = -1;
- lastIndex = -1;
+ /**
+ * Constructs a SimpleHostSet.
+ *
+ * Introduced for testing purposes. getAllByName() is a static method of InetAddress, therefore cannot be easily mocked.
+ * By abstraction of Resolver interface we can easily inject a mocked implementation in tests.
+ *
+ * @param serverAddresses
+ * possibly unresolved ZooKeeper server addresses
+ * @param resolver
+ * custom resolver implementation
+ */
+ public StaticHostProvider(Collection<InetSocketAddress> serverAddresses, Resolver resolver) {
+ init(serverAddresses, System.currentTimeMillis() ^ this.hashCode(), resolver);
}
/**
@@ -96,36 +121,47 @@ public final class StaticHostProvider implements HostProvider {
*/
public StaticHostProvider(Collection<InetSocketAddress> serverAddresses,
long randomnessSeed) {
- sourceOfRandomness = new Random(randomnessSeed);
+ init(serverAddresses, randomnessSeed, new Resolver() {
+ @Override
+ public InetAddress[] getAllByName(String name) throws UnknownHostException {
+ return InetAddress.getAllByName(name);
+ }
+ });
+ }
- this.serverAddresses = resolveAndShuffle(serverAddresses);
- if (this.serverAddresses.isEmpty()) {
+ private void init(Collection<InetSocketAddress> serverAddresses, long randomnessSeed, Resolver resolver) {
+ this.sourceOfRandomness = new Random(randomnessSeed);
+ this.resolver = resolver;
+ if (serverAddresses.isEmpty()) {
throw new IllegalArgumentException(
"A HostProvider may not be empty!");
- }
+ }
+ this.serverAddresses = shuffle(serverAddresses);
currentIndex = -1;
- lastIndex = -1;
+ lastIndex = -1;
}
- private List<InetSocketAddress> resolveAndShuffle(Collection<InetSocketAddress> serverAddresses) {
- List<InetSocketAddress> tmpList = new ArrayList<InetSocketAddress>(serverAddresses.size());
- for (InetSocketAddress address : serverAddresses) {
- try {
- InetAddress ia = address.getAddress();
- String addr = (ia != null) ? ia.getHostAddress() : address.getHostString();
- InetAddress resolvedAddresses[] = InetAddress.getAllByName(addr);
- for (InetAddress resolvedAddress : resolvedAddresses) {
- InetAddress taddr = InetAddress.getByAddress(address.getHostString(), resolvedAddress.getAddress());
- tmpList.add(new InetSocketAddress(taddr, address.getPort()));
- }
- } catch (UnknownHostException ex) {
- LOG.warn("No IP address found for server: {}", address, ex);
+ private InetSocketAddress resolve(InetSocketAddress address) {
+ try {
+ String curHostString = address.getHostString();
+ List<InetAddress> resolvedAddresses = new ArrayList<>(Arrays.asList(this.resolver.getAllByName(curHostString)));
+ if (resolvedAddresses.isEmpty()) {
+ return address;
}
+ Collections.shuffle(resolvedAddresses);
+ return new InetSocketAddress(resolvedAddresses.get(0), address.getPort());
+ } catch (UnknownHostException e) {
+ LOG.error("Unable to resolve address: {}", address.toString(), e);
+ return address;
}
+ }
+
+ private List<InetSocketAddress> shuffle(Collection<InetSocketAddress> serverAddresses) {
+ List<InetSocketAddress> tmpList = new ArrayList<>(serverAddresses.size());
+ tmpList.addAll(serverAddresses);
Collections.shuffle(tmpList, sourceOfRandomness);
return tmpList;
- }
-
+ }
/**
* Update the list of servers. This returns true if changing connections is necessary for load-balancing, false
@@ -149,15 +185,12 @@ public final class StaticHostProvider implements HostProvider {
* @param currentHost the host to which this client is currently connected
* @return true if changing connections is necessary for load-balancing, false otherwise
*/
-
-
@Override
public synchronized boolean updateServerList(
Collection<InetSocketAddress> serverAddresses,
InetSocketAddress currentHost) {
- // Resolve server addresses and shuffle them
- List<InetSocketAddress> resolvedList = resolveAndShuffle(serverAddresses);
- if (resolvedList.isEmpty()) {
+ List<InetSocketAddress> shuffledList = shuffle(serverAddresses);
+ if (shuffledList.isEmpty()) {
throw new IllegalArgumentException(
"A HostProvider may not be empty!");
}
@@ -183,7 +216,7 @@ public final class StaticHostProvider implements HostProvider {
}
}
- for (InetSocketAddress addr : resolvedList) {
+ for (InetSocketAddress addr : shuffledList) {
if (addr.getPort() == myServer.getPort()
&& ((addr.getAddress() != null
&& myServer.getAddress() != null && addr
@@ -200,11 +233,11 @@ public final class StaticHostProvider implements HostProvider {
oldServers.clear();
// Divide the new servers into oldServers that were in the previous list
// and newServers that were not in the previous list
- for (InetSocketAddress resolvedAddress : resolvedList) {
- if (this.serverAddresses.contains(resolvedAddress)) {
- oldServers.add(resolvedAddress);
+ for (InetSocketAddress address : shuffledList) {
+ if (this.serverAddresses.contains(address)) {
+ oldServers.add(address);
} else {
- newServers.add(resolvedAddress);
+ newServers.add(address);
}
}
@@ -245,11 +278,11 @@ public final class StaticHostProvider implements HostProvider {
}
if (!reconfigMode) {
- currentIndex = resolvedList.indexOf(getServerAtCurrentIndex());
+ currentIndex = shuffledList.indexOf(getServerAtCurrentIndex());
} else {
currentIndex = -1;
}
- this.serverAddresses = resolvedList;
+ this.serverAddresses = shuffledList;
currentIndexOld = -1;
currentIndexNew = -1;
lastIndex = currentIndex;
@@ -314,7 +347,7 @@ public final class StaticHostProvider implements HostProvider {
addr = nextHostInReconfigMode();
if (addr != null) {
currentIndex = serverAddresses.indexOf(addr);
- return addr;
+ return resolve(addr);
}
//tried all servers and couldn't connect
reconfigMode = false;
@@ -339,7 +372,7 @@ public final class StaticHostProvider implements HostProvider {
}
}
- return addr;
+ return resolve(addr);
}
public synchronized void onConnected() {
http://git-wip-us.apache.org/repos/asf/zookeeper/blob/0a311873/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
----------------------------------------------------------------------
diff --git a/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java b/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
index 10c6d1c..998d6e5 100644
--- a/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
+++ b/src/java/test/org/apache/zookeeper/test/StaticHostProviderTest.java
@@ -18,10 +18,6 @@
package org.apache.zookeeper.test;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertTrue;
-
import org.apache.zookeeper.ZKTestCase;
import org.apache.zookeeper.client.HostProvider;
import org.apache.zookeeper.client.StaticHostProvider;
@@ -32,10 +28,29 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.UnknownHostException;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
-import java.util.HashMap;
+import java.util.Collections;
+import java.util.List;
import java.util.Random;
+import static org.hamcrest.CoreMatchers.anyOf;
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.hasItems;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertSame;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.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 static org.mockito.Mockito.when;
+
public class StaticHostProviderTest extends ZKTestCase {
private Random r = new Random(1);
@@ -77,23 +92,28 @@ public class StaticHostProviderTest extends ZKTestCase {
}
@Test(expected = IllegalArgumentException.class)
- public void testTwoInvalidHostAddresses() {
- ArrayList<InetSocketAddress> list = new ArrayList<InetSocketAddress>();
- list.add(new InetSocketAddress("a...", 2181));
- list.add(new InetSocketAddress("b...", 2181));
- new StaticHostProvider(list);
+ public void testEmptyServerAddressesList() {
+ HostProvider hp = new StaticHostProvider(new ArrayList<>());
}
@Test
- public void testOneInvalidHostAddresses() {
- Collection<InetSocketAddress> addr = getServerAddresses((byte) 1);
- addr.add(new InetSocketAddress("a...", 2181));
+ public void testInvalidHostAddresses() {
+ // Arrange
+ final List<InetSocketAddress> invalidAddresses = new ArrayList<>();
+ InetSocketAddress unresolved = InetSocketAddress.createUnresolved("a", 1234);
+ invalidAddresses.add(unresolved);
+ StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() {
+ @Override
+ public InetAddress[] getAllByName(String name) throws UnknownHostException {
+ throw new UnknownHostException();
+ }
+ };
+ StaticHostProvider sp = new StaticHostProvider(invalidAddresses, resolver);
- StaticHostProvider sp = new StaticHostProvider(addr);
+ // Act & Assert
InetSocketAddress n1 = sp.next(0);
- InetSocketAddress n2 = sp.next(0);
-
- assertEquals(n2, n1);
+ assertTrue("Provider should return unresolved address is host is unresolvable", n1.isUnresolved());
+ assertSame("Provider should return original address is host is unresolvable", unresolved, n1);
}
@Test
@@ -111,6 +131,8 @@ public class StaticHostProviderTest extends ZKTestCase {
assertNotSame(first, second);
}
+ /* Reconfig tests with IP addresses */
+
private final double slackPercent = 10;
private final int numClients = 10000;
@@ -123,12 +145,12 @@ public class StaticHostProviderTest extends ZKTestCase {
// Number of machines becomes smaller, my server is in the new cluster
boolean disconnectRequired = hostProvider.updateServerList(newList, myServer);
- assertTrue(!disconnectRequired);
+ assertFalse(disconnectRequired);
hostProvider.onConnected();
// Number of machines stayed the same, my server is in the new cluster
disconnectRequired = hostProvider.updateServerList(newList, myServer);
- assertTrue(!disconnectRequired);
+ assertFalse(disconnectRequired);
hostProvider.onConnected();
// Number of machines became smaller, my server is not in the new
@@ -170,7 +192,7 @@ public class StaticHostProviderTest extends ZKTestCase {
}
hostProvider.onConnected();
- // should be numClients/10 in expectation, we test that its numClients/10 +- slackPercent
+ // should be numClients/10 in expectation, we test that its numClients/10 +- slackPercent
assertTrue(numDisconnects < upperboundCPS(numClients, 10));
}
@@ -178,8 +200,7 @@ public class StaticHostProviderTest extends ZKTestCase {
public void testUpdateMigrationGoesRound() throws UnknownHostException {
HostProvider hostProvider = getHostProvider((byte) 4);
// old list (just the ports): 1238, 1237, 1236, 1235
- Collection<InetSocketAddress> newList = new ArrayList<InetSocketAddress>(
- 10);
+ Collection<InetSocketAddress> newList = new ArrayList<InetSocketAddress>(10);
for (byte i = 12; i > 2; i--) { // 1246, 1245, 1244, 1243, 1242, 1241,
// 1240, 1239, 1238, 1237
newList.add(new InetSocketAddress(InetAddress.getByAddress(new byte[]{10, 10, 10, i}), 1234 + i));
@@ -460,10 +481,7 @@ public class StaticHostProviderTest extends ZKTestCase {
return new StaticHostProvider(getServerAddresses(size), r.nextLong());
}
- private HashMap<Byte, Collection<InetSocketAddress>> precomputedLists = new
- HashMap<Byte, Collection<InetSocketAddress>>();
private Collection<InetSocketAddress> getServerAddresses(byte size) {
- if (precomputedLists.containsKey(size)) return precomputedLists.get(size);
ArrayList<InetSocketAddress> list = new ArrayList<InetSocketAddress>(
size);
while (size > 0) {
@@ -475,10 +493,205 @@ public class StaticHostProviderTest extends ZKTestCase {
}
--size;
}
- precomputedLists.put(size, list);
return list;
}
+ /* Reconfig test with unresolved hostnames */
+
+ /**
+ * Number of machines becomes smaller, my server is in the new cluster
+ */
+ @Test
+ public void testUpdateServerList_UnresolvedHostnames_NoDisconnection1() {
+ // Arrange
+ // [testhost-4.testdomain.com:1238, testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(4);
+ // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ Collection<InetSocketAddress> newList = getUnresolvedHostnames(3);
+ InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-3.testdomain.com", 1237);
+
+ // Act
+ boolean disconnectRequired = hostProvider.updateServerList(newList, myServer);
+
+ // Assert
+ assertFalse(disconnectRequired);
+ hostProvider.onConnected();
+ }
+
+ /**
+ * Number of machines stayed the same, my server is in the new cluster
+ */
+ @Test
+ public void testUpdateServerList_UnresolvedHostnames_NoDisconnection2() {
+ // Arrange
+ // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(3);
+ // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ Collection<InetSocketAddress> newList = getUnresolvedHostnames(3);
+ InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-3.testdomain.com", 1237);
+
+ // Act
+ boolean disconnectRequired = hostProvider.updateServerList(newList, myServer);
+
+ // Assert
+ assertFalse(disconnectRequired);
+ hostProvider.onConnected();
+ }
+
+ /**
+ * Number of machines became smaller, my server is not in the new cluster
+ */
+ @Test
+ public void testUpdateServerList_UnresolvedHostnames_Disconnection1() {
+ // Arrange
+ // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(3);
+ // [testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ Collection<InetSocketAddress> newList = getUnresolvedHostnames(2);
+ InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-3.testdomain.com", 1237);
+
+ // Act
+ boolean disconnectRequired = hostProvider.updateServerList(newList, myServer);
+
+ // Assert
+ assertTrue(disconnectRequired);
+ hostProvider.onConnected();
+ }
+
+ /**
+ * Number of machines stayed the same, my server is not in the new cluster
+ */
+ @Test
+ public void testUpdateServerList_UnresolvedHostnames_Disconnection2() {
+ // Arrange
+ // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ HostProvider hostProvider = getHostProviderWithUnresolvedHostnames(3);
+ // [testhost-3.testdomain.com:1237, testhost-2.testdomain.com:1236, testhost-1.testdomain.com:1235]
+ Collection<InetSocketAddress> newList = getUnresolvedHostnames(3);
+ InetSocketAddress myServer = InetSocketAddress.createUnresolved("testhost-4.testdomain.com", 1237);
+
+ // Act
+ boolean disconnectRequired = hostProvider.updateServerList(newList, myServer);
+
+ // Assert
+ assertTrue(disconnectRequired);
+ hostProvider.onConnected();
+ }
+
+ @Test
+ public void testUpdateServerList_ResolvedWithUnResolvedAddress_ForceDisconnect() {
+ // Arrange
+ // Create a HostProvider with a list of unresolved server address(es)
+ List<InetSocketAddress> addresses = Collections.singletonList(
+ InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235)
+ );
+ HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver());
+ InetSocketAddress currentHost = hostProvider.next(100);
+ assertThat("CurrentHost is which the client is currently connecting to, it should be resolved",
+ currentHost.isUnresolved(), is(false));
+
+ // Act
+ InetSocketAddress replaceHost = InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235);
+ assertThat("Replace host must be unresolved in this test case", replaceHost.isUnresolved(), is(true));
+ boolean disconnect = hostProvider.updateServerList(new ArrayList<>(
+ Collections.singletonList(replaceHost)),
+ currentHost
+ );
+
+ // Assert
+ assertThat(disconnect, is(false));
+ }
+
+ @Test
+ public void testUpdateServerList_ResolvedWithResolvedAddress_NoDisconnect() throws UnknownHostException {
+ // Arrange
+ // Create a HostProvider with a list of unresolved server address(es)
+ List<InetSocketAddress> addresses = Collections.singletonList(
+ InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235)
+ );
+ HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver());
+ InetSocketAddress currentHost = hostProvider.next(100);
+ assertThat("CurrentHost is which the client is currently connecting to, it should be resolved",
+ currentHost.isUnresolved(), is(false));
+
+ // Act
+ InetSocketAddress replaceHost =
+ new InetSocketAddress(InetAddress.getByAddress(currentHost.getHostString(),
+ currentHost.getAddress().getAddress()), currentHost.getPort());
+ assertThat("Replace host must be resolved in this test case", replaceHost.isUnresolved(), is(false));
+ boolean disconnect = hostProvider.updateServerList(new ArrayList<>(
+ Collections.singletonList(replaceHost)),
+ currentHost
+ );
+
+ // Assert
+ assertThat(disconnect, equalTo(false));
+ }
+
+ @Test
+ public void testUpdateServerList_UnResolvedWithUnResolvedAddress_ForceDisconnect() {
+ // Arrange
+ // Create a HostProvider with a list of unresolved server address(es)
+ List<InetSocketAddress> addresses = Collections.singletonList(
+ InetSocketAddress.createUnresolved("testhost-1.zookeepertest.zk", 1235)
+ );
+ HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver());
+ InetSocketAddress currentHost = hostProvider.next(100);
+ assertThat("CurrentHost is not resolvable in this test case",
+ currentHost.isUnresolved(), is(true));
+
+ // Act
+ InetSocketAddress replaceHost = InetSocketAddress.createUnresolved("testhost-1.resolvable.zk", 1235);
+ assertThat("Replace host must be unresolved in this test case", replaceHost.isUnresolved(), is(true));
+ boolean disconnect = hostProvider.updateServerList(new ArrayList<>(
+ Collections.singletonList(replaceHost)),
+ currentHost
+ );
+
+ // Assert
+ assertThat(disconnect, is(true));
+ }
+
+ @Test
+ public void testUpdateServerList_UnResolvedWithResolvedAddress_ForceDisconnect() throws UnknownHostException {
+ // Arrange
+ // Create a HostProvider with a list of unresolved server address(es)
+ List<InetSocketAddress> addresses = Collections.singletonList(
+ InetSocketAddress.createUnresolved("testhost-1.zookeepertest.zk", 1235)
+ );
+ HostProvider hostProvider = new StaticHostProvider(addresses, new TestResolver());
+ InetSocketAddress currentHost = hostProvider.next(100);
+ assertThat("CurrentHost not resolvable in this test case",
+ currentHost.isUnresolved(), is(true));
+
+ // Act
+ byte[] addr = new byte[] { 10, 0, 0, 1 };
+ InetSocketAddress replaceHost =
+ new InetSocketAddress(InetAddress.getByAddress(currentHost.getHostString(), addr),
+ currentHost.getPort());
+ assertThat("Replace host must be resolved in this test case", replaceHost.isUnresolved(), is(false));
+ boolean disconnect = hostProvider.updateServerList(new ArrayList<>(
+ Collections.singletonList(replaceHost)),
+ currentHost
+ );
+
+ // Assert
+ assertThat(disconnect, equalTo(false));
+ }
+
+ private class TestResolver implements StaticHostProvider.Resolver {
+ private byte counter = 1;
+
+ @Override
+ public InetAddress[] getAllByName(String name) throws UnknownHostException {
+ if (name.contains("resolvable")) {
+ byte[] addr = new byte[] { 10, 0, 0, (byte)(counter++ % 10) };
+ return new InetAddress[] { InetAddress.getByAddress(name, addr) };
+ }
+ throw new UnknownHostException();
+ }
+ }
+
private double lowerboundCPS(int numClients, int numServers) {
return (1 - slackPercent/100.0) * numClients / numServers;
}
@@ -487,24 +700,210 @@ public class StaticHostProviderTest extends ZKTestCase {
return (1 + slackPercent/100.0) * numClients / numServers;
}
+ /* DNS resolution tests */
+
@Test
- public void testLiteralIPNoReverseNS() throws Exception {
+ public void testLiteralIPNoReverseNS() {
byte size = 30;
HostProvider hostProvider = getHostProviderUnresolved(size);
for (int i = 0; i < size; i++) {
InetSocketAddress next = hostProvider.next(0);
- assertTrue(next instanceof InetSocketAddress);
- assertTrue(!next.isUnresolved());
- assertTrue(!next.toString().startsWith("/"));
+ assertThat(next, instanceOf(InetSocketAddress.class));
+ assertFalse(next.isUnresolved());
+ assertTrue(next.toString().startsWith("/"));
// Do NOT trigger the reverse name service lookup.
String hostname = next.getHostString();
// In this case, the hostname equals literal IP address.
- hostname.equals(next.getAddress().getHostAddress());
+ assertEquals(next.getAddress().getHostAddress(), hostname);
+ }
+ }
+
+ @Test
+ public void testReResolvingSingle() throws UnknownHostException {
+ // Arrange
+ byte size = 1;
+ ArrayList<InetSocketAddress> list = new ArrayList<InetSocketAddress>(size);
+
+ // Test a hostname that resolves to a single address
+ list.add(InetSocketAddress.createUnresolved("issues.apache.org", 1234));
+
+ final InetAddress issuesApacheOrg = mock(InetAddress.class);
+ when(issuesApacheOrg.getHostAddress()).thenReturn("192.168.1.1");
+ when(issuesApacheOrg.toString()).thenReturn("issues.apache.org");
+ when(issuesApacheOrg.getHostName()).thenReturn("issues.apache.org");
+
+ StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() {
+ @Override
+ public InetAddress[] getAllByName(String name) {
+ return new InetAddress[] {
+ issuesApacheOrg
+ };
+ }
+ };
+ StaticHostProvider.Resolver spyResolver = spy(resolver);
+
+ // Act
+ StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver);
+ for (int i = 0; i < 10; i++) {
+ InetSocketAddress next = hostProvider.next(0);
+ assertEquals(issuesApacheOrg, next.getAddress());
+ }
+
+ // Assert
+ // Resolver called 10 times, because we shouldn't cache the resolved addresses
+ verify(spyResolver, times(10)).getAllByName("issues.apache.org"); // resolution occurred
+ }
+
+ @Test
+ public void testReResolvingMultiple() throws UnknownHostException {
+ // Arrange
+ byte size = 1;
+ ArrayList<InetSocketAddress> list = new ArrayList<InetSocketAddress>(size);
+
+ // Test a hostname that resolves to multiple addresses
+ list.add(InetSocketAddress.createUnresolved("www.apache.org", 1234));
+
+ final InetAddress apacheOrg1 = mock(InetAddress.class);
+ when(apacheOrg1.getHostAddress()).thenReturn("192.168.1.1");
+ when(apacheOrg1.toString()).thenReturn("www.apache.org");
+ when(apacheOrg1.getHostName()).thenReturn("www.apache.org");
+
+ final InetAddress apacheOrg2 = mock(InetAddress.class);
+ when(apacheOrg2.getHostAddress()).thenReturn("192.168.1.2");
+ when(apacheOrg2.toString()).thenReturn("www.apache.org");
+ when(apacheOrg2.getHostName()).thenReturn("www.apache.org");
+
+ final List<InetAddress> resolvedAddresses = new ArrayList<InetAddress>();
+ resolvedAddresses.add(apacheOrg1);
+ resolvedAddresses.add(apacheOrg2);
+ StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() {
+ @Override
+ public InetAddress[] getAllByName(String name) {
+ return resolvedAddresses.toArray(new InetAddress[resolvedAddresses.size()]);
+ }
+ };
+ StaticHostProvider.Resolver spyResolver = spy(resolver);
+
+ // Act & Assert
+ StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver);
+ assertEquals(1, hostProvider.size()); // single address not extracted
+
+ for (int i = 0; i < 10; i++) {
+ InetSocketAddress next = hostProvider.next(0);
+ assertThat("Bad IP address returned", next.getAddress().getHostAddress(), anyOf(equalTo(apacheOrg1.getHostAddress()), equalTo(apacheOrg2.getHostAddress())));
+ assertEquals(1, hostProvider.size()); // resolve() call keeps the size of provider
}
+ // Resolver called 10 times, because we shouldn't cache the resolved addresses
+ verify(spyResolver, times(10)).getAllByName("www.apache.org"); // resolution occurred
}
- private StaticHostProvider getHostProviderUnresolved(byte size)
- throws UnknownHostException {
+ @Test
+ public void testReResolveMultipleOneFailing() throws UnknownHostException {
+ // Arrange
+ final List<InetSocketAddress> list = new ArrayList<InetSocketAddress>();
+ list.add(InetSocketAddress.createUnresolved("www.apache.org", 1234));
+ final List<String> ipList = new ArrayList<String>();
+ final List<InetAddress> resolvedAddresses = new ArrayList<InetAddress>();
+ for (int i = 0; i < 3; i++) {
+ ipList.add(String.format("192.168.1.%d", i+1));
+ final InetAddress apacheOrg = mock(InetAddress.class);
+ when(apacheOrg.getHostAddress()).thenReturn(String.format("192.168.1.%d", i+1));
+ when(apacheOrg.toString()).thenReturn(String.format("192.168.1.%d", i+1));
+ when(apacheOrg.getHostName()).thenReturn("www.apache.org");
+ resolvedAddresses.add(apacheOrg);
+ }
+
+ StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() {
+ @Override
+ public InetAddress[] getAllByName(String name) {
+ return resolvedAddresses.toArray(new InetAddress[resolvedAddresses.size()]);
+ }
+ };
+ StaticHostProvider.Resolver spyResolver = spy(resolver);
+ StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver);
+
+ // Act & Assert
+ InetSocketAddress resolvedFirst = hostProvider.next(0);
+ assertFalse("HostProvider should return resolved addresses", resolvedFirst.isUnresolved());
+ assertThat("Bad IP address returned", ipList, hasItems(resolvedFirst.getAddress().getHostAddress()));
+
+ hostProvider.onConnected(); // first address worked
+
+ InetSocketAddress resolvedSecond = hostProvider.next(0);
+ assertFalse("HostProvider should return resolved addresses", resolvedSecond.isUnresolved());
+ assertThat("Bad IP address returned", ipList, hasItems(resolvedSecond.getAddress().getHostAddress()));
+
+ // Second address doesn't work, so we don't call onConnected() this time
+ // StaticHostProvider should try to re-resolve the address in this case
+
+ InetSocketAddress resolvedThird = hostProvider.next(0);
+ assertFalse("HostProvider should return resolved addresses", resolvedThird.isUnresolved());
+ assertThat("Bad IP address returned", ipList, hasItems(resolvedThird.getAddress().getHostAddress()));
+
+ verify(spyResolver, times(3)).getAllByName("www.apache.org"); // resolution occured every time
+ }
+
+ @Test
+ public void testEmptyResolution() throws UnknownHostException {
+ // Arrange
+ final List<InetSocketAddress> list = new ArrayList<InetSocketAddress>();
+ list.add(InetSocketAddress.createUnresolved("www.apache.org", 1234));
+ list.add(InetSocketAddress.createUnresolved("www.google.com", 1234));
+ final List<InetAddress> resolvedAddresses = new ArrayList<InetAddress>();
+
+ final InetAddress apacheOrg1 = mock(InetAddress.class);
+ when(apacheOrg1.getHostAddress()).thenReturn("192.168.1.1");
+ when(apacheOrg1.toString()).thenReturn("www.apache.org");
+ when(apacheOrg1.getHostName()).thenReturn("www.apache.org");
+
+ resolvedAddresses.add(apacheOrg1);
+
+ StaticHostProvider.Resolver resolver = new StaticHostProvider.Resolver() {
+ @Override
+ public InetAddress[] getAllByName(String name) {
+ if ("www.apache.org".equalsIgnoreCase(name)) {
+ return resolvedAddresses.toArray(new InetAddress[resolvedAddresses.size()]);
+ } else {
+ return new InetAddress[0];
+ }
+ }
+ };
+ StaticHostProvider.Resolver spyResolver = spy(resolver);
+ StaticHostProvider hostProvider = new StaticHostProvider(list, spyResolver);
+
+ // Act & Assert
+ for (int i = 0; i < 10; i++) {
+ InetSocketAddress resolved = hostProvider.next(0);
+ hostProvider.onConnected();
+ if (resolved.getHostName().equals("www.google.com")) {
+ assertTrue("HostProvider should return unresolved address if host is unresolvable", resolved.isUnresolved());
+ } else {
+ assertFalse("HostProvider should return resolved addresses", resolved.isUnresolved());
+ assertEquals("192.168.1.1", resolved.getAddress().getHostAddress());
+ }
+ }
+
+ verify(spyResolver, times(5)).getAllByName("www.apache.org");
+ verify(spyResolver, times(5)).getAllByName("www.google.com");
+ }
+
+ @Test
+ public void testReResolvingLocalhost() {
+ byte size = 2;
+ ArrayList<InetSocketAddress> list = new ArrayList<InetSocketAddress>(size);
+
+ // Test a hostname that resolves to multiple addresses
+ list.add(InetSocketAddress.createUnresolved("localhost", 1234));
+ list.add(InetSocketAddress.createUnresolved("localhost", 1235));
+ StaticHostProvider hostProvider = new StaticHostProvider(list);
+ int sizeBefore = hostProvider.size();
+ InetSocketAddress next = hostProvider.next(0);
+ next = hostProvider.next(0);
+ assertTrue("Different number of addresses in the list: " + hostProvider.size() +
+ " (after), " + sizeBefore + " (before)", hostProvider.size() == sizeBefore);
+ }
+
+ private StaticHostProvider getHostProviderUnresolved(byte size) {
return new StaticHostProvider(getUnresolvedServerAddresses(size), r.nextLong());
}
@@ -516,4 +915,18 @@ public class StaticHostProviderTest extends ZKTestCase {
}
return list;
}
+
+ private StaticHostProvider getHostProviderWithUnresolvedHostnames(int size) {
+ return new StaticHostProvider(getUnresolvedHostnames(size), r.nextLong());
+ }
+
+ private Collection<InetSocketAddress> getUnresolvedHostnames(int size) {
+ ArrayList<InetSocketAddress> list = new ArrayList<>(size);
+ while (size > 0) {
+ list.add(InetSocketAddress.createUnresolved(String.format("testhost-%d.testdomain.com", size), 1234 + size));
+ --size;
+ }
+ System.out.println(Arrays.toString(list.toArray()));
+ return list;
+ }
}