From af8e8dc2d6dcc0b9899e7f1236105e4b400f388d Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Thu, 19 Mar 2020 09:55:03 +0100 Subject: [PATCH] ZOOKEEPER-3758: Leader reachability check fails even with single leader Quorum address MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since ZooKeeper 3.6.0 we can specify multiple addresses for each ZooKeeper server instance (this can increase availability when multiple physical network interfaces can be used parallel in the cluster). ZooKeeper will perform ICMP ECHO requests or try to establish a TCP connection on port 7 (Echo) of the destination host in order to find the reachable addresses. This should happen only if the user provide multiple addresses in the configuration, in a single addess is used then ZooKeeper shouldn’t send any ICMP requests. This works as we expected for the leader election connections, but in this Jira issue we found a bug when the reachability check was performed even with a single address when the Follower tries to connect to the newly elected Leader. The fix follows the same approach we discussed for the election protocol before (see ZOOKEEPER-3698). We avoid the reachability check for single addresses. Also when we have multiple addresses and none of them can be reached, we still start to connect to all addresses in parallel. --- .../zookeeper/server/quorum/Learner.java | 4 ++- .../server/quorum/MultipleAddresses.java | 18 +++++++++++++ .../server/quorum/MultipleAddressesTest.java | 26 +++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java index 0f8f444a672..8a0cac17ad6 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java @@ -303,7 +303,9 @@ protected void connectToLeader(MultipleAddresses multiAddr, String hostname) thr this.leaderAddr = multiAddr; Set addresses; if (self.isMultiAddressReachabilityCheckEnabled()) { - addresses = multiAddr.getAllReachableAddresses(); + // even if none of the addresses are reachable, we want to try to establish connection + // see ZOOKEEPER-3758 + addresses = multiAddr.getAllReachableAddressesOrAll(); } else { addresses = multiAddr.getAllAddresses(); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/MultipleAddresses.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/MultipleAddresses.java index 3ee63fa9d6c..9f15389df62 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/MultipleAddresses.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/MultipleAddresses.java @@ -139,6 +139,24 @@ public Set getAllReachableAddresses() { .collect(Collectors.toSet()); } + /** + * Returns a set of all reachable addresses. If none is reachable than returns all addresses. + * + * @return all reachable addresses, or all addresses if none is reachable. + */ + public Set getAllReachableAddressesOrAll() { + // if there is only a single address provided then we don't need to do any reachability check + if (addresses.size() == 1) { + return getAllAddresses(); + } + + Set allReachable = getAllReachableAddresses(); + if (allReachable.isEmpty()) { + return getAllAddresses(); + } + return allReachable; + } + /** * Returns a reachable address or an arbitrary one, if none is reachable. It throws an exception * if there are no addresses registered. The function is nondeterministic in the sense that the diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/MultipleAddressesTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/MultipleAddressesTest.java index e8f08c6d90c..0e1615df9f5 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/MultipleAddressesTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/MultipleAddressesTest.java @@ -187,6 +187,32 @@ public void testGetAllReachableAddresses() throws Exception { Assert.assertEquals(reachableHosts, multipleAddresses.getAllReachableAddresses()); } + @Test + public void testGetAllReachableAddressesOrAllWhenSomeReachable() throws Exception { + InetSocketAddress reachableHost1 = new InetSocketAddress("127.0.0.1", 1234); + InetSocketAddress reachableHost2 = new InetSocketAddress("127.0.0.1", 2345); + InetSocketAddress unreachableHost1 = new InetSocketAddress("unreachable1.address.zookeeper.apache.com", 1234); + InetSocketAddress unreachableHost2 = new InetSocketAddress("unreachable2.address.zookeeper.apache.com", 1234); + + MultipleAddresses multipleAddresses = new MultipleAddresses( + Arrays.asList(unreachableHost1, unreachableHost2, reachableHost1, reachableHost2)); + + Set reachableHosts = new HashSet<>(Arrays.asList(reachableHost1, reachableHost2)); + Assert.assertEquals(reachableHosts, multipleAddresses.getAllReachableAddressesOrAll()); + } + + @Test + public void testGetAllReachableAddressesOrAllWhenNoneReachable() throws Exception { + InetSocketAddress unreachableHost1 = new InetSocketAddress("unreachable1.address.zookeeper.apache.com", 1234); + InetSocketAddress unreachableHost2 = new InetSocketAddress("unreachable2.address.zookeeper.apache.com", 1234); + InetSocketAddress unreachableHost3 = new InetSocketAddress("unreachable3.address.zookeeper.apache.com", 1234); + List allUnreachableAddresses = Arrays.asList(unreachableHost1, unreachableHost2, unreachableHost3); + + MultipleAddresses multipleAddresses = new MultipleAddresses(allUnreachableAddresses); + + Assert.assertEquals(new HashSet<>(allUnreachableAddresses), multipleAddresses.getAllReachableAddressesOrAll()); + } + @Test public void testEquals() { List addresses = getAddressList();