From 7bfbe7e5ca5225925e68ba8eee71fd985f4ecb11 Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Fri, 9 Aug 2019 10:29:55 +0200 Subject: [PATCH 01/14] ZOOKEEPER-3188: Improve resilience to network --- .../apache/zookeeper/server/ObserverBean.java | 6 +- .../zookeeper/server/admin/Commands.java | 38 ++- .../server/quorum/AuthFastLeaderElection.java | 14 +- .../zookeeper/server/quorum/Leader.java | 227 ++++++++----- .../zookeeper/server/quorum/Learner.java | 153 ++++++--- .../server/quorum/LocalPeerBean.java | 8 +- .../server/quorum/MultipleAddresses.java | 226 ++++++++++++ .../server/quorum/ObserverMaster.java | 11 +- .../server/quorum/QuorumCnxManager.java | 321 +++++++++++------- .../zookeeper/server/quorum/QuorumPeer.java | 221 ++++++------ .../server/quorum/QuorumZooKeeperServer.java | 8 +- .../quorum/ReadOnlyZooKeeperServer.java | 8 +- .../server/quorum/RemotePeerBean.java | 10 +- .../server/quorum/CnxManagerTest.java | 37 +- .../zookeeper/server/quorum/LearnerTest.java | 17 +- .../server/quorum/MultipleAddressesTest.java | 169 +++++++++ .../server/quorum/QuorumPeerMainTest.java | 1 - .../quorum/ReconfigFailureCasesTest.java | 4 +- .../org/apache/zookeeper/test/QuorumUtil.java | 2 +- .../zookeeper/test/ReconfigExceptionTest.java | 4 +- .../zookeeper/test/ReconfigMisconfigTest.java | 4 +- .../apache/zookeeper/test/ReconfigTest.java | 66 ++-- 22 files changed, 1090 insertions(+), 465 deletions(-) create mode 100644 zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/MultipleAddresses.java create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/MultipleAddressesTest.java diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ObserverBean.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ObserverBean.java index 167c96d27d8..379af7800aa 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/ObserverBean.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/ObserverBean.java @@ -21,6 +21,7 @@ import org.apache.zookeeper.server.quorum.Observer; import org.apache.zookeeper.server.quorum.ObserverMXBean; import org.apache.zookeeper.server.quorum.QuorumPeer; +import java.net.InetSocketAddress; /** * ObserverBean @@ -49,10 +50,11 @@ public String getQuorumAddress() { public String getLearnerMaster() { QuorumPeer.QuorumServer learnerMaster = observer.getCurrentLearnerMaster(); - if (learnerMaster == null || learnerMaster.addr == null) { + InetSocketAddress address = learnerMaster.addr.getReachableOrOne(); + if (learnerMaster == null || address == null) { return "Unknown"; } - return learnerMaster.addr.getAddress().getHostAddress() + ":" + learnerMaster.addr.getPort(); + return address.getAddress().getHostAddress() + ":" + address.getPort(); } public void setLearnerMaster(String learnerMaster) { diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java index 2e2ae37590a..1996807edd2 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java @@ -18,16 +18,8 @@ package org.apache.zookeeper.server.admin; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.List; -import java.util.Map; -import java.util.Properties; -import java.util.Set; -import java.util.SortedMap; -import java.util.TreeMap; +import java.net.InetSocketAddress; +import java.util.*; import java.util.stream.Collectors; import com.fasterxml.jackson.annotation.JsonAnyGetter; @@ -644,10 +636,8 @@ private static class VotingView { this.view = view.entrySet().stream() .filter(e -> e.getValue().addr != null) .collect(Collectors.toMap(Map.Entry::getKey, - e -> String.format("%s:%d%s:%s%s", - QuorumPeer.QuorumServer.delimitedHostString(e.getValue().addr), - e.getValue().addr.getPort(), - e.getValue().electionAddr == null ? "" : ":" + e.getValue().electionAddr.getPort(), + e -> String.format("%s:%s%s", + getMultiAddressString(e.getValue()), e.getValue().type.equals(QuorumPeer.LearnerType.PARTICIPANT) ? "participant" : "observer", e.getValue().clientAddr ==null || e.getValue().isClientAddrFromStatic ? "" : String.format(";%s:%d", @@ -657,11 +647,31 @@ private static class VotingView { TreeMap::new)); } + private String getMultiAddressString(QuorumPeer.QuorumServer qs) { + return qs.addr.getAllAddresses().stream() + .map(address -> getSingleAddressString(qs, address)) + .collect(Collectors.joining(",")); + } + + private String getSingleAddressString(QuorumPeer.QuorumServer qs, InetSocketAddress address) { + final String addressHostString = address.getHostString(); + final String delimitedHostString = QuorumPeer.QuorumServer.delimitedHostString(address); + + Optional matchingElectionAddress = qs.electionAddr.getAllAddresses().stream() + .filter(electionAddress -> electionAddress.getHostString().equals(addressHostString)) + .findFirst(); + final String electionPort = matchingElectionAddress.map(e-> ":" + e.getPort()).orElse(""); + + return String.format("%s:%d%s", delimitedHostString, address.getPort(), electionPort); + } + @JsonAnyGetter public Map getView() { return view; } } + + } /** diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java index 933cbfd486c..a3f9aa74b65 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.net.DatagramPacket; import java.net.DatagramSocket; +import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.SocketException; import java.nio.ByteBuffer; @@ -44,8 +45,6 @@ import org.apache.zookeeper.jmx.MBeanRegistry; import org.apache.zookeeper.server.ZooKeeperThread; -import org.apache.zookeeper.server.quorum.Election; -import org.apache.zookeeper.server.quorum.Vote; import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer; import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState; @@ -732,8 +731,8 @@ private void process(ToSend m) { } for (QuorumServer server : self.getVotingView().values()) { - InetSocketAddress saddr = new InetSocketAddress(server.addr - .getAddress(), port); + InetAddress address = server.addr.getReachableOrOne().getAddress(); + InetSocketAddress saddr = new InetSocketAddress(address, port); addrChallengeMap.put(saddr, new ConcurrentHashMap()); } @@ -763,7 +762,7 @@ public AuthFastLeaderElection(QuorumPeer self) { private void starter(QuorumPeer self) { this.self = self; - port = self.getVotingView().get(self.getId()).electionAddr.getPort(); + port = self.getVotingView().get(self.getId()).electionAddr.getAllPorts().get(0); proposedLeader = -1; proposedZxid = -1; @@ -786,11 +785,10 @@ private void leaveInstance() { private void sendNotifications() { for (QuorumServer server : self.getView().values()) { - + InetSocketAddress address = self.getView().get(server.id).electionAddr.getReachableOrOne(); ToSend notmsg = new ToSend(ToSend.mType.notification, AuthFastLeaderElection.sequencer++, proposedLeader, - proposedZxid, logicalclock.get(), QuorumPeer.ServerState.LOOKING, - self.getView().get(server.id).electionAddr); + proposedZxid, logicalclock.get(), QuorumPeer.ServerState.LOOKING, address); sendqueue.offer(notmsg); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java index f9be136cb0b..b5b1a507300 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java @@ -25,6 +25,7 @@ import java.io.DataOutputStream; import java.io.IOException; import java.net.BindException; +import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; import java.net.SocketAddress; @@ -35,20 +36,26 @@ import java.util.HashMap; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedList; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Set; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.stream.Collectors; import javax.security.sasl.SaslException; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.ZooDefs.OpCode; import org.apache.zookeeper.common.Time; -import org.apache.zookeeper.common.X509Exception; import org.apache.zookeeper.jmx.MBeanRegistry; import org.apache.zookeeper.server.FinalRequestProcessor; import org.apache.zookeeper.server.Request; @@ -271,39 +278,42 @@ public boolean isQuorumSynced(QuorumVerifier qv) { return qv.containsQuorum(ids); } - private final ServerSocket ss; + private final List serverSockets = new LinkedList<>(); Leader(QuorumPeer self,LeaderZooKeeperServer zk) throws IOException { this.self = self; this.proposalStats = new BufferStats(); + + Set addresses; + if (self.getQuorumListenOnAllIPs()) { + addresses = self.getQuorumAddress().getWildcardAddresses(); + } else { + addresses = self.getQuorumAddress().getAllAddresses(); + } + + for (InetSocketAddress address : addresses) { + serverSockets.add(createServerSocket(address, self.shouldUsePortUnification(), self.isSslQuorum())); + } + + this.zk = zk; + } + + ServerSocket createServerSocket(InetSocketAddress address, boolean portUnification, boolean sslQuorum) + throws IOException { + ServerSocket serverSocket; try { - if (self.shouldUsePortUnification() || self.isSslQuorum()) { - boolean allowInsecureConnection = self.shouldUsePortUnification(); - if (self.getQuorumListenOnAllIPs()) { - ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection, self.getQuorumAddress().getPort()); - } else { - ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection); - } + if (portUnification || sslQuorum) { + serverSocket = new UnifiedServerSocket(self.getX509Util(), portUnification); } else { - if (self.getQuorumListenOnAllIPs()) { - ss = new ServerSocket(self.getQuorumAddress().getPort()); - } else { - ss = new ServerSocket(); - } - } - ss.setReuseAddress(true); - if (!self.getQuorumListenOnAllIPs()) { - ss.bind(self.getQuorumAddress()); + serverSocket = new ServerSocket(); } + serverSocket.setReuseAddress(true); + serverSocket.bind(address); + return serverSocket; } catch (BindException e) { - if (self.getQuorumListenOnAllIPs()) { - LOG.error("Couldn't bind to port " + self.getQuorumAddress().getPort(), e); - } else { - LOG.error("Couldn't bind to " + self.getQuorumAddress(), e); - } + LOG.error("Couldn't bind to " + self.getQuorumAddress(), e); throw e; } - this.zk = zk; } /** @@ -417,71 +427,111 @@ public boolean isQuorumSynced(QuorumVerifier qv) { protected final Proposal newLeaderProposal = new Proposal(); class LearnerCnxAcceptor extends ZooKeeperCriticalThread { - private volatile boolean stop = false; + private final AtomicBoolean stop = new AtomicBoolean(false); + private final AtomicBoolean fail = new AtomicBoolean(false); - public LearnerCnxAcceptor() { - super("LearnerCnxAcceptor-" + ss.getLocalSocketAddress(), zk - .getZooKeeperServerListener()); + LearnerCnxAcceptor() { + super("LearnerCnxAcceptor-" + serverSockets.stream() + .map(ServerSocket::getLocalSocketAddress) + .map(Objects::toString) + .collect(Collectors.joining(",")), + zk.getZooKeeperServerListener()); } @Override public void run() { - try { - while (!stop) { - Socket s = null; - boolean error = false; - try { - s = ss.accept(); - - // start with the initLimit, once the ack is processed - // in LearnerHandler switch to the syncLimit - s.setSoTimeout(self.tickTime * self.initLimit); - s.setTcpNoDelay(nodelay); - - BufferedInputStream is = new BufferedInputStream( - s.getInputStream()); - LearnerHandler fh = new LearnerHandler(s, is, - Leader.this); - fh.start(); - } catch (SocketException e) { - error = true; - if (stop) { - LOG.info("exception while shutting down acceptor: " - + e); - - // When Leader.shutdown() calls ss.close(), - // the call to accept throws an exception. - // We catch and set stop to true. - stop = true; - } else { - throw e; - } - } catch (SaslException e){ - LOG.error("Exception while connecting to quorum learner", e); - error = true; - } catch (Exception e) { - error = true; + if (!stop.get() && !serverSockets.isEmpty()) { + ExecutorService executor = Executors.newFixedThreadPool(serverSockets.size()); + CountDownLatch latch = new CountDownLatch(serverSockets.size()); + + serverSockets.forEach(serverSocket -> + executor.submit(new LearnerCnxAcceptorHandler(serverSocket, latch))); + + try { + latch.await(); + } catch (InterruptedException ie) { + LOG.error("Interrupted while sleeping. " + + "Ignoring exception", ie); + } finally { + closeSockets(); + } + } + } + + public void halt() { + stop.set(true); + closeSockets(); + } + + class LearnerCnxAcceptorHandler implements Runnable { + private ServerSocket serverSocket; + private CountDownLatch latch; + + LearnerCnxAcceptorHandler(ServerSocket serverSocket, CountDownLatch latch) { + this.serverSocket = serverSocket; + this.latch = latch; + } + + @Override + public void run() { + try { + Thread.currentThread().setName("LearnerCnxAcceptorHandler-" + serverSocket.getLocalSocketAddress()); + + while (!stop.get()) { + acceptConnections(); + } + } catch (Exception e) { + LOG.warn("Exception while accepting follower", e); + if (fail.compareAndSet(false, true)) { + handleException(getName(), e); + halt(); + } + } finally { + latch.countDown(); + } + } + + private void acceptConnections() throws IOException { + Socket socket = null; + boolean error = false; + try { + socket = serverSocket.accept(); + + // start with the initLimit, once the ack is processed + // in LearnerHandler switch to the syncLimit + socket.setSoTimeout(self.tickTime * self.initLimit); + socket.setTcpNoDelay(nodelay); + + BufferedInputStream is = new BufferedInputStream(socket.getInputStream()); + LearnerHandler fh = new LearnerHandler(socket, is, Leader.this); + fh.start(); + } catch (SocketException e) { + error = true; + if (stop.get()) { + LOG.info("Exception while shutting down acceptor", e); + } else { throw e; - } finally { - // Don't leak sockets on errors - if (error && s != null && !s.isClosed()) { - try { - s.close(); - } catch (IOException e) { - LOG.warn("Error closing socket", e); - } + } + } catch (SaslException e) { + LOG.error("Exception while connecting to quorum learner", e); + error = true; + } catch (Exception e) { + error = true; + throw e; + } finally { + // Don't leak sockets on errors + if (error && socket != null && !socket.isClosed()) { + try { + socket.close(); + } catch (IOException e) { + LOG.warn("Error closing socket", e); } } } - } catch (Exception e) { - LOG.warn("Exception while accepting follower", e.getMessage()); - handleException(this.getName(), e); } - } - public void halt() { - stop = true; } + } StateSummary leaderStateSummary; @@ -594,8 +644,8 @@ void lead() throws IOException, InterruptedException { waitForEpochAck(self.getId(), leaderStateSummary); self.setCurrentEpoch(epoch); - self.setLeaderAddressAndId(self.getQuorumAddress(), self.getId()); - self.setZabState(QuorumPeer.ZabState.SYNCHRONIZATION); + self.setLeaderAddressAndId(self.getQuorumAddress(), self.getId()); + self.setZabState(QuorumPeer.ZabState.SYNCHRONIZATION); try { waitForNewLeaderAck(self.getId(), zk.getZxid()); @@ -743,16 +793,13 @@ void shutdown(String reason) { if (cnxAcceptor != null) { cnxAcceptor.halt(); + } else { + closeSockets(); } // NIO should not accept conenctions self.setZooKeeperServer(null); self.adminServer.setZooKeeperServer(null); - try { - ss.close(); - } catch (IOException e) { - LOG.warn("Ignoring unexpected exception during close",e); - } self.closeAllConnections(); // shutdown the previous zk if (zk != null) { @@ -769,6 +816,18 @@ void shutdown(String reason) { isShutdown = true; } + synchronized void closeSockets() { + for (ServerSocket serverSocket : serverSockets) { + if (!serverSocket.isClosed()) { + try { + serverSocket.close(); + } catch (IOException e) { + LOG.warn("Ignoring unexpected exception during close {}", serverSocket, e); + } + } + } + } + /** In a reconfig operation, this method attempts to find the best leader for next configuration. * If the current leader is a voter in the next configuartion, then it remains the leader. * Otherwise, choose one of the new voters that acked the reconfiguartion, such that it is as 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 63a5454bc1c..f7f3e9e9320 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 @@ -24,7 +24,6 @@ import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; -import java.net.ConnectException; import java.net.InetSocketAddress; import java.net.Socket; import java.nio.ByteBuffer; @@ -32,7 +31,12 @@ import java.util.Deque; import java.util.Map; import java.util.Map.Entry; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.atomic.AtomicReference; import org.apache.jute.BinaryInputArchive; import org.apache.jute.BinaryOutputArchive; @@ -241,7 +245,7 @@ protected void sockConnect(Socket sock, InetSocketAddress addr, int timeout) throws IOException { sock.connect(addr, timeout); } - + /** * Establish a connection with the LearnerMaster found by findLearnerMaster. * Followers only connect to Leaders, Observers can connect to any active LearnerMaster. @@ -252,58 +256,21 @@ protected void sockConnect(Socket sock, InetSocketAddress addr, int timeout) * @throws X509Exception * @throws InterruptedException */ - protected void connectToLeader(InetSocketAddress addr, String hostname) - throws IOException, InterruptedException, X509Exception { - this.sock = createSocket(); + protected void connectToLeader(MultipleAddresses addr, String hostname) + throws IOException, InterruptedException { - // leader connection timeout defaults to tickTime * initLimit - int connectTimeout = self.tickTime * self.initLimit; + Set addresses = addr.getAllAddresses(); + ExecutorService executor = Executors.newFixedThreadPool(addresses.size()); + CountDownLatch latch = new CountDownLatch(addresses.size()); + AtomicReference socket = new AtomicReference<>(null); + addresses.stream().map(address -> new LeaderConnector(address, socket, latch)).forEach(executor::submit); - // but if connectToLearnerMasterLimit is specified, use that value to calculate - // timeout instead of using the initLimit value - if (self.connectToLearnerMasterLimit > 0) { - connectTimeout = self.tickTime * self.connectToLearnerMasterLimit; - } + latch.await(); - int remainingTimeout; - long startNanoTime = nanoTime(); - - for (int tries = 0; tries < 5; tries++) { - try { - // recalculate the init limit time because retries sleep for 1000 milliseconds - remainingTimeout = connectTimeout - (int)((nanoTime() - startNanoTime) / 1000000); - if (remainingTimeout <= 0) { - LOG.error("connectToLeader exceeded on retries."); - throw new IOException("connectToLeader exceeded on retries."); - } - - sockConnect(sock, addr, Math.min(connectTimeout, remainingTimeout)); - if (self.isSslQuorum()) { - ((SSLSocket) sock).startHandshake(); - } - sock.setTcpNoDelay(nodelay); - break; - } catch (IOException e) { - remainingTimeout = connectTimeout - (int)((nanoTime() - startNanoTime) / 1000000); - - if (remainingTimeout <= 1000) { - LOG.error("Unexpected exception, connectToLeader exceeded. tries=" + tries + - ", remaining init limit=" + remainingTimeout + - ", connecting to " + addr,e); - throw e; - } else if (tries >= 4) { - LOG.error("Unexpected exception, retries exceeded. tries=" + tries + - ", remaining init limit=" + remainingTimeout + - ", connecting to " + addr,e); - throw e; - } else { - LOG.warn("Unexpected exception, tries=" + tries + - ", remaining init limit=" + remainingTimeout + - ", connecting to " + addr,e); - this.sock = createSocket(); - } - } - Thread.sleep(leaderConnectDelayDuringRetryMs); + if (socket.get() == null) { + throw new IOException("Failed connect to " + addr); + } else { + sock = socket.get(); } self.authLearner.authenticate(sock, hostname); @@ -314,6 +281,90 @@ protected void connectToLeader(InetSocketAddress addr, String hostname) leaderOs = BinaryOutputArchive.getArchive(bufferedOutput); } + class LeaderConnector implements Runnable { + + private AtomicReference socket; + private InetSocketAddress address; + private CountDownLatch latch; + + LeaderConnector(InetSocketAddress address, AtomicReference socket, CountDownLatch latch) { + this.address = address; + this.socket = socket; + this.latch = latch; + } + + @Override + public void run() { + try { + Thread.currentThread().setName("LeaderConnector-" + address); + Socket sock = connectToLeader(); + + if (sock != null && sock.isConnected() && !socket.compareAndSet(null, sock)) { + LOG.info("Connection to the leader is already established, close the redundant connection"); + sock.close(); + } + + } catch (Exception e) { + LOG.error("Failed connect to {}", address, e); + } finally { + latch.countDown(); + } + } + + private Socket connectToLeader() throws IOException, X509Exception, InterruptedException { + Socket sock = createSocket(); + + // leader connection timeout defaults to tickTime * initLimit + int connectTimeout = self.tickTime * self.initLimit; + + // but if connectToLearnerMasterLimit is specified, use that value to calculate + // timeout instead of using the initLimit value + if (self.connectToLearnerMasterLimit > 0) { + connectTimeout = self.tickTime * self.connectToLearnerMasterLimit; + } + + int remainingTimeout; + long startNanoTime = nanoTime(); + + for (int tries = 0; tries < 5 && socket.get() == null; tries++) { + try { + // recalculate the init limit time because retries sleep for 1000 milliseconds + remainingTimeout = connectTimeout - (int) ((nanoTime() - startNanoTime) / 1_000_000); + if (remainingTimeout <= 0) { + LOG.error("connectToLeader exceeded on retries."); + throw new IOException("connectToLeader exceeded on retries."); + } + + sockConnect(sock, address, Math.min(connectTimeout, remainingTimeout)); + if (self.isSslQuorum()) { + ((SSLSocket) sock).startHandshake(); + } + sock.setTcpNoDelay(nodelay); + break; + } catch (IOException e) { + remainingTimeout = connectTimeout - (int) ((nanoTime() - startNanoTime) / 1_000_000); + + if (remainingTimeout <= leaderConnectDelayDuringRetryMs) { + LOG.error("Unexpected exception, connectToLeader exceeded. tries={}, remaining init limit={}, " + + "connecting to {}", tries, remainingTimeout, address, e); + throw e; + } else if (tries >= 4) { + LOG.error("Unexpected exception, retries exceeded. tries={}, remaining init limit={}, " + + "connecting to {}", tries, remainingTimeout, address, e); + throw e; + } else { + LOG.warn("Unexpected exception, tries={}, remaining init limit={}, connecting to {}", tries, + remainingTimeout, address, e); + sock = createSocket(); + } + } + Thread.sleep(leaderConnectDelayDuringRetryMs); + } + + return sock; + } + } + private Socket createSocket() throws X509Exception, IOException { Socket sock; if (self.isSslQuorum()) { diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java index 170d712ee1e..4ea99cef59e 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java @@ -18,6 +18,8 @@ package org.apache.zookeeper.server.quorum; +import org.apache.zookeeper.common.NetUtils; +import java.util.stream.Collectors; import static org.apache.zookeeper.common.NetUtils.formatInetAddr; /** @@ -79,7 +81,8 @@ public String getState() { } public String getQuorumAddress() { - return formatInetAddr(peer.getQuorumAddress()); + return peer.getQuorumAddress().getAllAddresses().stream().map(NetUtils::formatInetAddr) + .collect(Collectors.joining(",")); } public int getElectionType() { @@ -87,7 +90,8 @@ public int getElectionType() { } public String getElectionAddress() { - return formatInetAddr(peer.getElectionAddress()); + return peer.getElectionAddress().getAllAddresses().stream().map(NetUtils::formatInetAddr) + .collect(Collectors.joining(",")); } public String getClientAddress() { 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 new file mode 100644 index 00000000000..b890420c3da --- /dev/null +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/MultipleAddresses.java @@ -0,0 +1,226 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.server.quorum; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.NoRouteToHostException; +import java.net.UnknownHostException; +import java.util.Collections; +import java.util.List; +import java.util.Objects; +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import java.util.stream.Stream; + +/** + * This class allows to store several quorum and electing addresses. + * + * See ZOOKEEPER-3188 for a discussion of this feature. + */ +public class MultipleAddresses { + private final static int DEFAULT_TIMEOUT = 100; + + private Set addresses; + private int timeout; + + public MultipleAddresses() { + addresses = Collections.newSetFromMap(new ConcurrentHashMap<>()); + timeout = DEFAULT_TIMEOUT; + } + + public MultipleAddresses(List addresses) { + this(addresses, DEFAULT_TIMEOUT); + } + + public MultipleAddresses(InetSocketAddress address) { + this(address, DEFAULT_TIMEOUT); + } + + public MultipleAddresses(List addresses, int timeout) { + this.addresses = Collections.newSetFromMap(new ConcurrentHashMap<>()); + this.addresses.addAll(addresses); + this.timeout = timeout; + } + + public MultipleAddresses(InetSocketAddress address, int timeout) { + addresses = Collections.newSetFromMap(new ConcurrentHashMap<>()); + addresses.add(address); + this.timeout = timeout; + } + + public int getTimeout() { + return timeout; + } + + public void setTimeout(int timeout) { + this.timeout = timeout; + } + + public boolean isEmpty() { + return addresses.isEmpty(); + } + + /** + * Returns all addresses. + * + * @return set of all InetSocketAddress + */ + public Set getAllAddresses() { + return Collections.unmodifiableSet(addresses); + } + + /** + * Returns wildcard addresses for all ports + * + * @return set of InetSocketAddress with wildcards for all ports + */ + public Set getWildcardAddresses() { + return addresses.stream().map(a -> new InetSocketAddress(a.getPort())).collect(Collectors.toSet()); + } + + /** + * Returns all ports + * + * @return list of all ports + */ + public List getAllPorts() { + return addresses.stream().map(InetSocketAddress::getPort).distinct().collect(Collectors.toList()); + } + + /** + * Returns distinct list of all host strings + * + * @return list of all hosts + */ + public List getAllHostStrings() { + return addresses.stream().map(InetSocketAddress::getHostString).distinct().collect(Collectors.toList()); + } + + public void addAddress(InetSocketAddress address) { + addresses.add(address); + } + + /** + * Returns reachable address. If none is reachable than throws exception. + * + * @return address which is reachable. + * @throws NoRouteToHostException if none address is reachable + */ + public InetSocketAddress getReachableAddress() throws NoRouteToHostException { + AtomicReference address = new AtomicReference<>(null); + getInetSocketAddressStream().forEach(addr -> checkIfAddressIsReachableAndSet(addr, address)); + + if (address.get() != null) { + return address.get(); + } else { + throw new NoRouteToHostException("No valid address among " + addresses); + } + } + + /** + * Returns reachable address or first one, if none is reachable. + * + * @return address which is reachable or fist one. + */ + public InetSocketAddress getReachableOrOne() { + InetSocketAddress address; + try { + address = getReachableAddress(); + } catch (NoRouteToHostException e) { + address = getOne(); + } + return address; + } + + /** + * Performs a DNS lookup for addresses. + * + * If the DNS lookup fails, than address remain unmodified. + */ + public void recreateSocketAddresses() { + Set temp = Collections.newSetFromMap(new ConcurrentHashMap<>()); + temp.addAll(getInetSocketAddressStream().map(this::recreateSocketAddress).collect(Collectors.toSet())); + addresses = temp; + } + + /** + * Returns first address from set. + * + * @return address from a set. + */ + public InetSocketAddress getOne() { + return addresses.iterator().next(); + } + + private void checkIfAddressIsReachableAndSet(InetSocketAddress address, + AtomicReference reachableAddress) { + for (int i = 0; i < 5 && reachableAddress.get() == null; i++) { + try { + if (address.getAddress().isReachable((i + 1) * timeout)) { + reachableAddress.compareAndSet(null, address); + break; + } + Thread.sleep(timeout); + } catch (NullPointerException | IOException | InterruptedException ignored) { + } + } + } + + private InetSocketAddress recreateSocketAddress(InetSocketAddress address) { + try { + return new InetSocketAddress(InetAddress.getByName(address.getHostString()), address.getPort()); + } catch (UnknownHostException e) { + return address; + } + } + + private Stream getInetSocketAddressStream() { + if (addresses.size() > 1) { + return addresses.parallelStream(); + } else { + return addresses.stream(); + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } else if (o == null || getClass() != o.getClass()) { + return false; + } + + MultipleAddresses that = (MultipleAddresses) o; + return Objects.equals(addresses, that.addresses); + } + + @Override + public int hashCode() { + return Objects.hash(addresses); + } + + @Override + public String toString() { + return addresses.stream().map(InetSocketAddress::toString).collect(Collectors.joining(",")); + } +} \ No newline at end of file diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java index 98f1acd4cd9..79e911a50a3 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java @@ -26,6 +26,7 @@ import java.io.ByteArrayInputStream; import java.io.DataInputStream; import java.io.IOException; +import java.net.InetAddress; import java.net.ServerSocket; import java.net.Socket; import java.net.SocketAddress; @@ -415,23 +416,19 @@ synchronized public void start() throws IOException { } listenerRunning = true; int backlog = 10; // dog science + InetAddress address = self.getQuorumAddress().getReachableOrOne().getAddress(); if (self.shouldUsePortUnification() || self.isSslQuorum()) { boolean allowInsecureConnection = self.shouldUsePortUnification(); if (self.getQuorumListenOnAllIPs()) { ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection, port, backlog); } else { - ss = new UnifiedServerSocket( - self.getX509Util(), - allowInsecureConnection, - port, - backlog, - self.getQuorumAddress().getAddress()); + ss = new UnifiedServerSocket(self.getX509Util(), allowInsecureConnection, port, backlog, address); } } else { if (self.getQuorumListenOnAllIPs()) { ss = new ServerSocket(port, backlog); } else { - ss = new ServerSocket(port, backlog, self.getQuorumAddress().getAddress()); + ss = new ServerSocket(port, backlog, address); } } thread = new Thread(this, "ObserverMaster"); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java index 5039d83cde1..9b9a6f6dec5 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java @@ -22,10 +22,10 @@ import java.io.BufferedInputStream; import java.io.BufferedOutputStream; +import java.io.Closeable; import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; -import java.net.BindException; import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; @@ -37,29 +37,38 @@ import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.NoSuchElementException; import java.util.Set; import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; import java.util.concurrent.SynchronousQueue; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import javax.net.ssl.SSLSocket; +import java.util.stream.Collectors; + +import org.apache.zookeeper.common.NetUtils; import org.apache.zookeeper.common.X509Exception; import org.apache.zookeeper.server.ExitCode; -import org.apache.zookeeper.server.ZooKeeperThread; import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException; +import org.apache.zookeeper.server.util.ConfigUtils; +import org.apache.zookeeper.server.ZooKeeperThread; import org.apache.zookeeper.server.quorum.auth.QuorumAuthLearner; import org.apache.zookeeper.server.quorum.auth.QuorumAuthServer; import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier; -import org.apache.zookeeper.server.util.ConfigUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; + /** * This class implements a connection manager for leader election using TCP. It * maintains one connection for every pair of servers. The tricky part is to @@ -126,7 +135,7 @@ public class QuorumCnxManager { final boolean listenOnAllIPs; private ThreadPoolExecutor connectionExecutor; private final Set inprogressConnections = Collections - .synchronizedSet(new HashSet()); + .synchronizedSet(new HashSet<>()); private QuorumAuthServer authServer; private QuorumAuthLearner authLearner; private boolean quorumSaslAuthEnabled; @@ -262,10 +271,10 @@ public QuorumCnxManager(QuorumPeer self, boolean listenOnAllIPs, int quorumCnxnThreadsSize, boolean quorumSaslAuthEnabled) { - this.recvQueue = new ArrayBlockingQueue(RECV_CAPACITY); - this.queueSendMap = new ConcurrentHashMap>(); - this.senderWorkerMap = new ConcurrentHashMap(); - this.lastMessageSent = new ConcurrentHashMap(); + this.recvQueue = new ArrayBlockingQueue<>(RECV_CAPACITY); + this.queueSendMap = new ConcurrentHashMap<>(); + this.senderWorkerMap = new ConcurrentHashMap<>(); + this.lastMessageSent = new ConcurrentHashMap<>(); String cnxToValue = System.getProperty("zookeeper.cnxTimeout"); if(cnxToValue != null){ @@ -331,7 +340,8 @@ public void testInitiateConnection(long sid) throws Exception { LOG.debug("Opening channel to server {}", sid); Socket sock = new Socket(); setSockOpts(sock); - sock.connect(self.getVotingView().get(sid).electionAddr, cnxTO); + InetSocketAddress address = self.getVotingView().get(sid).electionAddr.getReachableOrOne(); + sock.connect(address, cnxTO); initiateConnection(sock, sid); } @@ -414,7 +424,8 @@ private boolean startConnection(Socket sock, Long sid) // represents protocol version (in other words - message type) dout.writeLong(PROTOCOL_VERSION); dout.writeLong(self.getId()); - String addr = formatInetAddr(self.getElectionAddress()); + InetSocketAddress address = self.getElectionAddress().getReachableOrOne(); + String addr = formatInetAddr(address); byte[] addr_bytes = addr.getBytes(); dout.writeInt(addr_bytes.length); dout.write(addr_bytes); @@ -574,7 +585,7 @@ private void handleConnection(Socket sock, DataInputStream din) closeSocket(sock); if (electionAddr != null) { - connectOne(sid, electionAddr); + connectOne(sid, new MultipleAddresses(electionAddr)); } else { connectOne(sid); } @@ -634,7 +645,7 @@ public void toSend(Long sid, ByteBuffer b) { * @param sid server id * @return boolean success indication */ - synchronized boolean connectOne(long sid, InetSocketAddress electionAddr){ + synchronized boolean connectOne(long sid, MultipleAddresses electionAddr){ if (senderWorkerMap.get(sid) != null) { LOG.debug("There is a connection already for server {}", sid); return true; @@ -649,7 +660,7 @@ synchronized boolean connectOne(long sid, InetSocketAddress electionAddr){ sock = new Socket(); } setSockOpts(sock); - sock.connect(electionAddr, cnxTO); + sock.connect(electionAddr.getReachableOrOne(), cnxTO); if (sock instanceof SSLSocket) { SSLSocket sslSock = (SSLSocket) sock; sslSock.startHandshake(); @@ -843,7 +854,7 @@ private void resetConnectionThreadCount() { } /** - * Thread to listen on some port + * Thread to listen on some ports */ public class Listener extends ZooKeeperThread { @@ -852,25 +863,27 @@ public class Listener extends ZooKeeperThread { private final int portBindMaxRetry; private Runnable socketBindErrorHandler = () -> System.exit(ExitCode.UNABLE_TO_BIND_QUORUM_PORT.getValue()); - volatile ServerSocket ss = null; + private List listenerHandlers; + private final AtomicBoolean socketException; + public Listener() { // During startup of thread, thread name will be overridden to // specific election address super("ListenerThread"); + socketException = new AtomicBoolean(false); + // maximum retry count while trying to bind to election port // see ZOOKEEPER-3320 for more details final Integer maxRetry = Integer.getInteger(ELECTION_PORT_BIND_RETRY, - DEFAULT_PORT_BIND_MAX_RETRY); + DEFAULT_PORT_BIND_MAX_RETRY); if (maxRetry >= 0) { - LOG.info("Election port bind maximum retries is {}", - maxRetry == 0 ? "infinite" : maxRetry); + LOG.info("Election port bind maximum retries is {}", maxRetry == 0 ? "infinite" : maxRetry); portBindMaxRetry = maxRetry; } else { - LOG.info("'{}' contains invalid value: {}(must be >= 0). " - + "Use default value of {} instead.", - ELECTION_PORT_BIND_RETRY, maxRetry, DEFAULT_PORT_BIND_MAX_RETRY); + LOG.info("'{}' contains invalid value: {}(must be >= 0). Use default value of {} instead.", + ELECTION_PORT_BIND_RETRY, maxRetry, DEFAULT_PORT_BIND_MAX_RETRY); portBindMaxRetry = DEFAULT_PORT_BIND_MAX_RETRY; } } @@ -882,122 +895,196 @@ void setSocketBindErrorHandler(Runnable errorHandler) { this.socketBindErrorHandler = errorHandler; } - /** - * Sleeps on accept(). - */ @Override public void run() { - int numRetries = 0; - InetSocketAddress addr; - Socket client = null; - Exception exitException = null; - while ((!shutdown) && (portBindMaxRetry == 0 || numRetries < portBindMaxRetry)) { - try { - if (self.shouldUsePortUnification()) { - LOG.info("Creating TLS-enabled quorum server socket"); - ss = new UnifiedServerSocket(self.getX509Util(), true); - } else if (self.isSslQuorum()) { - LOG.info("Creating TLS-only quorum server socket"); - ss = new UnifiedServerSocket(self.getX509Util(), false); - } else { - ss = new ServerSocket(); - } + if(!shutdown) { + Set addresses; - ss.setReuseAddress(true); + if (self.getQuorumListenOnAllIPs()) { + addresses = self.getElectionAddress().getWildcardAddresses(); + } else { + addresses = self.getElectionAddress().getAllAddresses(); + } - if (self.getQuorumListenOnAllIPs()) { - int port = self.getElectionAddress().getPort(); - addr = new InetSocketAddress(port); - } else { - // Resolve hostname for this server in case the - // underlying ip address has changed. - self.recreateSocketAddresses(self.getId()); - addr = self.getElectionAddress(); - } - LOG.info("My election bind port: " + addr.toString()); - setName(addr.toString()); - ss.bind(addr); - while (!shutdown) { + CountDownLatch latch = new CountDownLatch(addresses.size()); + listenerHandlers = addresses.stream().map(address -> + new ListenerHandler(address, self.shouldUsePortUnification(), self.isSslQuorum(), latch)) + .collect(Collectors.toList()); + + ExecutorService executor = Executors.newFixedThreadPool(addresses.size()); + listenerHandlers.forEach(executor::submit); + + try { + latch.await(); + } catch (InterruptedException ie) { + LOG.error("Interrupted while sleeping. Ignoring exception", ie); + } finally { + // Clean up for shutdown. + for (ListenerHandler handler : listenerHandlers) { try { - client = ss.accept(); - setSockOpts(client); - LOG.info("Received connection request " - + formatInetAddr((InetSocketAddress)client.getRemoteSocketAddress())); - // Receive and handle the connection request - // asynchronously if the quorum sasl authentication is - // enabled. This is required because sasl server - // authentication process may take few seconds to finish, - // this may delay next peer connection requests. - if (quorumSaslAuthEnabled) { - receiveConnectionAsync(client); - } else { - receiveConnection(client); - } - numRetries = 0; - } catch (SocketTimeoutException e) { - LOG.warn("The socket is listening for the election accepted " - + "and it timed out unexpectedly, but will retry." - + "see ZOOKEEPER-2836"); + handler.close(); + } catch (IOException ie) { + // Don't log an error for shutdown. + LOG.debug("Error closing server socket", ie); } } - } catch (IOException e) { - if (shutdown) { - break; - } - LOG.error("Exception while listening", e); - exitException = e; - numRetries++; - try { - ss.close(); - Thread.sleep(1000); - } catch (IOException ie) { - LOG.error("Error closing server socket", ie); - } catch (InterruptedException ie) { - LOG.error("Interrupted while sleeping. " + - "Ignoring exception", ie); - } - closeSocket(client); } } + LOG.info("Leaving listener"); if (!shutdown) { - LOG.error("As I'm leaving the listener thread after " - + numRetries + " errors. " - + "I won't be able to participate in leader " - + "election any longer: " - + formatInetAddr(self.getElectionAddress()) - + ". Use " + ELECTION_PORT_BIND_RETRY + " property to " - + "increase retry count."); - if (exitException instanceof SocketException) { - // After leaving listener thread, the host cannot join the - // quorum anymore, this is a severe error that we cannot - // recover from, so we need to exit + LOG.error("As I'm leaving the listener thread, " + + "I won't be able to participate in leader " + + "election any longer: {}" + , self.getElectionAddress().getAllAddresses().stream().map(NetUtils::formatInetAddr) + .collect(Collectors.joining(","))); + if (socketException.get()) { + // After leaving listener thread, the host cannot join the quorum anymore, + // this is a severe error that we cannot recover from, so we need to exit socketBindErrorHandler.run(); } - } else if (ss != null) { - // Clean up for shutdown. - try { - ss.close(); - } catch (IOException ie) { - // Don't log an error for shutdown. - LOG.debug("Error closing server socket", ie); - } } } /** * Halts this listener thread. */ - void halt(){ - try{ - LOG.debug("Trying to close listener: {}", ss); - if(ss != null) { - LOG.debug("Closing listener: {}", - + QuorumCnxManager.this.mySid); - ss.close(); + void halt() { + LOG.debug("Trying to close listeners"); + if (listenerHandlers != null) { + LOG.debug("Closing listener: {}", QuorumCnxManager.this.mySid); + for (ListenerHandler handler : listenerHandlers) { + try { + handler.close(); + } catch (IOException e) { + LOG.warn("Exception when shutting down listener: ", e); + } + } + } + } + + class ListenerHandler implements Runnable, Closeable { + private ServerSocket serverSocket; + private InetSocketAddress address; + private boolean portUnification; + private boolean sslQuorum; + private CountDownLatch latch; + + ListenerHandler(InetSocketAddress address, boolean portUnification, boolean sslQuorum, + CountDownLatch latch) { + this.address = address; + this.portUnification = portUnification; + this.sslQuorum = sslQuorum; + this.latch = latch; + } + + /** + * Sleeps on acceptConnections(). + */ + @Override + public void run() { + try { + Thread.currentThread().setName("ListenerHandler-" + address); + acceptConnections(); + try { + close(); + } catch (IOException e) { + LOG.warn("Exception when shutting down listener: ", e); + } + } catch (Exception e) { + // Output of unexpected exception, should never happen + LOG.error("Unexpected error ", e); + } finally { + latch.countDown(); + } + } + + @Override + public synchronized void close() throws IOException { + if (serverSocket != null && !serverSocket.isClosed()) { + LOG.debug("Trying to close listeners: {}", serverSocket); + serverSocket.close(); + } + } + + /** + * Sleeps on accept(). + */ + private void acceptConnections() { + int numRetries = 0; + Socket client = null; + + while ((!shutdown) && (portBindMaxRetry == 0 || numRetries < portBindMaxRetry)) { + try { + serverSocket = createNewServerSocket(); + LOG.info("My election bind port: {}", address.toString()); + while (!shutdown) { + try { + client = serverSocket.accept(); + setSockOpts(client); + LOG.info("Received connection request {}", client.getRemoteSocketAddress()); + // Receive and handle the connection request + // asynchronously if the quorum sasl authentication is + // enabled. This is required because sasl server + // authentication process may take few seconds to finish, + // this may delay next peer connection requests. + if (quorumSaslAuthEnabled) { + receiveConnectionAsync(client); + } else { + receiveConnection(client); + } + numRetries = 0; + } catch (SocketTimeoutException e) { + LOG.warn("The socket is listening for the election accepted " + + "and it timed out unexpectedly, but will retry." + + "see ZOOKEEPER-2836"); + } + } + } catch (IOException e) { + if (shutdown) { + break; + } + + LOG.error("Exception while listening", e); + + if (e instanceof SocketException) + socketException.set(true); + + numRetries++; + try { + close(); + Thread.sleep(1000); + } catch (IOException ie) { + LOG.error("Error closing server socket", ie); + } catch (InterruptedException ie) { + LOG.error("Interrupted while sleeping. Ignoring exception", ie); + } + closeSocket(client); + } + } + if (!shutdown) { + LOG.error("Leaving listener thread for address {} after {} errors. Use {} property " + + "to increase retry count.", formatInetAddr(address), numRetries, ELECTION_PORT_BIND_RETRY); + } + } + + private ServerSocket createNewServerSocket() throws IOException { + ServerSocket socket; + + if (portUnification) { + LOG.info("Creating TLS-enabled quorum server socket"); + socket = new UnifiedServerSocket(self.getX509Util(), true); + } else if (sslQuorum) { + LOG.info("Creating TLS-only quorum server socket"); + socket = new UnifiedServerSocket(self.getX509Util(), false); + } else { + socket = new ServerSocket(); } - } catch (IOException e){ - LOG.warn("Exception when shutting down listener: " + e); + + socket.setReuseAddress(true); + socket.bind(address); + + return socket; } } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java index 2e202d02e10..9d43b3dfcc8 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java @@ -33,8 +33,10 @@ import java.nio.ByteBuffer; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedList; import java.util.List; import java.util.Map; import java.util.Map.Entry; @@ -43,6 +45,8 @@ import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; +import java.util.stream.Collectors; +import java.util.stream.IntStream; import javax.security.sasl.SaslException; import org.apache.yetus.audience.InterfaceAudience; @@ -139,11 +143,11 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider private JvmPauseMonitor jvmPauseMonitor; public static final class AddressTuple { - public final InetSocketAddress quorumAddr; - public final InetSocketAddress electionAddr; + public final MultipleAddresses quorumAddr; + public final MultipleAddresses electionAddr; public final InetSocketAddress clientAddr; - public AddressTuple(InetSocketAddress quorumAddr, InetSocketAddress electionAddr, InetSocketAddress clientAddr) { + public AddressTuple(MultipleAddresses quorumAddr, MultipleAddresses electionAddr, InetSocketAddress clientAddr) { this.quorumAddr = quorumAddr; this.electionAddr = electionAddr; this.clientAddr = clientAddr; @@ -161,9 +165,9 @@ public void setObserverMasterPort(int observerMasterPort) { } public static class QuorumServer { - public InetSocketAddress addr = null; + public MultipleAddresses addr = new MultipleAddresses(); - public InetSocketAddress electionAddr = null; + public MultipleAddresses electionAddr = new MultipleAddresses(); public InetSocketAddress clientAddr = null; @@ -203,36 +207,26 @@ public long getId() { * unmodified. */ public void recreateSocketAddresses() { - if (this.addr == null) { + if (this.addr.isEmpty()) { LOG.warn("Server address has not been initialized"); return; } - if (this.electionAddr == null) { + if (this.electionAddr.isEmpty()) { LOG.warn("Election address has not been initialized"); return; } - String host = this.addr.getHostString(); - InetAddress address = null; - try { - address = InetAddress.getByName(host); - } catch (UnknownHostException ex) { - LOG.warn("Failed to resolve address: {}", host, ex); - return; - } - LOG.debug("Resolved address for {}: {}", host, address); - int port = this.addr.getPort(); - this.addr = new InetSocketAddress(address, port); - port = this.electionAddr.getPort(); - this.electionAddr = new InetSocketAddress(address, port); - } - - private void setType(String s) throws ConfigException { - if (s.toLowerCase().equals("observer")) { - type = LearnerType.OBSERVER; - } else if (s.toLowerCase().equals("participant")) { - type = LearnerType.PARTICIPANT; - } else { - throw new ConfigException("Unrecognised peertype: " + s); + this.addr.recreateSocketAddresses(); + this.electionAddr.recreateSocketAddresses(); + } + + private LearnerType getType(String s) throws ConfigException { + switch (s.trim().toLowerCase()) { + case "observer": + return LearnerType.OBSERVER; + case "participant": + return LearnerType.PARTICIPANT; + default: + throw new ConfigException("Unrecognised peertype: " + s); } } @@ -240,17 +234,12 @@ private void setType(String s) throws ConfigException { " where server_config is host:port:port or host:port:port:type and client_config is port or host:port"; public QuorumServer(long sid, String addressStr) throws ConfigException { - // LOG.warn("sid = " + sid + " addressStr = " + addressStr); this.id = sid; + LearnerType newType = null; String serverClientParts[] = addressStr.split(";"); - String serverParts[] = ConfigUtils.getHostAndPort(serverClientParts[0]); - if ((serverClientParts.length > 2) || (serverParts.length < 3) - || (serverParts.length > 4)) { - throw new ConfigException(addressStr + wrongFormat); - } + String serverAddresses[] = serverClientParts[0].split(","); if (serverClientParts.length == 2) { - //LOG.warn("ClientParts: " + serverClientParts[1]); String clientParts[] = ConfigUtils.getHostAndPort(serverClientParts[1]); if (clientParts.length > 2) { throw new ConfigException(addressStr + wrongFormat); @@ -261,37 +250,52 @@ public QuorumServer(long sid, String addressStr) throws ConfigException { try { clientAddr = new InetSocketAddress(hostname, Integer.parseInt(clientParts[clientParts.length - 1])); - //LOG.warn("Set clientAddr to " + clientAddr); } catch (NumberFormatException e) { throw new ConfigException("Address unresolved: " + hostname + ":" + clientParts[clientParts.length - 1]); } } - // server_config should be either host:port:port or host:port:port:type - try { - addr = new InetSocketAddress(serverParts[0], - Integer.parseInt(serverParts[1])); - } catch (NumberFormatException e) { - throw new ConfigException("Address unresolved: " + serverParts[0] + ":" + serverParts[1]); - } - try { - electionAddr = new InetSocketAddress(serverParts[0], - Integer.parseInt(serverParts[2])); - } catch (NumberFormatException e) { - throw new ConfigException("Address unresolved: " + serverParts[0] + ":" + serverParts[2]); - } + for(String serverAddress : serverAddresses) { + String serverParts[] = ConfigUtils.getHostAndPort(serverAddress); + if ((serverClientParts.length > 2) || (serverParts.length < 3) + || (serverParts.length > 4)) { + throw new ConfigException(addressStr + wrongFormat); + } + + // server_config should be either host:port:port or host:port:port:type + InetSocketAddress tempAddress; + InetSocketAddress tempElectionAddress; + try { + tempAddress = new InetSocketAddress(serverParts[0], Integer.parseInt(serverParts[1])); + addr.addAddress(tempAddress); + } catch (NumberFormatException e) { + throw new ConfigException("Address unresolved: " + serverParts[0] + ":" + serverParts[1]); + } + try { + tempElectionAddress = new InetSocketAddress(serverParts[0], Integer.parseInt(serverParts[2])); + electionAddr.addAddress(tempElectionAddress); + } catch (NumberFormatException e) { + throw new ConfigException("Address unresolved: " + serverParts[0] + ":" + serverParts[2]); + } - if(addr.getPort() == electionAddr.getPort()) { + if(tempAddress.getPort() == tempElectionAddress.getPort()) { throw new ConfigException( "Client and election port must be different! Please update the configuration file on server." + sid); - } + } - if (serverParts.length == 4) { - setType(serverParts[3]); - } + if (serverParts.length == 4) { + LearnerType tempType = getType(serverParts[3]); + if (newType == null) + newType = tempType; + + if (newType != tempType) + throw new ConfigException("Multiple addresses should have similar roles: " + type + " vs " + tempType); + } - this.hostname = serverParts[0]; - + this.hostname = serverParts[0]; + } + if(newType != null) + type = newType; setMyAddrs(); } @@ -303,8 +307,10 @@ public QuorumServer(long id, InetSocketAddress addr, public QuorumServer(long id, InetSocketAddress addr, InetSocketAddress electionAddr, InetSocketAddress clientAddr, LearnerType type) { this.id = id; - this.addr = addr; - this.electionAddr = electionAddr; + if(addr != null) + this.addr.addAddress(addr); + if(electionAddr != null) + this.electionAddr.addAddress(electionAddr); this.type = type; this.clientAddr = clientAddr; @@ -312,10 +318,10 @@ public QuorumServer(long id, InetSocketAddress addr, } private void setMyAddrs() { - this.myAddrs = new ArrayList(); - this.myAddrs.add(this.addr); + this.myAddrs = new ArrayList<>(); + this.myAddrs.addAll(this.addr.getAllAddresses()); this.myAddrs.add(this.clientAddr); - this.myAddrs.add(this.electionAddr); + this.myAddrs.addAll(this.electionAddr.getAllAddresses()); this.myAddrs = excludedSpecialAddresses(this.myAddrs); } @@ -331,25 +337,29 @@ public static String delimitedHostString(InetSocketAddress addr) public String toString(){ StringWriter sw = new StringWriter(); - //addr should never be null, but just to make sure - if (addr !=null) { - sw.append(delimitedHostString(addr)); - sw.append(":"); - sw.append(String.valueOf(addr.getPort())); + + List addrList = new LinkedList<>(addr.getAllAddresses()); + List electionAddrList = new LinkedList<>(electionAddr.getAllAddresses()); + + if(addrList.size() > 0 && electionAddrList.size() > 0) { + addrList.sort(Comparator.comparing(InetSocketAddress::getHostString)); + electionAddrList.sort(Comparator.comparing(InetSocketAddress::getHostString)); + sw.append(IntStream.range(0, addrList.size()).mapToObj(i -> String.format("%s:%d:%d", + delimitedHostString(addrList.get(i)), addrList.get(i).getPort(), electionAddrList.get(i).getPort())) + .collect(Collectors.joining(","))); } - if (electionAddr!=null){ - sw.append(":"); - sw.append(String.valueOf(electionAddr.getPort())); - } + if (type == LearnerType.OBSERVER) sw.append(":observer"); - else if (type == LearnerType.PARTICIPANT) sw.append(":participant"); + else if (type == LearnerType.PARTICIPANT) sw.append(":participant"); + if (clientAddr!=null && !isClientAddrFromStatic){ sw.append(";"); sw.append(delimitedHostString(clientAddr)); sw.append(":"); sw.append(String.valueOf(clientAddr.getPort())); } - return sw.toString(); + + return sw.toString(); } public int hashCode() { @@ -367,18 +377,16 @@ private boolean checkAddressesEqual(InetSocketAddress addr1, InetSocketAddress a public boolean equals(Object o){ if (!(o instanceof QuorumServer)) return false; QuorumServer qs = (QuorumServer)o; - if ((qs.id != id) || (qs.type != type)) return false; - if (!checkAddressesEqual(addr, qs.addr)) return false; - if (!checkAddressesEqual(electionAddr, qs.electionAddr)) return false; - if (!checkAddressesEqual(clientAddr, qs.clientAddr)) return false; - return true; + if ((qs.id != id) || (qs.type != type)) return false; + if (!addr.equals(qs.addr)) return false; + if (!electionAddr.equals(qs.electionAddr)) return false; + return checkAddressesEqual(clientAddr, qs.clientAddr); } public void checkAddressDuplicate(QuorumServer s) throws BadArgumentsException { - List otherAddrs = new ArrayList(); - otherAddrs.add(s.addr); + List otherAddrs = new ArrayList<>(s.addr.getAllAddresses()); otherAddrs.add(s.clientAddr); - otherAddrs.add(s.electionAddr); + otherAddrs.addAll(s.electionAddr.getAllAddresses()); otherAddrs = excludedSpecialAddresses(otherAddrs); for (InetSocketAddress my: this.myAddrs) { @@ -805,9 +813,9 @@ public SyncMode getSyncMode() { return syncMode.get(); } - public void setLeaderAddressAndId(InetSocketAddress addr, long newId) { + public void setLeaderAddressAndId(MultipleAddresses addr, long newId) { if (addr != null) { - leaderAddress.set(addr.getHostString()); + leaderAddress.set(String.join(",",addr.getAllHostStrings())); } else { leaderAddress.set(null); } @@ -899,11 +907,11 @@ private AddressTuple getAddrs(){ } } - public InetSocketAddress getQuorumAddress(){ + public MultipleAddresses getQuorumAddress(){ return getAddrs().quorumAddr; } - public InetSocketAddress getElectionAddress(){ + public MultipleAddresses getElectionAddress(){ return getAddrs().electionAddr; } @@ -912,7 +920,7 @@ public InetSocketAddress getClientAddress(){ return (addrs == null) ? null : addrs.clientAddr; } - private void setAddrs(InetSocketAddress quorumAddr, InetSocketAddress electionAddr, InetSocketAddress clientAddr){ + private void setAddrs(MultipleAddresses quorumAddr, MultipleAddresses electionAddr, InetSocketAddress clientAddr){ synchronized (QV_LOCK) { myAddrs.set(new AddressTuple(quorumAddr, electionAddr, clientAddr)); QV_LOCK.notifyAll(); @@ -2171,7 +2179,8 @@ private void updateObserverMasterList() { observerMasters.clear(); StringBuilder sb = new StringBuilder(); for (QuorumServer server : quorumVerifier.getVotingMembers().values()) { - InetSocketAddress addr = new InetSocketAddress(server.addr.getAddress(), observerMasterPort); + InetAddress address = server.addr.getReachableOrOne().getAddress(); + InetSocketAddress addr = new InetSocketAddress(address, observerMasterPort); observerMasters.add(new QuorumServer(server.id, addr)); sb.append(addr).append(","); } @@ -2226,9 +2235,11 @@ QuorumServer validateLearnerMaster(String desiredMaster) { } for (QuorumServer server : observerMasters) { if (sid == null) { - String serverAddr = server.addr.getAddress().getHostAddress() + ':' + server.addr.getPort(); - if (serverAddr.startsWith(desiredMaster)) { - return server; + for(InetSocketAddress address : server.addr.getAllAddresses()) { + String serverAddr = address.getAddress().getHostAddress() + ':' + address.getPort(); + if (serverAddr.startsWith(desiredMaster)) { + return server; + } } } else { if (sid.equals(server.id)) { @@ -2293,39 +2304,39 @@ private boolean updateVote(long designatedLeader, long zxid){ * Updates leader election info to avoid inconsistencies when * a new server tries to join the ensemble. * - * Here is the inconsistency scenario we try to solve by updating the peer + * Here is the inconsistency scenario we try to solve by updating the peer * epoch after following leader: * * Let's say we have an ensemble with 3 servers z1, z2 and z3. * - * 1. z1, z2 were following z3 with peerEpoch to be 0xb8, the new epoch is + * 1. z1, z2 were following z3 with peerEpoch to be 0xb8, the new epoch is * 0xb9, aka current accepted epoch on disk. * 2. z2 get restarted, which will use 0xb9 as it's peer epoch when loading * the current accept epoch from disk. - * 3. z2 received notification from z1 and z3, which is following z3 with + * 3. z2 received notification from z1 and z3, which is following z3 with * epoch 0xb8, so it started following z3 again with peer epoch 0xb8. - * 4. before z2 successfully connected to z3, z3 get restarted with new + * 4. before z2 successfully connected to z3, z3 get restarted with new * epoch 0xb9. - * 5. z2 will retry around a few round (default 5s) before giving up, + * 5. z2 will retry around a few round (default 5s) before giving up, * meanwhile it will report z3 as leader. * 6. z1 restarted, and looking with peer epoch 0xb9. * 7. z1 voted z3, and z3 was elected as leader again with peer epoch 0xb9. - * 8. z2 successfully connected to z3 before giving up, but with peer + * 8. z2 successfully connected to z3 before giving up, but with peer * epoch 0xb8. - * 9. z1 get restarted, looking for leader with peer epoch 0xba, but cannot - * join, because z2 is reporting peer epoch 0xb8, while z3 is reporting + * 9. z1 get restarted, looking for leader with peer epoch 0xba, but cannot + * join, because z2 is reporting peer epoch 0xb8, while z3 is reporting * 0xb9. * - * By updating the election vote after actually following leader, we can + * By updating the election vote after actually following leader, we can * avoid this kind of stuck happened. * - * Btw, the zxid and electionEpoch could be inconsistent because of the same - * reason, it's better to update these as well after syncing with leader, but - * that required protocol change which is non trivial. This problem is worked - * around by skipping comparing the zxid and electionEpoch when counting for + * Btw, the zxid and electionEpoch could be inconsistent because of the same + * reason, it's better to update these as well after syncing with leader, but + * that required protocol change which is non trivial. This problem is worked + * around by skipping comparing the zxid and electionEpoch when counting for * votes for out of election servers during looking for leader. - * - * See https://issues.apache.org/jira/browse/ZOOKEEPER-1732 + * + * {@see https://issues.apache.org/jira/browse/ZOOKEEPER-1732} */ protected void updateElectionVote(long newEpoch) { Vote currentVote = getCurrentVote(); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java index 7759a2cd6b8..e52b41d3bab 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java @@ -20,7 +20,9 @@ import java.io.IOException; import java.io.PrintWriter; import java.nio.ByteBuffer; +import java.util.Objects; import java.util.function.BiConsumer; +import java.util.stream.Collectors; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.KeeperException; @@ -176,9 +178,11 @@ public void dumpConf(PrintWriter pwriter) { pwriter.print("electionAlg="); pwriter.println(self.getElectionType()); pwriter.print("electionPort="); - pwriter.println(self.getElectionAddress().getPort()); + pwriter.println(self.getElectionAddress().getAllPorts() + .stream().map(Objects::toString).collect(Collectors.joining(","))); pwriter.print("quorumPort="); - pwriter.println(self.getQuorumAddress().getPort()); + pwriter.println(self.getQuorumAddress().getAllPorts() + .stream().map(Objects::toString).collect(Collectors.joining(","))); pwriter.print("peerType="); pwriter.println(self.getLearnerType().ordinal()); pwriter.println("membership: "); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java index 103ff21e0ac..fed1d84ef15 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java @@ -19,6 +19,8 @@ package org.apache.zookeeper.server.quorum; import java.io.PrintWriter; +import java.util.Objects; +import java.util.stream.Collectors; import org.apache.zookeeper.jmx.MBeanRegistry; import org.apache.zookeeper.server.DataTreeBean; @@ -166,9 +168,11 @@ public void dumpConf(PrintWriter pwriter) { pwriter.print("electionAlg="); pwriter.println(self.getElectionType()); pwriter.print("electionPort="); - pwriter.println(self.getElectionAddress().getPort()); + pwriter.println(self.getElectionAddress().getAllPorts() + .stream().map(Objects::toString).collect(Collectors.joining(","))); pwriter.print("quorumPort="); - pwriter.println(self.getQuorumAddress().getPort()); + pwriter.println(self.getQuorumAddress().getAllPorts() + .stream().map(Objects::toString).collect(Collectors.joining(","))); pwriter.print("peerType="); pwriter.println(self.getLearnerType().ordinal()); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java index 285f11a8593..c6e0185a093 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java @@ -20,6 +20,8 @@ import org.apache.zookeeper.jmx.ZKMBeanInfo; +import java.util.stream.Collectors; + /** * A remote peer bean only provides limited information about the remote peer, * and the peer cannot be managed remotely. @@ -45,11 +47,15 @@ public boolean isHidden() { } public String getQuorumAddress() { - return peer.addr.getHostString()+":"+peer.addr.getPort(); + return peer.addr.getAllAddresses().stream() + .map(address -> String.format("%s:%d", address.getHostString(), address.getPort())) + .collect(Collectors.joining(",")); } public String getElectionAddress() { - return peer.electionAddr.getHostString() + ":" + peer.electionAddr.getPort(); + return peer.electionAddr.getAllAddresses().stream() + .map(address -> String.format("%s:%d", address.getHostString(), address.getPort())) + .collect(Collectors.joining(",")); } public String getClientAddress() { diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java index 276f35f4767..25373fae035 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java @@ -32,7 +32,7 @@ import java.util.Date; import java.util.HashMap; import java.util.Map; -import java.util.Random; +import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; import java.net.Socket; @@ -47,10 +47,8 @@ import org.slf4j.LoggerFactory; import org.apache.zookeeper.PortAssignment; import org.apache.zookeeper.ZKTestCase; -import org.apache.zookeeper.server.quorum.QuorumCnxManager; import org.apache.zookeeper.server.quorum.QuorumCnxManager.Message; import org.apache.zookeeper.server.quorum.QuorumCnxManager.InitialMessage; -import org.apache.zookeeper.server.quorum.QuorumPeer; import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType; import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer; import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState; @@ -196,15 +194,14 @@ public void testCnxManager() throws Exception { @Test public void testCnxManagerTimeout() throws Exception { - Random rand = new Random(); - byte b = (byte) rand.nextInt(); + int address = ThreadLocalRandom.current().nextInt(1, 255); int deadPort = PortAssignment.unique(); - String deadAddress = "10.1.1." + b; + String deadAddress = "10.1.1." + address; LOG.info("This is the dead address I'm trying: " + deadAddress); peers.put(Long.valueOf(2), - new QuorumServer(2, + new QuorumServer(2L, new InetSocketAddress(deadAddress, deadPort), new InetSocketAddress(deadAddress, PortAssignment.unique()), new InetSocketAddress(deadAddress, PortAssignment.unique()))); @@ -223,7 +220,7 @@ public void testCnxManagerTimeout() throws Exception { cnxManager.toSend(2L, createMsg(ServerState.LOOKING.ordinal(), 1, -1, 1)); long end = Time.currentElapsedTime(); - if((end - begin) > 6000) Assert.fail("Waited more than necessary"); + if((end - begin) > 10_000) Assert.fail("Waited more than necessary"); cnxManager.halt(); Assert.assertFalse(cnxManager.listener.isAlive()); } @@ -247,18 +244,18 @@ public void testCnxManagerSpinLock() throws Exception { LOG.error("Null listener when initializing cnx manager"); } - int port = peers.get(peer.getId()).electionAddr.getPort(); - LOG.info("Election port: " + port); + InetSocketAddress address = peers.get(peer.getId()).electionAddr.getReachableOrOne(); + LOG.info("Election port: " + address.getPort()); Thread.sleep(1000); SocketChannel sc = SocketChannel.open(); - sc.socket().connect(peers.get(1L).electionAddr, 5000); + sc.socket().connect(address, 5000); - InetSocketAddress otherAddr = peers.get(Long.valueOf(2)).electionAddr; + InetSocketAddress otherAddr = peers.get(2L).electionAddr.getReachableOrOne(); DataOutputStream dout = new DataOutputStream(sc.socket().getOutputStream()); dout.writeLong(QuorumCnxManager.PROTOCOL_VERSION); - dout.writeLong(2); + dout.writeLong(2L); String addr = otherAddr.getHostString()+ ":" + otherAddr.getPort(); byte[] addr_bytes = addr.getBytes(); dout.writeInt(addr_bytes.length); @@ -309,7 +306,7 @@ public void testCnxManagerListenerThreadConfigurableRetry() throws Exception { 2181, 3, myid, 1000, 2, 2, 2); final QuorumCnxManager cnxManager = peer.createCnxnManager(); final QuorumCnxManager.Listener listener = cnxManager.listener; - final AtomicBoolean errorHappend = new AtomicBoolean(); + final AtomicBoolean errorHappend = new AtomicBoolean(false); listener.setSocketBindErrorHandler(() -> errorHappend.set(true)); listener.start(); // listener thread should stop and throws error which notify QuorumPeer about error. @@ -341,13 +338,13 @@ public void testCnxManagerNPE() throws Exception { } else { LOG.error("Null listener when initializing cnx manager"); } - int port = peers.get(peer.getId()).electionAddr.getPort(); - LOG.info("Election port: " + port); + InetSocketAddress address = peers.get(peer.getId()).electionAddr.getReachableOrOne(); + LOG.info("Election port: " + address.getPort()); Thread.sleep(1000); SocketChannel sc = SocketChannel.open(); - sc.socket().connect(peers.get(1L).electionAddr, 5000); + sc.socket().connect(address, 5000); /* * Write id (3.4.6 protocol). This previously caused a NPE in @@ -388,12 +385,12 @@ public void testSocketTimeout() throws Exception { } else { LOG.error("Null listener when initializing cnx manager"); } - int port = peers.get(peer.getId()).electionAddr.getPort(); - LOG.info("Election port: " + port); + InetSocketAddress address = peers.get(peer.getId()).electionAddr.getReachableOrOne(); + LOG.info("Election port: " + address.getPort()); Thread.sleep(1000); Socket sock = new Socket(); - sock.connect(peers.get(1L).electionAddr, 5000); + sock.connect(address, 5000); long begin = Time.currentElapsedTime(); // Read without sending data. Verify timeout. cnxManager.receiveConnection(sock); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java index 7eccf608994..c24b3847cb2 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java @@ -98,7 +98,7 @@ protected void sockConnect(Socket sock, InetSocketAddress addr, int timeout) } } - @Test(expected=IOException.class) + @Test(expected = IOException.class) public void connectionRetryTimeoutTest() throws Exception { Learner learner = new TimeoutLearner(); learner.self = new QuorumPeer(); @@ -110,8 +110,9 @@ public void connectionRetryTimeoutTest() throws Exception { InetSocketAddress addr = new InetSocketAddress(1111); // we expect this to throw an IOException since we're faking socket connect errors every time - learner.connectToLeader(addr, ""); + learner.connectToLeader(new MultipleAddresses(addr), ""); } + @Test public void connectionInitLimitTimeoutTest() throws Exception { TimeoutLearner learner = new TimeoutLearner(); @@ -124,17 +125,17 @@ public void connectionInitLimitTimeoutTest() throws Exception { InetSocketAddress addr = new InetSocketAddress(1111); // pretend each connect attempt takes 4000 milliseconds - learner.setTimeMultiplier((long)4000 * 1000000); + learner.setTimeMultiplier((long)4000 * 1000_000); learner.setPassConnectAttempt(5); // we expect this to throw an IOException since we're faking socket connect errors every time try { - learner.connectToLeader(addr, ""); + learner.connectToLeader(new MultipleAddresses(addr), ""); Assert.fail("should have thrown IOException!"); } catch (IOException e) { //good, wanted to see that, let's make sure we ran out of time - Assert.assertTrue(learner.nanoTime() > 2000*5*1000000); + Assert.assertTrue(learner.nanoTime() > 2000 * 5 * 1000_000); Assert.assertEquals(3, learner.getSockConnectAttempt()); } } @@ -149,14 +150,14 @@ public void connectToLearnerMasterLimitTest() throws Exception { learner.self.setConnectToLearnerMasterLimit(5); InetSocketAddress addr = new InetSocketAddress(1111); - learner.setTimeMultiplier((long)4000 * 1000000); + learner.setTimeMultiplier((long)4000 * 1000_000); learner.setPassConnectAttempt(5); try { - learner.connectToLeader(addr, ""); + learner.connectToLeader(new MultipleAddresses(addr), ""); Assert.fail("should have thrown IOException!"); } catch (IOException e) { - Assert.assertTrue(learner.nanoTime() > 2000*5*1000000); + Assert.assertTrue(learner.nanoTime() > 2000 * 5 * 1000_000); Assert.assertEquals(3, learner.getSockConnectAttempt()); } } 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 new file mode 100644 index 00000000000..206ef7dbf38 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/MultipleAddressesTest.java @@ -0,0 +1,169 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.server.quorum; + +import org.apache.commons.collections.CollectionUtils; +import org.apache.zookeeper.PortAssignment; +import org.junit.Assert; +import org.junit.Test; + +import java.net.InetAddress; +import java.net.InetSocketAddress; +import java.net.NoRouteToHostException; +import java.net.UnknownHostException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.stream.Collectors; +import java.util.stream.IntStream; + +public class MultipleAddressesTest { + + public final static int PORTS_AMOUNT = 10; + + @Test + public void testIsEmpty() { + MultipleAddresses multipleAddresses = new MultipleAddresses(); + Assert.assertTrue(multipleAddresses.isEmpty()); + + multipleAddresses.addAddress(new InetSocketAddress(22)); + Assert.assertFalse(multipleAddresses.isEmpty()); + } + + @Test + public void testGetAllAddresses() { + List addresses = getAddressList(); + MultipleAddresses multipleAddresses = new MultipleAddresses(addresses); + + Assert.assertTrue(CollectionUtils.isEqualCollection(addresses, multipleAddresses.getAllAddresses())); + + multipleAddresses.addAddress(addresses.get(1)); + Assert.assertTrue(CollectionUtils.isEqualCollection(addresses, multipleAddresses.getAllAddresses())); + } + + @Test + public void testGetAllHostStrings() { + List addresses = getAddressList(); + List hostStrings = getHostStrings(addresses); + MultipleAddresses multipleAddresses = new MultipleAddresses(addresses); + + Assert.assertTrue(CollectionUtils.isEqualCollection(hostStrings, multipleAddresses.getAllHostStrings())); + + multipleAddresses.addAddress(addresses.get(addresses.size()-1)); + Assert.assertTrue(CollectionUtils.isEqualCollection(hostStrings, multipleAddresses.getAllHostStrings())); + } + + @Test + public void testGetAllPorts() { + List ports = getPortList(); + MultipleAddresses multipleAddresses = new MultipleAddresses(getAddressList(ports)); + + Assert.assertTrue(CollectionUtils.isEqualCollection(ports, multipleAddresses.getAllPorts())); + + multipleAddresses.addAddress(new InetSocketAddress("localhost", ports.get(ports.size() - 1))); + Assert.assertTrue(CollectionUtils.isEqualCollection(ports, multipleAddresses.getAllPorts())); + } + + @Test + public void testGetWildcardAddresses() { + List ports = getPortList(); + List addresses = getAddressList(ports); + MultipleAddresses multipleAddresses = new MultipleAddresses(addresses); + List allAddresses = ports.stream().map(InetSocketAddress::new).collect(Collectors.toList()); + + Assert.assertTrue(CollectionUtils.isEqualCollection(allAddresses, multipleAddresses.getWildcardAddresses())); + + multipleAddresses.addAddress(new InetSocketAddress("localhost", ports.get(ports.size() - 1))); + Assert.assertTrue(CollectionUtils.isEqualCollection(allAddresses, multipleAddresses.getWildcardAddresses())); + } + + @Test + public void testGetValidAddress() throws NoRouteToHostException { + List addresses = getAddressList(); + MultipleAddresses multipleAddresses = new MultipleAddresses(addresses); + + Assert.assertTrue(addresses.contains(multipleAddresses.getReachableAddress())); + } + + @Test(expected = NoRouteToHostException.class) + public void testGetValidAddressWithNotValid() throws NoRouteToHostException { + MultipleAddresses multipleAddresses = new MultipleAddresses(new InetSocketAddress("10.0.0.1", 22)); + multipleAddresses.getReachableAddress(); + } + + @Test + public void testRecreateSocketAddresses() throws UnknownHostException { + List searchedAddresses = Arrays.stream(InetAddress.getAllByName("google.com")) + .map(addr -> new InetSocketAddress(addr, 222)).collect(Collectors.toList()); + + MultipleAddresses multipleAddresses = new MultipleAddresses(searchedAddresses.get(searchedAddresses.size() - 1)); + List addresses = new ArrayList<>(multipleAddresses.getAllAddresses()); + + Assert.assertEquals(1, addresses.size()); + Assert.assertEquals(searchedAddresses.get(searchedAddresses.size() - 1), addresses.get(0)); + + multipleAddresses.recreateSocketAddresses(); + + addresses = new ArrayList<>(multipleAddresses.getAllAddresses()); + Assert.assertEquals(1, addresses.size()); + Assert.assertEquals(searchedAddresses.get(0), addresses.get(0)); + } + + @Test + public void testRecreateSocketAddressesWithWrongAddresses() { + InetSocketAddress address = new InetSocketAddress("locahost", 222); + MultipleAddresses multipleAddresses = new MultipleAddresses(address); + multipleAddresses.recreateSocketAddresses(); + + Assert.assertEquals(address, multipleAddresses.getOne()); + } + + @Test + public void testEquals() { + List addresses = getAddressList(); + + MultipleAddresses multipleAddresses = new MultipleAddresses(addresses); + MultipleAddresses multipleAddressesEquals = new MultipleAddresses(addresses); + + Assert.assertEquals(multipleAddresses, multipleAddressesEquals); + + MultipleAddresses multipleAddressesNotEquals = new MultipleAddresses(getAddressList()); + + Assert.assertNotEquals(multipleAddresses, multipleAddressesNotEquals); + } + + public List getPortList() { + return IntStream.range(0, PORTS_AMOUNT).mapToObj(i -> PortAssignment.unique()).collect(Collectors.toList()); + } + + public List getAddressList() { + return getAddressList(getPortList()); + } + + public List getAddressList(List ports) { + return IntStream.range(0, ports.size()) + .mapToObj(i -> new InetSocketAddress("127.0.0." + i, ports.get(i))).collect(Collectors.toList()); + } + + private List getHostStrings(List addresses) { + return IntStream.range(0, addresses.size()) + .mapToObj(i -> "127.0.0."+ i).collect(Collectors.toList()); + } + +} \ No newline at end of file diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java index a0bbc09deeb..84dcedcd6ca 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java @@ -384,7 +384,6 @@ public void testElectionFraud() throws IOException, InterruptedException { Assert.assertTrue("All servers should join the quorum", servers.mt[falseLeader].main.quorumPeer.follower != null); // to keep the quorum peer running and force it to go into the looking state, we kill leader election - // and close the connection to the leader servers.mt[falseLeader].main.quorumPeer.electionAlg.shutdown(); servers.mt[falseLeader].main.quorumPeer.follower.getSocket().close(); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java index bd9e5881c25..652767f2446 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/ReconfigFailureCasesTest.java @@ -79,8 +79,8 @@ public void testIncrementalReconfigInvokedOnHiearchicalQS() throws Exception { for (int i = 1; i <= 5; i++) { members.add("server." + i + "=127.0.0.1:" - + qu.getPeer(i).peer.getQuorumAddress().getPort() + ":" - + qu.getPeer(i).peer.getElectionAddress().getPort() + ";" + + qu.getPeer(i).peer.getQuorumAddress().getAllPorts().get(0) + ":" + + qu.getPeer(i).peer.getElectionAddress().getAllPorts().get(0) + ";" + "127.0.0.1:" + qu.getPeer(i).peer.getClientPort()); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java index 6d711fc911b..796df06b888 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/QuorumUtil.java @@ -151,7 +151,7 @@ public void startAll() throws IOException { LOG.info("Checking ports " + hostPort); for (String hp : hostPort.split(",")) { - Assert.assertTrue("waiting for server up", ClientBase.waitForServerUp(hp, + Assert.assertTrue("waiting for server " + hp + " up", ClientBase.waitForServerUp(hp, ClientBase.CONNECTION_TIMEOUT)); LOG.info(hp + " is accepting client connections"); } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigExceptionTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigExceptionTest.java index 5eda4b0f429..a564dcdfc91 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigExceptionTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigExceptionTest.java @@ -204,8 +204,8 @@ private boolean reconfigPort() throws KeeperException, InterruptedException { leaderId++; int followerId = leaderId == 1 ? 2 : 1; joiningServers.add("server." + followerId + "=localhost:" - + qu.getPeer(followerId).peer.getQuorumAddress().getPort() /*quorum port*/ - + ":" + qu.getPeer(followerId).peer.getElectionAddress().getPort() /*election port*/ + + qu.getPeer(followerId).peer.getQuorumAddress().getAllPorts().get(0) /*quorum port*/ + + ":" + qu.getPeer(followerId).peer.getElectionAddress().getAllPorts().get(0) /*election port*/ + ":participant;localhost:" + PortAssignment.unique()/* new client port */); zkAdmin.reconfigure(joiningServers, null, null, -1, new Stat()); return true; diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigMisconfigTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigMisconfigTest.java index 219981e1023..484a7600584 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigMisconfigTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigMisconfigTest.java @@ -119,8 +119,8 @@ private boolean reconfigPort() throws KeeperException, InterruptedException { leaderId++; int followerId = leaderId == 1 ? 2 : 1; joiningServers.add("server." + followerId + "=localhost:" - + qu.getPeer(followerId).peer.getQuorumAddress().getPort() /*quorum port*/ - + ":" + qu.getPeer(followerId).peer.getElectionAddress().getPort() /*election port*/ + + qu.getPeer(followerId).peer.getQuorumAddress().getAllPorts().get(0) /*quorum port*/ + + ":" + qu.getPeer(followerId).peer.getElectionAddress().getAllPorts().get(0) /*election port*/ + ":participant;localhost:" + PortAssignment.unique()/* new client port */); zkAdmin.reconfigure(joiningServers, null, null, -1, new Stat()); return true; diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java index 39da2eb20fe..e8c80b879a4 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java @@ -143,12 +143,13 @@ public static String testServerHasConfig(ZooKeeper zk, String configStr = new String(config); if (joiningServers != null) { for (String joiner : joiningServers) { - Assert.assertTrue(configStr.contains(joiner)); + Assert.assertTrue("Config:<" + configStr + ">\n" + joiner, configStr.contains(joiner)); } } if (leavingServers != null) { for (String leaving : leavingServers) - Assert.assertFalse(configStr.contains("server.".concat(leaving))); + Assert.assertFalse("Config:<" + configStr + ">\n" + leaving, + configStr.contains("server.".concat(leaving))); } return configStr; @@ -293,11 +294,10 @@ public void testRemoveAddOne() throws Exception { joiningServers.add("server." + leavingIndex + "=localhost:" - + qu.getPeer(leavingIndex).peer.getQuorumAddress() - .getPort() + + qu.getPeer(leavingIndex).peer.getQuorumAddress().getAllPorts().get(0) + ":" - + qu.getPeer(leavingIndex).peer.getElectionAddress() - .getPort() + ":participant;localhost:" + + qu.getPeer(leavingIndex).peer.getElectionAddress().getAllPorts().get(0) + + ":participant;localhost:" + qu.getPeer(leavingIndex).peer.getClientPort()); String configStr = reconfig(zkAdmin1, null, leavingServers, null, -1); @@ -370,17 +370,17 @@ public void testRemoveAddTwo() throws Exception { // remember these servers so we can add them back later joiningServers.add("server." + leavingIndex1 + "=localhost:" - + qu.getPeer(leavingIndex1).peer.getQuorumAddress().getPort() + + qu.getPeer(leavingIndex1).peer.getQuorumAddress().getAllPorts().get(0) + ":" - + qu.getPeer(leavingIndex1).peer.getElectionAddress().getPort() + + qu.getPeer(leavingIndex1).peer.getElectionAddress().getAllPorts().get(0) + ":participant;localhost:" + qu.getPeer(leavingIndex1).peer.getClientPort()); // this server will be added back as an observer joiningServers.add("server." + leavingIndex2 + "=localhost:" - + qu.getPeer(leavingIndex2).peer.getQuorumAddress().getPort() + + qu.getPeer(leavingIndex2).peer.getQuorumAddress().getAllPorts().get(0) + ":" - + qu.getPeer(leavingIndex2).peer.getElectionAddress().getPort() + + qu.getPeer(leavingIndex2).peer.getElectionAddress().getAllPorts().get(0) + ":observer;localhost:" + qu.getPeer(leavingIndex2).peer.getClientPort()); @@ -548,11 +548,10 @@ public void testRoleChange() throws Exception { joiningServers.add("server." + changingIndex + "=localhost:" - + qu.getPeer(changingIndex).peer.getQuorumAddress() - .getPort() + + qu.getPeer(changingIndex).peer.getQuorumAddress().getAllPorts().get(0) + ":" - + qu.getPeer(changingIndex).peer.getElectionAddress() - .getPort() + ":" + newRole + ";localhost:" + + qu.getPeer(changingIndex).peer.getElectionAddress().getAllPorts().get(0) + + ":" + newRole + ";localhost:" + qu.getPeer(changingIndex).peer.getClientPort()); reconfig(zkAdmin1, joiningServers, null, null, -1); @@ -599,8 +598,8 @@ public void testPortChange() throws Exception { // modify follower's client port - int quorumPort = qu.getPeer(followerIndex).peer.getQuorumAddress().getPort(); - int electionPort = qu.getPeer(followerIndex).peer.getElectionAddress().getPort(); + int quorumPort = qu.getPeer(followerIndex).peer.getQuorumAddress().getAllPorts().get(0); + int electionPort = qu.getPeer(followerIndex).peer.getElectionAddress().getAllPorts().get(0); int oldClientPort = qu.getPeer(followerIndex).peer.getClientPort(); int newClientPort = PortAssignment.unique(); joiningServers.add("server." + followerIndex + "=localhost:" + quorumPort @@ -668,7 +667,7 @@ ClientBase.CONNECTION_TIMEOUT, new Watcher() { joiningServers.add("server." + leaderIndex + "=localhost:" + newQuorumPort + ":" - + qu.getPeer(leaderIndex).peer.getElectionAddress().getPort() + + qu.getPeer(leaderIndex).peer.getElectionAddress().getAllPorts().get(0) + ":participant;localhost:" + qu.getPeer(leaderIndex).peer.getClientPort()); @@ -676,8 +675,7 @@ ClientBase.CONNECTION_TIMEOUT, new Watcher() { testNormalOperation(zkArr[followerIndex], zkArr[leaderIndex]); - Assert.assertTrue(qu.getPeer(leaderIndex).peer.getQuorumAddress() - .getPort() == newQuorumPort); + Assert.assertEquals((int) qu.getPeer(leaderIndex).peer.getQuorumAddress().getAllPorts().get(0), newQuorumPort); joiningServers.clear(); @@ -685,7 +683,7 @@ ClientBase.CONNECTION_TIMEOUT, new Watcher() { for (int i = 1; i <= 3; i++) { joiningServers.add("server." + i + "=localhost:" - + qu.getPeer(i).peer.getQuorumAddress().getPort() + ":" + + qu.getPeer(i).peer.getQuorumAddress().getAllPorts().get(0) + ":" + PortAssignment.unique() + ":participant;localhost:" + qu.getPeer(i).peer.getClientPort()); } @@ -731,8 +729,8 @@ private void testPortChangeToBlockedPort(boolean testLeader) throws Exception { int reconfigIndex = testLeader ? followerIndex : leaderIndex; // modify server's client port - int quorumPort = qu.getPeer(serverIndex).peer.getQuorumAddress().getPort(); - int electionPort = qu.getPeer(serverIndex).peer.getElectionAddress().getPort(); + int quorumPort = qu.getPeer(serverIndex).peer.getQuorumAddress().getAllPorts().get(0); + int electionPort = qu.getPeer(serverIndex).peer.getElectionAddress().getAllPorts().get(0); int oldClientPort = qu.getPeer(serverIndex).peer.getClientPort(); int newClientPort = PortAssignment.unique(); @@ -829,8 +827,8 @@ public void testQuorumSystemChange() throws Exception { for (int i = 1; i <= 5; i++) { members.add("server." + i + "=127.0.0.1:" - + qu.getPeer(i).peer.getQuorumAddress().getPort() + ":" - + qu.getPeer(i).peer.getElectionAddress().getPort() + ";" + + qu.getPeer(i).peer.getQuorumAddress().getAllPorts().get(0) + ":" + + qu.getPeer(i).peer.getElectionAddress().getAllPorts().get(0) + ";" + "127.0.0.1:" + qu.getPeer(i).peer.getClientPort()); } @@ -861,8 +859,8 @@ public void testQuorumSystemChange() throws Exception { members.clear(); for (int i = 1; i <= 3; i++) { members.add("server." + i + "=127.0.0.1:" - + qu.getPeer(i).peer.getQuorumAddress().getPort() + ":" - + qu.getPeer(i).peer.getElectionAddress().getPort() + ";" + + qu.getPeer(i).peer.getQuorumAddress().getAllPorts().get(0) + ":" + + qu.getPeer(i).peer.getElectionAddress().getAllPorts().get(0) + ";" + "127.0.0.1:" + qu.getPeer(i).peer.getClientPort()); } @@ -941,9 +939,9 @@ public void testJMXBeanAfterRemoveAddOne() throws Exception { // remember this server so we can add it back later joiningServers.add("server." + leavingIndex + "=127.0.0.1:" - + qu.getPeer(leavingIndex).peer.getQuorumAddress().getPort() + + qu.getPeer(leavingIndex).peer.getQuorumAddress().getAllPorts().get(0) + ":" - + qu.getPeer(leavingIndex).peer.getElectionAddress().getPort() + + qu.getPeer(leavingIndex).peer.getElectionAddress().getAllPorts().get(0) + ":participant;127.0.0.1:" + qu.getPeer(leavingIndex).peer.getClientPort()); @@ -1019,9 +1017,9 @@ public void testJMXBeanAfterRoleChange() throws Exception { // exactly as it is now, except for role change joiningServers.add("server." + changingIndex + "=127.0.0.1:" - + qu.getPeer(changingIndex).peer.getQuorumAddress().getPort() + + qu.getPeer(changingIndex).peer.getQuorumAddress().getAllPorts().get(0) + ":" - + qu.getPeer(changingIndex).peer.getElectionAddress().getPort() + + qu.getPeer(changingIndex).peer.getElectionAddress().getAllPorts().get(0) + ":" + newRole + ";127.0.0.1:" + qu.getPeer(changingIndex).peer.getClientPort()); @@ -1058,7 +1056,8 @@ private void assertLocalPeerMXBeanAttributes(QuorumPeer qp, qp.getClientAddress().getHostString() + ":" + qp.getClientAddress().getPort(), JMXEnv.ensureBeanAttribute(beanName, "ClientAddress")); Assert.assertEquals("Mismatches LearnerType!", - qp.getElectionAddress().getHostString() + ":" + qp.getElectionAddress().getPort(), + qp.getElectionAddress().getOne().getHostString() + ":" + + qp.getElectionAddress().getOne().getPort(), JMXEnv.ensureBeanAttribute(beanName, "ElectionAddress")); Assert.assertEquals("Mismatches PartOfEnsemble!", isPartOfEnsemble, JMXEnv.ensureBeanAttribute(beanName, "PartOfEnsemble")); @@ -1096,10 +1095,11 @@ private void assertRemotePeerMXBeanAttributes(QuorumServer qs, getNumericalAddrPort(qs.clientAddr.getHostString() + ":" + qs.clientAddr.getPort()), getAddrPortFromBean(beanName, "ClientAddress") ); Assert.assertEquals("Mismatches ElectionAddress!", - getNumericalAddrPort(qs.electionAddr.getHostString() + ":" + qs.electionAddr.getPort()), + getNumericalAddrPort(qs.electionAddr.getOne().getHostString() + ":" + + qs.electionAddr.getOne().getPort()), getAddrPortFromBean(beanName, "ElectionAddress") ); Assert.assertEquals("Mismatches QuorumAddress!", - getNumericalAddrPort(qs.addr.getHostString() + ":" + qs.addr.getPort()), + getNumericalAddrPort(qs.addr.getOne().getHostString() + ":" + qs.addr.getOne().getPort()), getAddrPortFromBean(beanName, "QuorumAddress") ); } } From 5b22432c15794d069ee48a1711161b1bc6f71fd6 Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Mon, 12 Aug 2019 13:00:36 +0200 Subject: [PATCH 02/14] ZOOKEEPER-3188: fix LeaderElection to work with multiple election addresses --- .../server/quorum/QuorumCnxManager.java | 106 +++++++++++++----- .../server/quorum/QuorumPeerMainTest.java | 3 +- 2 files changed, 80 insertions(+), 29 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java index 9b9a6f6dec5..e505fb49542 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java @@ -26,7 +26,9 @@ import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; +import java.net.InetAddress; import java.net.InetSocketAddress; +import java.net.NoRouteToHostException; import java.net.ServerSocket; import java.net.Socket; import java.net.SocketException; @@ -34,6 +36,7 @@ import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.nio.channels.UnresolvedAddressException; +import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.HashSet; @@ -197,11 +200,11 @@ static public class Message { */ static public class InitialMessage { public Long sid; - public InetSocketAddress electionAddr; + public List electionAddr; - InitialMessage(Long sid, InetSocketAddress address) { + InitialMessage(Long sid, List addresses) { this.sid = sid; - this.electionAddr = address; + this.electionAddr = addresses; } @SuppressWarnings("serial") @@ -237,28 +240,33 @@ static public InitialMessage parse(Long protocolVersion, DataInputStream din) num_read, remaining, sid); } - String addr = new String(b); - String[] host_port; - try { - host_port = ConfigUtils.getHostAndPort(addr); - } catch (ConfigException e) { - throw new InitialMessageException("Badly formed address: %s", addr); - } + String[] addressStrings = new String(b).split(","); + List addresses = new ArrayList<>(addressStrings.length); + for(String addr : addressStrings) { - if (host_port.length != 2) { - throw new InitialMessageException("Badly formed address: %s", addr); - } + String[] host_port; + try { + host_port = ConfigUtils.getHostAndPort(addr); + } catch (ConfigException e) { + throw new InitialMessageException("Badly formed address: %s", addr); + } - int port; - try { - port = Integer.parseInt(host_port[1]); - } catch (NumberFormatException e) { - throw new InitialMessageException("Bad port number: %s", host_port[1]); - } catch (ArrayIndexOutOfBoundsException e) { - throw new InitialMessageException("No port number in: %s", addr); + if (host_port.length != 2) { + throw new InitialMessageException("Badly formed address: %s", addr); + } + + int port; + try { + port = Integer.parseInt(host_port[1]); + } catch (NumberFormatException e) { + throw new InitialMessageException("Bad port number: %s", host_port[1]); + } catch (ArrayIndexOutOfBoundsException e) { + throw new InitialMessageException("No port number in: %s", addr); + } + addresses.add(new InetSocketAddress(host_port[0], port)); } - return new InitialMessage(sid, new InetSocketAddress(host_port[0], port)); + return new InitialMessage(sid, addresses); } } @@ -424,8 +432,8 @@ private boolean startConnection(Socket sock, Long sid) // represents protocol version (in other words - message type) dout.writeLong(PROTOCOL_VERSION); dout.writeLong(self.getId()); - InetSocketAddress address = self.getElectionAddress().getReachableOrOne(); - String addr = formatInetAddr(address); + String addr = self.getElectionAddress().getAllAddresses().stream() + .map(NetUtils::formatInetAddr).collect(Collectors.joining(",")); byte[] addr_bytes = addr.getBytes(); dout.writeInt(addr_bytes.length); dout.write(addr_bytes); @@ -532,7 +540,7 @@ public void run() { private void handleConnection(Socket sock, DataInputStream din) throws IOException { Long sid = null, protocolVersion = null; - InetSocketAddress electionAddr = null; + MultipleAddresses electionAddr = null; try { protocolVersion = din.readLong(); @@ -542,7 +550,7 @@ private void handleConnection(Socket sock, DataInputStream din) try { InitialMessage init = InitialMessage.parse(protocolVersion, din); sid = init.sid; - electionAddr = init.electionAddr; + electionAddr = new MultipleAddresses(init.electionAddr); } catch (InitialMessage.InitialMessageException ex) { LOG.error(ex.toString()); closeSocket(sock); @@ -585,7 +593,7 @@ private void handleConnection(Socket sock, DataInputStream din) closeSocket(sock); if (electionAddr != null) { - connectOne(sid, new MultipleAddresses(electionAddr)); + connectOne(sid, electionAddr); } else { connectOne(sid); } @@ -648,6 +656,10 @@ public void toSend(Long sid, ByteBuffer b) { synchronized boolean connectOne(long sid, MultipleAddresses electionAddr){ if (senderWorkerMap.get(sid) != null) { LOG.debug("There is a connection already for server {}", sid); + // since ZOOKEEPER-3188 we can use multiple election addresses to reach a server. It is possible, that the + // one we are using is already dead and if we need to clean-up, so when we will create a new connection + // then we will choose an other one, which is actually reachable + senderWorkerMap.get(sid).asyncValidateIfSocketIsStillReachable(); return true; } @@ -660,7 +672,7 @@ synchronized boolean connectOne(long sid, MultipleAddresses electionAddr){ sock = new Socket(); } setSockOpts(sock); - sock.connect(electionAddr.getReachableOrOne(), cnxTO); + sock.connect(electionAddr.getReachableAddress(), cnxTO); if (sock instanceof SSLSocket) { SSLSocket sslSock = (SSLSocket) sock; sslSock.startHandshake(); @@ -692,6 +704,10 @@ synchronized boolean connectOne(long sid, MultipleAddresses electionAddr){ + " at election address " + electionAddr, e); closeSocket(sock); return false; + } catch (NoRouteToHostException e) { + LOG.warn("None of the addresses ({}) are reachable for sid {}", electionAddr, sid, e); + closeSocket(sock); + return false; } catch (IOException e) { LOG.warn("Cannot open channel to " + sid + " at election address " + electionAddr, @@ -709,6 +725,10 @@ synchronized boolean connectOne(long sid, MultipleAddresses electionAddr){ synchronized void connectOne(long sid){ if (senderWorkerMap.get(sid) != null) { LOG.debug("There is a connection already for server {}", sid); + // since ZOOKEEPER-3188 we can use multiple election addresses to reach a server. It is possible, that the + // one we are using is already dead and if we need to clean-up, so when we will create a new connection + // then we will choose an other one, which is actually reachable + senderWorkerMap.get(sid).asyncValidateIfSocketIsStillReachable(); return; } synchronized (self.QV_LOCK) { @@ -1100,6 +1120,7 @@ class SendWorker extends ZooKeeperThread { RecvWorker recvWorker; volatile boolean running = true; DataOutputStream dout; + AtomicBoolean ongoingAsyncValidation = new AtomicBoolean(false); /** * An instance of this thread receives messages to send @@ -1239,6 +1260,37 @@ public void run() { this.finish(); LOG.warn("Send worker leaving thread " + " id " + sid + " my id = " + self.getId()); } + + public void asyncValidateIfSocketIsStillReachable() { + if(ongoingAsyncValidation.compareAndSet(false, true)) { + Thread validator = new Thread(() -> { + LOG.debug("validate if destination address is reachable for sid {}", sid); + if(sock != null) { + InetAddress address = sock.getInetAddress(); + try { + if (address.isReachable(500)) { + LOG.debug("destination address {} is reachable for sid {}", address.toString(), sid); + return; + } + } catch (NullPointerException | IOException ignored) { + } + LOG.warn("destination address {} not reachable anymore, shutting down the SendWorker for sid {}", address.toString(), sid); + this.finish(); + } + }); + validator.start(); + try { + validator.join(); + } catch (InterruptedException ignored) { + // we don't care if the validation was interrupted. If SenderWorker is not working, we will + // try to connect and re-validate later + } + ongoingAsyncValidation.set(false); + } else { + LOG.debug("validation of destination address for sid {} is skipped (it is already running)", sid); + } + } + } /** diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java index 84dcedcd6ca..62d66a943c1 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainTest.java @@ -480,8 +480,7 @@ public void testBadPeerAddressInQuorum() throws Exception { LineNumberReader r = new LineNumberReader(new StringReader(os.toString())); String line; boolean found = false; - Pattern p = - Pattern.compile(".*Cannot open channel to .* at election address .*"); + Pattern p = Pattern.compile(".*None of the addresses .* are reachable for sid 2"); while ((line = r.readLine()) != null) { found = p.matcher(line).matches(); if (found) { From 6c4220a0d9eb2f8c299dbdd764fa74683ae2af9b Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Mon, 12 Aug 2019 13:35:35 +0200 Subject: [PATCH 03/14] ZOOKEEPER-3188: fix SendWorker.asyncValidateIfSocketIsStillReachable --- .../zookeeper/server/quorum/QuorumCnxManager.java | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java index e505fb49542..a8488334b3b 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java @@ -1263,13 +1263,14 @@ public void run() { public void asyncValidateIfSocketIsStillReachable() { if(ongoingAsyncValidation.compareAndSet(false, true)) { - Thread validator = new Thread(() -> { + new Thread(() -> { LOG.debug("validate if destination address is reachable for sid {}", sid); if(sock != null) { InetAddress address = sock.getInetAddress(); try { if (address.isReachable(500)) { LOG.debug("destination address {} is reachable for sid {}", address.toString(), sid); + ongoingAsyncValidation.set(false); return; } } catch (NullPointerException | IOException ignored) { @@ -1277,15 +1278,7 @@ public void asyncValidateIfSocketIsStillReachable() { LOG.warn("destination address {} not reachable anymore, shutting down the SendWorker for sid {}", address.toString(), sid); this.finish(); } - }); - validator.start(); - try { - validator.join(); - } catch (InterruptedException ignored) { - // we don't care if the validation was interrupted. If SenderWorker is not working, we will - // try to connect and re-validate later - } - ongoingAsyncValidation.set(false); + }).start(); } else { LOG.debug("validation of destination address for sid {} is skipped (it is already running)", sid); } From 42a52a68859d9b4355edc76c1448a93017cdad5e Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Wed, 14 Aug 2019 13:00:17 +0200 Subject: [PATCH 04/14] ZOOKEEPER-3188: improve based on code review comments --- .../zookeeper/server/admin/Commands.java | 81 ++++++++++--------- .../zookeeper/server/quorum/Leader.java | 2 +- .../server/quorum/QuorumCnxManager.java | 2 +- .../zookeeper/server/admin/CommandsTest.java | 6 ++ .../server/quorum/CnxManagerTest.java | 29 ++++++- 5 files changed, 80 insertions(+), 40 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java index 1996807edd2..97de99a753c 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java @@ -19,10 +19,19 @@ package org.apache.zookeeper.server.admin; import java.net.InetSocketAddress; -import java.util.*; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import java.util.Properties; +import java.util.Set; +import java.util.SortedMap; +import java.util.TreeMap; import java.util.stream.Collectors; -import com.fasterxml.jackson.annotation.JsonAnyGetter; +import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.zookeeper.Environment; import org.apache.zookeeper.Environment.Entry; import org.apache.zookeeper.Version; @@ -36,7 +45,9 @@ import org.apache.zookeeper.server.quorum.FollowerZooKeeperServer; import org.apache.zookeeper.server.quorum.Leader; import org.apache.zookeeper.server.quorum.LeaderZooKeeperServer; +import org.apache.zookeeper.server.quorum.MultipleAddresses; import org.apache.zookeeper.server.quorum.QuorumPeer; +import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType; import org.apache.zookeeper.server.quorum.QuorumZooKeeperServer; import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier; import org.apache.zookeeper.server.quorum.ReadOnlyZooKeeperServer; @@ -620,7 +631,8 @@ public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) CommandResponse response = initializeResponse(); if (zkServer instanceof QuorumZooKeeperServer) { QuorumPeer peer = ((QuorumZooKeeperServer) zkServer).self; - VotingView votingView = new VotingView(peer.getVotingView()); + Map votingView = peer.getVotingView().entrySet().stream() + .collect(Collectors.toMap(Map.Entry::getKey, e -> new QuorumServerView(e.getValue()))); response.put("current_config", votingView); } else { response.put("current_config", Collections.emptyMap()); @@ -628,49 +640,44 @@ public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) return response; } + private static class QuorumServerView { - private static class VotingView { - private final Map view; + @JsonProperty + private List serverAddresses; - VotingView(Map view) { - this.view = view.entrySet().stream() - .filter(e -> e.getValue().addr != null) - .collect(Collectors.toMap(Map.Entry::getKey, - e -> String.format("%s:%s%s", - getMultiAddressString(e.getValue()), - e.getValue().type.equals(QuorumPeer.LearnerType.PARTICIPANT) ? "participant" : "observer", - e.getValue().clientAddr ==null || e.getValue().isClientAddrFromStatic ? "" : - String.format(";%s:%d", - QuorumPeer.QuorumServer.delimitedHostString(e.getValue().clientAddr), - e.getValue().clientAddr.getPort())), - (v1, v2) -> v1, // cannot get duplicates as this straight draws from the other map - TreeMap::new)); - } - - private String getMultiAddressString(QuorumPeer.QuorumServer qs) { - return qs.addr.getAllAddresses().stream() - .map(address -> getSingleAddressString(qs, address)) - .collect(Collectors.joining(",")); - } + @JsonProperty + private List electionAddresses; - private String getSingleAddressString(QuorumPeer.QuorumServer qs, InetSocketAddress address) { - final String addressHostString = address.getHostString(); - final String delimitedHostString = QuorumPeer.QuorumServer.delimitedHostString(address); + @JsonProperty + private String clientAddress; - Optional matchingElectionAddress = qs.electionAddr.getAllAddresses().stream() - .filter(electionAddress -> electionAddress.getHostString().equals(addressHostString)) - .findFirst(); - final String electionPort = matchingElectionAddress.map(e-> ":" + e.getPort()).orElse(""); + @JsonProperty + private String learnerType; - return String.format("%s:%d%s", delimitedHostString, address.getPort(), electionPort); + public QuorumServerView(QuorumPeer.QuorumServer quorumServer) { + this.serverAddresses = getMultiAddressString(quorumServer.addr); + this.electionAddresses = getMultiAddressString(quorumServer.electionAddr); + this.learnerType = quorumServer.type.equals(LearnerType.PARTICIPANT) ? "participant" : "observer"; + this.clientAddress = getAddressString(quorumServer.clientAddr); } - @JsonAnyGetter - public Map getView() { - return view; + private static List getMultiAddressString(MultipleAddresses multipleAddresses) { + if(multipleAddresses == null) { + return Collections.emptyList(); + } + + return multipleAddresses.getAllAddresses().stream() + .map(QuorumServerView::getAddressString) + .collect(Collectors.toList()); } - } + private static String getAddressString(InetSocketAddress address) { + if(address == null) { + return ""; + } + return String.format("%s:%d", QuorumPeer.QuorumServer.delimitedHostString(address), address.getPort()); + } + } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java index b5b1a507300..c991a28fbad 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java @@ -311,7 +311,7 @@ ServerSocket createServerSocket(InetSocketAddress address, boolean portUnificati serverSocket.bind(address); return serverSocket; } catch (BindException e) { - LOG.error("Couldn't bind to " + self.getQuorumAddress(), e); + LOG.error("Couldn't bind to " + address.toString(), e); throw e; } } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java index a8488334b3b..2f8b8ccd69b 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java @@ -114,7 +114,7 @@ public class QuorumCnxManager { /* * Protocol identifier used among peers */ - public static final long PROTOCOL_VERSION = -65536L; + public static final long PROTOCOL_VERSION = -65535L; /* * Max buffer size to be read from the network. diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java index 86b5207ee3b..e432dd22218 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/admin/CommandsTest.java @@ -292,6 +292,12 @@ public void testWatchSummary() throws IOException, InterruptedException { new Field("num_total_watches", Integer.class)); } + @Test + public void testVotingViewCommand() throws IOException, InterruptedException { + testCommand("voting_view", + new Field("current_config", Map.class)); + } + @Test public void testConsCommandSecureOnly() { // Arrange diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java index 25373fae035..400996eee51 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java @@ -29,6 +29,7 @@ import java.nio.ByteBuffer; import java.nio.channels.SocketChannel; import java.util.ArrayList; +import java.util.Arrays; import java.util.Date; import java.util.HashMap; import java.util.Map; @@ -58,6 +59,8 @@ import org.junit.Before; import org.junit.Test; +import static org.junit.Assert.assertEquals; + public class CnxManagerTest extends ZKTestCase { protected static final Logger LOG = LoggerFactory.getLogger(FLENewEpochTest.class); protected static final int THRESHOLD = 4; @@ -608,7 +611,7 @@ public void testInitialMessage() throws Exception { Assert.fail("bad hostport accepted"); } catch (InitialMessage.InitialMessageException ex) {} - // good message + // good message, single election address try { hostport = "10.0.0.2:3888"; @@ -621,6 +624,30 @@ public void testInitialMessage() throws Exception { // now parse it din = new DataInputStream(new ByteArrayInputStream(bos.toByteArray())); msg = InitialMessage.parse(QuorumCnxManager.PROTOCOL_VERSION, din); + assertEquals(new Long(5L), msg.sid); + assertEquals(Arrays.asList(new InetSocketAddress("10.0.0.2", 3888)), msg.electionAddr); + } catch (InitialMessage.InitialMessageException ex) { + Assert.fail(ex.toString()); + } + + // good message, multiple election addresses (ZOOKEEPER-3188) + try { + + hostport = "1.1.1.1:9999,2.2.2.2:8888,3.3.3.3:7777"; + bos = new ByteArrayOutputStream(); + dout = new DataOutputStream(bos); + dout.writeLong(5L); // sid + dout.writeInt(hostport.getBytes().length); + dout.writeBytes(hostport); + + // now parse it + din = new DataInputStream(new ByteArrayInputStream(bos.toByteArray())); + msg = InitialMessage.parse(QuorumCnxManager.PROTOCOL_VERSION, din); + assertEquals(new Long(5L), msg.sid); + assertEquals(Arrays.asList(new InetSocketAddress("1.1.1.1", 9999), + new InetSocketAddress("2.2.2.2", 8888), + new InetSocketAddress("3.3.3.3", 7777)), + msg.electionAddr); } catch (InitialMessage.InitialMessageException ex) { Assert.fail(ex.toString()); } From 5bd1f4e2cddee63ad871b64be36b667128aab6db Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Wed, 14 Aug 2019 14:54:39 +0200 Subject: [PATCH 05/14] ZOOKEEPER-3188: supress spotbugs warning --- .../main/java/org/apache/zookeeper/server/admin/Commands.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java index 97de99a753c..299aa86e837 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/admin/Commands.java @@ -32,6 +32,7 @@ import java.util.stream.Collectors; import com.fasterxml.jackson.annotation.JsonProperty; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import org.apache.zookeeper.Environment; import org.apache.zookeeper.Environment.Entry; import org.apache.zookeeper.Version; @@ -640,6 +641,7 @@ public CommandResponse run(ZooKeeperServer zkServer, Map kwargs) return response; } + @SuppressFBWarnings(value = "URF_UNREAD_FIELD", justification="class is used only for JSON serialization") private static class QuorumServerView { @JsonProperty From da98a8da60c25952b18c0aa556dae65b296a96c9 Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Wed, 14 Aug 2019 15:43:33 +0200 Subject: [PATCH 06/14] ZOOKEEPER-3188: fix JDK-13 warning --- .../org/apache/zookeeper/server/quorum/CnxManagerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java index 400996eee51..57d1f28d06d 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java @@ -624,7 +624,7 @@ public void testInitialMessage() throws Exception { // now parse it din = new DataInputStream(new ByteArrayInputStream(bos.toByteArray())); msg = InitialMessage.parse(QuorumCnxManager.PROTOCOL_VERSION, din); - assertEquals(new Long(5L), msg.sid); + assertEquals(Long.valueOf(5), msg.sid); assertEquals(Arrays.asList(new InetSocketAddress("10.0.0.2", 3888)), msg.electionAddr); } catch (InitialMessage.InitialMessageException ex) { Assert.fail(ex.toString()); @@ -643,7 +643,7 @@ public void testInitialMessage() throws Exception { // now parse it din = new DataInputStream(new ByteArrayInputStream(bos.toByteArray())); msg = InitialMessage.parse(QuorumCnxManager.PROTOCOL_VERSION, din); - assertEquals(new Long(5L), msg.sid); + assertEquals(Long.valueOf(5), msg.sid); assertEquals(Arrays.asList(new InetSocketAddress("1.1.1.1", 9999), new InetSocketAddress("2.2.2.2", 8888), new InetSocketAddress("3.3.3.3", 7777)), From 8713a5bbfc5c63a79db76323eaf2b59a0ba0db3f Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Fri, 4 Oct 2019 13:44:52 +0200 Subject: [PATCH 07/14] ZOOKEEPER-3188: add fixes for PR comments --- .../zookeeper/server/quorum/Leader.java | 3 +- .../zookeeper/server/quorum/Learner.java | 13 ++- .../server/quorum/MultipleAddresses.java | 109 ++++++++---------- 3 files changed, 58 insertions(+), 67 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java index 9f6252248dc..e2319a633ed 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java @@ -452,6 +452,7 @@ public void run() { LOG.error("Interrupted while sleeping. Ignoring exception", ie); } finally { closeSockets(); + executor.shutdownNow(); } } } @@ -522,7 +523,7 @@ private void acceptConnections() throws IOException { try { socket.close(); } catch (IOException e) { - LOG.warn("Error closing socket", e); + LOG.warn("Error closing socket: " + socket, e); } } } 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 22d8045d327..f0befd4e44f 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 @@ -257,11 +257,8 @@ protected void sockConnect(Socket sock, InetSocketAddress addr, int timeout) thr * @param addr - the address of the Peer to connect to. * @throws IOException - if the socket connection fails on the 5th attempt * if there is an authentication failure while connecting to leader - * @throws X509Exception - * @throws InterruptedException */ - protected void connectToLeader(MultipleAddresses addr, String hostname) - throws IOException, InterruptedException { + protected void connectToLeader(MultipleAddresses addr, String hostname) throws IOException { this.leaderAddr = addr; Set addresses = addr.getAllAddresses(); @@ -270,7 +267,13 @@ protected void connectToLeader(MultipleAddresses addr, String hostname) AtomicReference socket = new AtomicReference<>(null); addresses.stream().map(address -> new LeaderConnector(address, socket, latch)).forEach(executor::submit); - latch.await(); + try { + latch.await(); + } catch (InterruptedException e) { + LOG.warn("Interrupted while trying to connect to Leader", e); + } finally { + executor.shutdownNow(); + } if (socket.get() == null) { throw new IOException("Failed connect to " + addr); 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 b5dad0e845b..e57be86c021 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 @@ -18,70 +18,61 @@ package org.apache.zookeeper.server.quorum; +import static java.util.Arrays.asList; import java.io.IOException; import java.net.InetAddress; import java.net.InetSocketAddress; import java.net.NoRouteToHostException; import java.net.UnknownHostException; +import java.time.Duration; +import java.util.Collection; import java.util.Collections; import java.util.List; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicReference; import java.util.stream.Collectors; -import java.util.stream.Stream; /** * This class allows to store several quorum and electing addresses. * * See ZOOKEEPER-3188 for a discussion of this feature. */ -public class MultipleAddresses { - private static final int DEFAULT_TIMEOUT = 100; +public final class MultipleAddresses { + private static final Duration DEFAULT_TIMEOUT = Duration.ofMillis(500); + + private static Set newConcurrentHashSet() { + return Collections.newSetFromMap(new ConcurrentHashMap<>()); + } private Set addresses; - private int timeout; + private final Duration timeout; public MultipleAddresses() { - addresses = Collections.newSetFromMap(new ConcurrentHashMap<>()); - timeout = DEFAULT_TIMEOUT; + this(Collections.emptyList()); } - public MultipleAddresses(List addresses) { + public MultipleAddresses(Collection addresses) { this(addresses, DEFAULT_TIMEOUT); } public MultipleAddresses(InetSocketAddress address) { - this(address, DEFAULT_TIMEOUT); + this(asList(address), DEFAULT_TIMEOUT); } - public MultipleAddresses(List addresses, int timeout) { - this.addresses = Collections.newSetFromMap(new ConcurrentHashMap<>()); + public MultipleAddresses(Collection addresses, Duration timeout) { + this.addresses = newConcurrentHashSet(); this.addresses.addAll(addresses); this.timeout = timeout; } - public MultipleAddresses(InetSocketAddress address, int timeout) { - addresses = Collections.newSetFromMap(new ConcurrentHashMap<>()); - addresses.add(address); - this.timeout = timeout; - } - - public int getTimeout() { - return timeout; - } - - public void setTimeout(int timeout) { - this.timeout = timeout; - } - public boolean isEmpty() { return addresses.isEmpty(); } /** - * Returns all addresses. + * Returns all addresses in an unmodifiable set. * * @return set of all InetSocketAddress */ @@ -121,26 +112,29 @@ public void addAddress(InetSocketAddress address) { } /** - * Returns reachable address. If none is reachable than throws exception. + * Returns a reachable address. If none is reachable than throws exception. + * The function is nondeterministic in the sense that the result of calling this function + * twice with the same set of reachable addresses might lead to different results. * * @return address which is reachable. - * @throws NoRouteToHostException if none address is reachable + * @throws NoRouteToHostException if none of the addresses are reachable */ public InetSocketAddress getReachableAddress() throws NoRouteToHostException { - AtomicReference address = new AtomicReference<>(null); - getInetSocketAddressStream().forEach(addr -> checkIfAddressIsReachableAndSet(addr, address)); - - if (address.get() != null) { - return address.get(); - } else { - throw new NoRouteToHostException("No valid address among " + addresses); - } + // using parallelStream() + findAny() will help to minimize the time spent, but + return addresses.parallelStream() + .filter(this::checkIfAddressIsReachable) + .findAny() + .orElseThrow(() -> new NoRouteToHostException("No valid address among " + addresses)); } /** - * Returns reachable address or first one, if none is reachable. + * 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 + * result of calling this function twice with the same set of reachable addresses might lead + * to different results. * * @return address which is reachable or fist one. + * @throws NoSuchElementException if there is no address registered */ public InetSocketAddress getReachableOrOne() { InetSocketAddress address; @@ -153,37 +147,38 @@ public InetSocketAddress getReachableOrOne() { } /** - * Performs a DNS lookup for addresses. + * Performs a parallel DNS lookup for all addresses. * - * If the DNS lookup fails, than address remain unmodified. + * If the DNS lookup fails, then address remain unmodified. */ public void recreateSocketAddresses() { - Set temp = Collections.newSetFromMap(new ConcurrentHashMap<>()); - temp.addAll(getInetSocketAddressStream().map(this::recreateSocketAddress).collect(Collectors.toSet())); - addresses = temp; + addresses = addresses.parallelStream() + .map(this::recreateSocketAddress) + .collect(Collectors.toCollection(MultipleAddresses::newConcurrentHashSet)); } /** - * Returns first address from set. + * Returns an address from the set. * * @return address from a set. + * @throws NoSuchElementException if there is no address registered */ public InetSocketAddress getOne() { return addresses.iterator().next(); } - private void checkIfAddressIsReachableAndSet(InetSocketAddress address, - AtomicReference reachableAddress) { - for (int i = 0; i < 5 && reachableAddress.get() == null; i++) { - try { - if (address.getAddress().isReachable((i + 1) * timeout)) { - reachableAddress.compareAndSet(null, address); - break; - } - Thread.sleep(timeout); - } catch (NullPointerException | IOException | InterruptedException ignored) { + private boolean checkIfAddressIsReachable(InetSocketAddress address) { + if (address.isUnresolved()) { + return false; + } + try { + if (address.getAddress().isReachable((int) timeout.toMillis())) { + return true; } + } catch (IOException ignored) { + // ignore, we don't really care if we can't reach it for timeout or for IO problems } + return false; } private InetSocketAddress recreateSocketAddress(InetSocketAddress address) { @@ -194,14 +189,6 @@ private InetSocketAddress recreateSocketAddress(InetSocketAddress address) { } } - private Stream getInetSocketAddressStream() { - if (addresses.size() > 1) { - return addresses.parallelStream(); - } else { - return addresses.stream(); - } - } - @Override public boolean equals(Object o) { if (this == o) { From ed31d2ce93497b9249bc80c2bd2aa38b7bab3e9a Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Fri, 4 Oct 2019 14:16:42 +0200 Subject: [PATCH 08/14] ZOOKEEPER-3188: better shutdown for executors (following PR comments) --- .../org/apache/zookeeper/server/quorum/Leader.java | 12 ++++++++++-- .../org/apache/zookeeper/server/quorum/Learner.java | 10 +++++++++- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java index e2319a633ed..e982e7f5f65 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java @@ -47,6 +47,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.stream.Collectors; @@ -449,10 +450,17 @@ public void run() { try { latch.await(); } catch (InterruptedException ie) { - LOG.error("Interrupted while sleeping. Ignoring exception", ie); + LOG.error("Interrupted while sleeping in LearnerCnxAcceptor.", ie); } finally { closeSockets(); - executor.shutdownNow(); + executor.shutdown(); + try { + if (!executor.awaitTermination(1, TimeUnit.SECONDS)) { + LOG.error("not all the LearnerCnxAcceptorHandler terminated properly"); + } + } catch (InterruptedException ie) { + LOG.error("Interrupted while terminating LearnerCnxAcceptor.", ie); + } } } } 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 f0befd4e44f..09c25d6d68b 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 @@ -37,6 +37,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicReference; import javax.net.ssl.SSLSocket; import org.apache.jute.BinaryInputArchive; @@ -272,7 +273,14 @@ protected void connectToLeader(MultipleAddresses addr, String hostname) throws I } catch (InterruptedException e) { LOG.warn("Interrupted while trying to connect to Leader", e); } finally { - executor.shutdownNow(); + executor.shutdown(); + try { + if (!executor.awaitTermination(1, TimeUnit.SECONDS)) { + LOG.error("not all the LeaderConnector terminated properly"); + } + } catch (InterruptedException ie) { + LOG.error("Interrupted while terminating LeaderConnector executor.", ie); + } } if (socket.get() == null) { From a5d6bcb9722bb25f691a9f31e93735274d242100 Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Tue, 8 Oct 2019 15:45:59 +0200 Subject: [PATCH 09/14] ZOOKEEPER-3188: support for dynamic reconfig + add more unit tests --- .../zookeeper/server/quorum/Leader.java | 2 +- .../zookeeper/server/quorum/Learner.java | 6 +- .../server/quorum/LocalPeerBean.java | 4 +- .../server/quorum/MultipleAddresses.java | 2 +- .../server/quorum/QuorumCnxManager.java | 6 +- .../zookeeper/server/quorum/QuorumPeer.java | 9 +- .../server/quorum/QuorumZooKeeperServer.java | 4 +- .../quorum/ReadOnlyZooKeeperServer.java | 4 +- .../server/quorum/RemotePeerBean.java | 4 +- .../server/quorum/CnxManagerTest.java | 2 +- .../zookeeper/server/quorum/LearnerTest.java | 109 ++++- .../QuorumPeerMainMultiAddressTest.java | 423 ++++++++++++++++++ .../apache/zookeeper/test/ReconfigTest.java | 93 +++- 13 files changed, 635 insertions(+), 33 deletions(-) create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java index e982e7f5f65..8af2a949473 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java @@ -434,7 +434,7 @@ class LearnerCnxAcceptor extends ZooKeeperCriticalThread { super("LearnerCnxAcceptor-" + serverSockets.stream() .map(ServerSocket::getLocalSocketAddress) .map(Objects::toString) - .collect(Collectors.joining(",")), + .collect(Collectors.joining("|")), zk.getZooKeeperServerListener()); } 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 09c25d6d68b..cd535d62e26 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 @@ -381,7 +381,11 @@ private Socket connectToLeader() throws IOException, X509Exception, InterruptedE } } - private Socket createSocket() throws X509Exception, IOException { + /** + * Creating a simple or and SSL socket. + * This can be overridden in tests to fake already connected sockets for connectToLeader. + */ + protected Socket createSocket() throws X509Exception, IOException { Socket sock; if (self.isSslQuorum()) { sock = self.getX509Util().createSSLSocket(); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java index 230a7692eba..fa6736f4466 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LocalPeerBean.java @@ -83,7 +83,7 @@ public String getState() { public String getQuorumAddress() { return peer.getQuorumAddress().getAllAddresses().stream().map(NetUtils::formatInetAddr) - .collect(Collectors.joining(",")); + .collect(Collectors.joining("|")); } public int getElectionType() { @@ -92,7 +92,7 @@ public int getElectionType() { public String getElectionAddress() { return peer.getElectionAddress().getAllAddresses().stream().map(NetUtils::formatInetAddr) - .collect(Collectors.joining(",")); + .collect(Collectors.joining("|")); } public String getClientAddress() { 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 e57be86c021..e6d35a529bb 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 @@ -208,6 +208,6 @@ public int hashCode() { @Override public String toString() { - return addresses.stream().map(InetSocketAddress::toString).collect(Collectors.joining(",")); + return addresses.stream().map(InetSocketAddress::toString).collect(Collectors.joining("|")); } } \ No newline at end of file diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java index 37e9dc9d41d..817463d8ce0 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java @@ -238,7 +238,7 @@ public static InitialMessage parse(Long protocolVersion, DataInputStream din) th throw new InitialMessageException("Read only %s bytes out of %s sent by server %s", num_read, remaining, sid); } - String[] addressStrings = new String(b).split(","); + String[] addressStrings = new String(b).split("\\|"); List addresses = new ArrayList<>(addressStrings.length); for (String addr : addressStrings) { @@ -420,7 +420,7 @@ private boolean startConnection(Socket sock, Long sid) throws IOException { dout.writeLong(PROTOCOL_VERSION); dout.writeLong(self.getId()); String addr = self.getElectionAddress().getAllAddresses().stream() - .map(NetUtils::formatInetAddr).collect(Collectors.joining(",")); + .map(NetUtils::formatInetAddr).collect(Collectors.joining("|")); byte[] addr_bytes = addr.getBytes(); dout.writeInt(addr_bytes.length); dout.write(addr_bytes); @@ -935,7 +935,7 @@ public void run() { + "I won't be able to participate in leader " + "election any longer: {}" , self.getElectionAddress().getAllAddresses().stream().map(NetUtils::formatInetAddr) - .collect(Collectors.joining(","))); + .collect(Collectors.joining("|"))); if (socketException.get()) { // After leaving listener thread, the host cannot join the quorum anymore, // this is a severe error that we cannot recover from, so we need to exit diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java index 39ca3781511..39d529e167c 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java @@ -228,13 +228,14 @@ private LearnerType getType(String s) throws ConfigException { private static final String wrongFormat = " does not have the form server_config or server_config;client_config" - + " where server_config is host:port:port or host:port:port:type and client_config is port or host:port"; + + " where server_config is the pipe separated list of host:port:port or host:port:port:type" + + " and client_config is port or host:port"; public QuorumServer(long sid, String addressStr) throws ConfigException { this.id = sid; LearnerType newType = null; String[] serverClientParts = addressStr.split(";"); - String[] serverAddresses = serverClientParts[0].split(","); + String[] serverAddresses = serverClientParts[0].split("\\|"); if (serverClientParts.length == 2) { String[] clientParts = ConfigUtils.getHostAndPort(serverClientParts[1]); @@ -346,7 +347,7 @@ public String toString() { electionAddrList.sort(Comparator.comparing(InetSocketAddress::getHostString)); sw.append(IntStream.range(0, addrList.size()).mapToObj(i -> String.format("%s:%d:%d", delimitedHostString(addrList.get(i)), addrList.get(i).getPort(), electionAddrList.get(i).getPort())) - .collect(Collectors.joining(","))); + .collect(Collectors.joining("|"))); } if (type == LearnerType.OBSERVER) { @@ -833,7 +834,7 @@ public SyncMode getSyncMode() { public void setLeaderAddressAndId(MultipleAddresses addr, long newId) { if (addr != null) { - leaderAddress.set(String.join(",", addr.getAllHostStrings())); + leaderAddress.set(String.join("|", addr.getAllHostStrings())); } else { leaderAddress.set(null); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java index 0b5a77fbd46..f58b751b08e 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumZooKeeperServer.java @@ -176,10 +176,10 @@ public void dumpConf(PrintWriter pwriter) { pwriter.println(self.getElectionType()); pwriter.print("electionPort="); pwriter.println(self.getElectionAddress().getAllPorts() - .stream().map(Objects::toString).collect(Collectors.joining(","))); + .stream().map(Objects::toString).collect(Collectors.joining("|"))); pwriter.print("quorumPort="); pwriter.println(self.getQuorumAddress().getAllPorts() - .stream().map(Objects::toString).collect(Collectors.joining(","))); + .stream().map(Objects::toString).collect(Collectors.joining("|"))); pwriter.print("peerType="); pwriter.println(self.getLearnerType().ordinal()); pwriter.println("membership: "); diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java index 1bcb24cbf21..b1a72c4e972 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ReadOnlyZooKeeperServer.java @@ -173,10 +173,10 @@ public void dumpConf(PrintWriter pwriter) { pwriter.println(self.getElectionType()); pwriter.print("electionPort="); pwriter.println(self.getElectionAddress().getAllPorts() - .stream().map(Objects::toString).collect(Collectors.joining(","))); + .stream().map(Objects::toString).collect(Collectors.joining("|"))); pwriter.print("quorumPort="); pwriter.println(self.getQuorumAddress().getAllPorts() - .stream().map(Objects::toString).collect(Collectors.joining(","))); + .stream().map(Objects::toString).collect(Collectors.joining("|"))); pwriter.print("peerType="); pwriter.println(self.getLearnerType().ordinal()); } diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java index 5f78f3e8917..2d9d7d3dd65 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/RemotePeerBean.java @@ -49,13 +49,13 @@ public boolean isHidden() { public String getQuorumAddress() { return peer.addr.getAllAddresses().stream() .map(address -> String.format("%s:%d", address.getHostString(), address.getPort())) - .collect(Collectors.joining(",")); + .collect(Collectors.joining("|")); } public String getElectionAddress() { return peer.electionAddr.getAllAddresses().stream() .map(address -> String.format("%s:%d", address.getHostString(), address.getPort())) - .collect(Collectors.joining(",")); + .collect(Collectors.joining("|")); } public String getClientAddress() { diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java index 48d0511a7c0..23805444afa 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/CnxManagerTest.java @@ -647,7 +647,7 @@ public void testInitialMessage() throws Exception { // good message, multiple election addresses (ZOOKEEPER-3188) try { - hostport = "1.1.1.1:9999,2.2.2.2:8888,3.3.3.3:7777"; + hostport = "1.1.1.1:9999|2.2.2.2:8888|3.3.3.3:7777"; bos = new ByteArrayOutputStream(); dout = new DataOutputStream(bos); dout.writeLong(5L); // sid diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java index e9fa6125c39..1d121ac8bde 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/LearnerTest.java @@ -18,9 +18,13 @@ package org.apache.zookeeper.server.quorum; +import static java.util.Arrays.asList; +import static java.util.Collections.emptySet; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.BufferedOutputStream; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; @@ -30,10 +34,13 @@ import java.net.InetSocketAddress; import java.net.Socket; import java.util.ArrayList; +import java.util.HashSet; +import java.util.Set; import org.apache.jute.BinaryInputArchive; import org.apache.jute.BinaryOutputArchive; import org.apache.zookeeper.ZKTestCase; import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.common.X509Exception; import org.apache.zookeeper.data.ACL; import org.apache.zookeeper.server.ZKDatabase; import org.apache.zookeeper.server.persistence.FileTxnSnapLog; @@ -71,17 +78,19 @@ static class SimpleLearner extends Learner { } - static class TimeoutLearner extends Learner { + static class TestLearner extends Learner { - int passSocketConnectOnAttempt = 10; - int socketConnectAttempt = 0; - long timeMultiplier = 0; + private int passSocketConnectOnAttempt = 10; + private int socketConnectAttempt = 0; + private long timeMultiplier = 0; + private Socket socketToBeCreated = null; + private Set unreachableAddresses = emptySet(); - public void setTimeMultiplier(long multiplier) { + private void setTimeMultiplier(long multiplier) { timeMultiplier = multiplier; } - public void setPassConnectAttempt(int num) { + private void setPassConnectAttempt(int num) { passSocketConnectOnAttempt = num; } @@ -89,22 +98,39 @@ protected long nanoTime() { return socketConnectAttempt * timeMultiplier; } - protected int getSockConnectAttempt() { + private int getSockConnectAttempt() { return socketConnectAttempt; } + private void setSocketToBeCreated(Socket socketToBeCreated) { + this.socketToBeCreated = socketToBeCreated; + } + + private void setUnreachableAddresses(Set unreachableAddresses) { + this.unreachableAddresses = unreachableAddresses; + } + @Override protected void sockConnect(Socket sock, InetSocketAddress addr, int timeout) throws IOException { - if (++socketConnectAttempt < passSocketConnectOnAttempt) { - throw new IOException("Test injected Socket.connect() error."); + synchronized (this) { + if (++socketConnectAttempt < passSocketConnectOnAttempt || unreachableAddresses.contains(addr)) { + throw new IOException("Test injected Socket.connect() error."); + } } } + @Override + protected Socket createSocket() throws X509Exception, IOException { + if (socketToBeCreated != null) { + return socketToBeCreated; + } + return super.createSocket(); + } } @Test(expected = IOException.class) public void connectionRetryTimeoutTest() throws Exception { - Learner learner = new TimeoutLearner(); + Learner learner = new TestLearner(); learner.self = new QuorumPeer(); learner.self.setTickTime(2000); learner.self.setInitLimit(5); @@ -119,7 +145,7 @@ public void connectionRetryTimeoutTest() throws Exception { @Test public void connectionInitLimitTimeoutTest() throws Exception { - TimeoutLearner learner = new TimeoutLearner(); + TestLearner learner = new TestLearner(); learner.self = new QuorumPeer(); learner.self.setTickTime(2000); learner.self.setInitLimit(5); @@ -144,9 +170,68 @@ public void connectionInitLimitTimeoutTest() throws Exception { } } + @Test + public void shouldTryMultipleAddresses() throws Exception { + TestLearner learner = new TestLearner(); + learner.self = new QuorumPeer(); + learner.self.setTickTime(2000); + learner.self.setInitLimit(5); + learner.self.setSyncLimit(2); + + // this addr won't even be used since we fake the Socket.connect + InetSocketAddress addrA = new InetSocketAddress(1111); + InetSocketAddress addrB = new InetSocketAddress(2222); + InetSocketAddress addrC = new InetSocketAddress(3333); + InetSocketAddress addrD = new InetSocketAddress(4444); + + // we will never pass (don't allow successful socker.connect) during this test + learner.setPassConnectAttempt(100); + + // we expect this to throw an IOException since we're faking socket connect errors every time + try { + learner.connectToLeader(new MultipleAddresses(asList(addrA, addrB, addrC, addrD)), ""); + fail("should have thrown IOException!"); + } catch (IOException e) { + //good, wanted to see the IOException, let's make sure we tried each address 5 times + assertEquals(4 * 5, learner.getSockConnectAttempt()); + } + } + + @Test + public void multipleAddressesSomeAreFailing() throws Exception { + TestLearner learner = new TestLearner(); + learner.self = new QuorumPeer(); + learner.self.setTickTime(2000); + learner.self.setInitLimit(5); + learner.self.setSyncLimit(2); + + // these addresses won't even be used since we fake the Socket.connect + InetSocketAddress addrWorking = new InetSocketAddress(1111); + InetSocketAddress addrBadA = new InetSocketAddress(2222); + InetSocketAddress addrBadB = new InetSocketAddress(3333); + InetSocketAddress addrBadC = new InetSocketAddress(4444); + + // we will emulate socket connection error for each 'bad' address + learner.setUnreachableAddresses(new HashSet<>(asList(addrBadA, addrBadB, addrBadC))); + + // all connection attempts should succeed (if it is not an unreachable address) + learner.setPassConnectAttempt(0); + + // initialize a mock socket, created by the Learner + Socket mockSocket = mock(Socket.class); + when(mockSocket.isConnected()).thenReturn(true); + learner.setSocketToBeCreated(mockSocket); + + + // we expect this to not throw an IOException since there is a single working address + learner.connectToLeader(new MultipleAddresses(asList(addrBadA, addrBadB, addrBadC, addrWorking)), ""); + + assertEquals("Learner connected to the wrong address", learner.getSocket(), mockSocket); + } + @Test public void connectToLearnerMasterLimitTest() throws Exception { - TimeoutLearner learner = new TimeoutLearner(); + TestLearner learner = new TestLearner(); learner.self = new QuorumPeer(); learner.self.setTickTime(2000); learner.self.setInitLimit(2); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java new file mode 100644 index 00000000000..e204d08a027 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java @@ -0,0 +1,423 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.server.quorum; + +import static org.junit.Assert.assertEquals; +import java.io.IOException; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.zookeeper.CreateMode; +import org.apache.zookeeper.DummyWatcher; +import org.apache.zookeeper.KeeperException; +import org.apache.zookeeper.PortAssignment; +import org.apache.zookeeper.ZooDefs; +import org.apache.zookeeper.ZooKeeper; +import org.apache.zookeeper.admin.ZooKeeperAdmin; +import org.apache.zookeeper.test.ClientBase; +import org.apache.zookeeper.test.ReconfigTest; +import org.junit.Before; +import org.junit.Test; + + +public class QuorumPeerMainMultiAddressTest extends QuorumPeerTestBase { + + private static final int FIRST_SERVER = 0; + private static final int SECOND_SERVER = 1; + private static final int THIRD_SERVER = 2; + private static final int FIRST_ADDRESS = 0; + private static final int SECOND_ADDRESS = 1; + private static final String UNREACHABLE_HOST = "invalid.hostname.unreachable.com"; + private static final String IPV6_LOCALHOST = "[0:0:0:0:0:0:0:1]"; + + // IPv4 by default, change to IPV6_LOCALHOST to test with servers binding to IPv6 + private String hostName = "127.0.0.1"; + + private int zNodeId = 0; + + @Before + public void setUp() throws Exception { + ClientBase.setupTestEnv(); + System.setProperty("zookeeper.DigestAuthenticationProvider.superDigest", "super:D/InIHSb7yEEbrWz8b9l71RjZJU="/* password is 'test'*/); + QuorumPeerConfig.setReconfigEnabled(true); + } + + + @Test + public void shouldStartClusterWithMultipleAddresses() throws Exception { + // we have three ZK servers, each server has two quorumPort and two electionPort registered + QuorumConfigBuilder quorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + + // we launch the three servers, each server having the same configuration + QuorumConfigBuilder builderForServer1 = new QuorumConfigBuilder(quorumConfig); + QuorumConfigBuilder builderForServer2 = new QuorumConfigBuilder(quorumConfig); + QuorumConfigBuilder builderForServer3 = new QuorumConfigBuilder(quorumConfig); + launchServers(Arrays.asList(builderForServer1, builderForServer2, builderForServer3)); + + checkIfZooKeeperQuorumWorks(quorumConfig); + } + + + @Test + public void shouldStartClusterWithMultipleAddresses_IPv6() throws Exception { + hostName = IPV6_LOCALHOST; + + shouldStartClusterWithMultipleAddresses(); + } + + + @Test + public void shouldStartClusterWhenSomeAddressesAreUnreachable() throws Exception { + // we have three ZK servers, each server has two quorumPort and two electionPort registered + QuorumConfigBuilder quorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + + // we launch the three servers + // in the config of each server, we misconfigure one of the addresses for the other two servers + QuorumConfigBuilder builderForServer1 = new QuorumConfigBuilder(quorumConfig) + .changeHostName(SECOND_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST) + .changeHostName(THIRD_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST); + + QuorumConfigBuilder builderForServer2 = new QuorumConfigBuilder(quorumConfig) + .changeHostName(FIRST_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST) + .changeHostName(THIRD_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST); + + QuorumConfigBuilder builderForServer3 = new QuorumConfigBuilder(quorumConfig) + .changeHostName(FIRST_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST) + .changeHostName(SECOND_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST); + + launchServers(Arrays.asList(builderForServer1, builderForServer2, builderForServer3)); + + checkIfZooKeeperQuorumWorks(quorumConfig); + } + + + @Test + public void shouldStartClusterWhenSomeAddressesAreUnreachable_IPv6() throws Exception { + hostName = IPV6_LOCALHOST; + + shouldStartClusterWhenSomeAddressesAreUnreachable(); + } + + + @Test + public void shouldReconfigIncrementallyByAddingMoreAddresses() throws Exception { + // we have three ZK servers, each server has two quorumPort and two electionPort registered + QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + + // we launch the three servers, each server should use the same initial config + launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); + + checkIfZooKeeperQuorumWorks(initialQuorumConfig); + + // we create a new config where we add a new address to each server with random available ports + QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(initialQuorumConfig) + .addNewServerAddress(FIRST_SERVER); + + + ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); + + // initiating a new incremental reconfig, by adding new ports to each server + ReconfigTest.reconfig(zkAdmin, newQuorumConfig.buildAsStringList(), null, null, -1); + + checkIfZooKeeperQuorumWorks(newQuorumConfig); + } + + + @Test + public void shouldReconfigIncrementallyByDeletingSomeAddresses() throws Exception { + // we have three ZK servers, each server has three quorumPort and three electionPort registered + QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 3); + + // we launch the three servers, each server should use the same initial config + launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); + + checkIfZooKeeperQuorumWorks(initialQuorumConfig); + + // we create a new config where we delete a few address from each server + QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(initialQuorumConfig) + .deleteLastServerAddress(FIRST_SERVER) + .deleteLastServerAddress(SECOND_SERVER) + .deleteLastServerAddress(SECOND_SERVER) + .deleteLastServerAddress(THIRD_SERVER); + + ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); + + // initiating a new incremental reconfig, by adding new ports to each server + ReconfigTest.reconfig(zkAdmin, newQuorumConfig.buildAsStringList(), null, null, -1); + + checkIfZooKeeperQuorumWorks(newQuorumConfig); + } + + @Test + public void shouldReconfigNonIncrementallyByChangingAllAddresses() throws Exception { + // we have three ZK servers, each server has two quorumPort and two electionPort registered + QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + + // we launch the three servers, each server should use the same initial config + launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); + + checkIfZooKeeperQuorumWorks(initialQuorumConfig); + + // we create a new config where no ports are the same + // each server will have two new quorumPorts and two new electionPorts + QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + + + ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); + + // initiating a new non-incremental reconfig, by adding new ports to each server + ReconfigTest.reconfig(zkAdmin, null, null, newQuorumConfig.buildAsStringList(), -1); + + checkIfZooKeeperQuorumWorks(newQuorumConfig); + } + + + @Test + public void shouldReconfigIncrementally_IPv6() throws Exception { + + hostName = IPV6_LOCALHOST; + + // we have three ZK servers, each server has two quorumPort and two electionPort registered + QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + + // we launch the three servers, each server should use the same initial config + launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); + + checkIfZooKeeperQuorumWorks(initialQuorumConfig); + + // we create a new config where we delete and add a few address for each server + QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(initialQuorumConfig) + .deleteLastServerAddress(FIRST_SERVER) + .deleteLastServerAddress(SECOND_SERVER) + .deleteLastServerAddress(SECOND_SERVER) + .deleteLastServerAddress(THIRD_SERVER) + .addNewServerAddress(SECOND_SERVER) + .addNewServerAddress(THIRD_SERVER); + + ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); + + // initiating a new incremental reconfig, by adding new ports to each server + ReconfigTest.reconfig(zkAdmin, newQuorumConfig.buildAsStringList(), null, null, -1); + + checkIfZooKeeperQuorumWorks(newQuorumConfig); + } + + + private Servers launchServers(List builders) throws IOException, InterruptedException { + + numServers = builders.size(); + + servers = new Servers(); + servers.clientPorts = new int[numServers]; + servers.mt = new MainThread[numServers]; + servers.zk = new ZooKeeper[numServers]; + + for (int i = 0; i < numServers; i++) { + QuorumConfigBuilder quorumConfigBuilder = builders.get(i); + String quorumCfgSection = quorumConfigBuilder.build(); + LOG.info(String.format("starting server %d with quorum config:\n%s", i, quorumCfgSection)); + servers.clientPorts[i] = quorumConfigBuilder.getClientPort(i); + servers.mt[i] = new MainThread(i, servers.clientPorts[i], quorumCfgSection); + servers.mt[i].start(); + servers.restartClient(i, this); + } + + waitForAll(servers, ZooKeeper.States.CONNECTED); + + for (int i = 0; i < numServers; i++) { + servers.zk[i].close(1000); + } + return servers; + } + + private void checkIfZooKeeperQuorumWorks(QuorumConfigBuilder builder) throws IOException, + InterruptedException, KeeperException { + + zNodeId += 1; + String zNodePath = "/foo_" + zNodeId; + ZooKeeper zk = connectToZkServer(builder, FIRST_SERVER); + zk.create(zNodePath, "foobar1".getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + assertEquals(new String(zk.getData(zNodePath, null, null)), "foobar1"); + zk.close(1000); + + + zk = connectToZkServer(builder, SECOND_SERVER); + assertEquals(new String(zk.getData(zNodePath, null, null)), "foobar1"); + zk.close(1000); + + zk = connectToZkServer(builder, THIRD_SERVER); + assertEquals(new String(zk.getData(zNodePath, null, null)), "foobar1"); + zk.close(1000); + + } + + private ZooKeeper connectToZkServer(QuorumConfigBuilder builder, int serverId) throws IOException, InterruptedException { + ServerAddress server = builder.getServerAddress(serverId, FIRST_ADDRESS); + int clientPort = builder.getClientPort(serverId); + ZooKeeper zk = new ZooKeeper(server.getHost() + ":" + clientPort, ClientBase.CONNECTION_TIMEOUT, this); + waitForOne(zk, ZooKeeper.States.CONNECTED); + return zk; + } + + private ZooKeeperAdmin newZooKeeperAdmin( + QuorumConfigBuilder quorumConfig) throws IOException { + ZooKeeperAdmin zkAdmin = new ZooKeeperAdmin( + hostName + ":" + quorumConfig.getClientPort(FIRST_SERVER), + ClientBase.CONNECTION_TIMEOUT, + DummyWatcher.INSTANCE); + zkAdmin.addAuthInfo("digest", "super:test".getBytes()); + return zkAdmin; + } + + + private static class QuorumConfigBuilder { + + // map of (serverId -> clientPort) + private final Map clientIds = new HashMap<>(); + + // map of (serverId -> (ServerAddress=host,quorumPort,electionPort) ) + private final Map> serverAddresses = new HashMap<>(); + private final String hostName; + private final int numberOfServers; + + private QuorumConfigBuilder(String hostName, int numberOfServers, int numberOfServerAddresses) { + this.numberOfServers = numberOfServers; + this.hostName = hostName; + for (int serverId = 0; serverId < numberOfServers; serverId++) { + clientIds.put(serverId, PortAssignment.unique()); + + List addresses = new ArrayList<>(); + serverAddresses.put(serverId, addresses); + + for (int serverAddressId = 0; serverAddressId < numberOfServerAddresses; serverAddressId++) { + addresses.add(new ServerAddress(hostName)); + } + + } + } + + private QuorumConfigBuilder(QuorumConfigBuilder otherBuilder) { + this.numberOfServers = otherBuilder.clientIds.size(); + this.clientIds.putAll(otherBuilder.clientIds); + this.hostName = otherBuilder.hostName; + for (int i : otherBuilder.serverAddresses.keySet()) { + List clonedServerAddresses = otherBuilder.serverAddresses.get(i).stream() + .map(ServerAddress::clone).collect(Collectors.toList()); + this.serverAddresses.put(i, clonedServerAddresses); + } + } + + private int getClientPort(int serverId) { + return clientIds.get(serverId); + } + + private ServerAddress getServerAddress(int serverId, int addressId) { + return serverAddresses.get(serverId).get(addressId); + } + + private QuorumConfigBuilder changeHostName(int serverId, int addressId, String hostName) { + serverAddresses.get(serverId).get(addressId).setHost(hostName); + return this; + } + + private QuorumConfigBuilder changeQuorumPort(int serverId, int addressId, int quorumPort) { + serverAddresses.get(serverId).get(addressId).setQuorumPort(quorumPort); + return this; + } + + private QuorumConfigBuilder changeElectionPort(int serverId, int addressId, int electionPort) { + serverAddresses.get(serverId).get(addressId).setElectionPort(electionPort); + return this; + } + + private QuorumConfigBuilder addNewServerAddress(int serverId) { + serverAddresses.get(serverId).add(new ServerAddress(hostName)); + return this; + } + + private QuorumConfigBuilder deleteLastServerAddress(int serverId) { + serverAddresses.get(serverId).remove(serverAddresses.get(serverId).size() - 1); + return this; + } + + private String build() { + return String.join("\n", buildAsStringList()); + } + + private List buildAsStringList() { + List result = new ArrayList<>(numberOfServers); + + for (int serverId = 0; serverId < numberOfServers; serverId++) { + String s = serverAddresses.get(serverId).stream() + .map(ServerAddress::toString) + .collect(Collectors.joining("|")); + + result.add(String.format("server.%d=%s;%d", serverId, s, clientIds.get(serverId))); + } + + return result; + } + } + + private static class ServerAddress { + private String host; + private int quorumPort; + private int electionPort; + + private ServerAddress(String host) { + this(host, PortAssignment.unique(), PortAssignment.unique()); + + } + + private ServerAddress(String host, int quorumPort, int electionPort) { + this.host = host; + this.quorumPort = quorumPort; + this.electionPort = electionPort; + } + + private String getHost() { + return host; + } + + private void setHost(String host) { + this.host = host; + } + + private void setQuorumPort(int quorumPort) { + this.quorumPort = quorumPort; + } + + private void setElectionPort(int electionPort) { + this.electionPort = electionPort; + } + + @Override + public ServerAddress clone() { + return new ServerAddress(host, quorumPort, electionPort); + } + + @Override + public String toString() { + return String.format("%s:%d:%d", host, quorumPort, electionPort); + } + } +} diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java index 70ac34f0fd5..94de1f4d192 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java @@ -18,7 +18,10 @@ package org.apache.zookeeper.test; +import static java.lang.Integer.parseInt; +import static java.lang.String.format; import static java.net.InetAddress.getLoopbackAddress; +import static java.util.stream.Collectors.toList; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; @@ -29,8 +32,14 @@ import java.net.ServerSocket; import java.net.UnknownHostException; import java.util.ArrayList; +import java.util.Arrays; +import java.util.HashMap; +import java.util.HashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; import org.apache.zookeeper.AsyncCallback.DataCallback; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.DummyWatcher; @@ -102,14 +111,24 @@ public static String reconfig( } String configStr = new String(config); + List currentServerConfigs = Arrays.stream(configStr.split("\n")) + .map(String::trim) + .filter(s->s.startsWith("server")) + .map(ServerConfigLine::new) + .collect(toList()); + if (joiningServers != null) { for (String joiner : joiningServers) { - assertTrue(configStr.contains(joiner)); + ServerConfigLine joinerServerConfigLine = new ServerConfigLine(joiner); + + String errorMessage = format("expected joiner config \"%s\" not found in current config:\n%s", joiner, configStr); + assertTrue(errorMessage, currentServerConfigs.stream().anyMatch(c -> c.equals(joinerServerConfigLine))); } } if (leavingServers != null) { for (String leaving : leavingServers) { - assertFalse(configStr.contains("server.".concat(leaving))); + String errorMessage = format("leaving server \"%s\" not removed from config: \n%s", leaving, configStr); + assertFalse(errorMessage, configStr.contains(format("server.%s=", leaving))); } } @@ -1152,4 +1171,74 @@ private void assertRemotePeerMXBeanAttributes(QuorumServer qs, String beanName) getAddrPortFromBean(beanName, "QuorumAddress")); } + + /* + * A helper class to parse / compare server address config lines. + * Example: server.1=127.0.0.1:11228:11231|127.0.0.1:11230:11229:participant;0.0.0.0:11227 + */ + private static class ServerConfigLine { + private final int serverId; + private Integer clientPort; + + // hostName -> + private final Map> quorumPorts = new HashMap<>(); + + // hostName -> + private final Map> electionPorts = new HashMap<>(); + + private ServerConfigLine(String configLine) { + String[] parts = configLine.trim().split("="); + serverId = parseInt(parts[0].split("\\.")[1]); + String[] serverConfig = parts[1].split(";"); + String[] serverAddresses = serverConfig[0].split("\\|"); + if (serverConfig.length > 1) { + String[] clientParts = serverConfig[1].split(":"); + if (clientParts.length > 1) { + clientPort = parseInt(clientParts[1]); + } else { + clientPort = parseInt(clientParts[0]); + } + } + + for (String addr : serverAddresses) { + // addr like: 127.0.0.1:11230:11229:participant or [0:0:0:0:0:0:0:1]:11346:11347 + String serverHost; + String[] ports; + if (addr.contains("[")) { + serverHost = addr.substring(1, addr.indexOf("]")); + ports = addr.substring(addr.indexOf("]") + 2).split(":"); + } else { + serverHost = addr.substring(0, addr.indexOf(":")); + ports = addr.substring(addr.indexOf(":") + 1).split(":"); + } + + quorumPorts.computeIfAbsent(serverHost, k -> new HashSet<>()).add(parseInt(ports[0])); + if (ports.length > 1) { + electionPorts.computeIfAbsent(serverHost, k -> new HashSet<>()).add(parseInt(ports[1])); + } + } + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + ServerConfigLine that = (ServerConfigLine) o; + return serverId == that.serverId + && Objects.equals(clientPort, that.clientPort) + && quorumPorts.equals(that.quorumPorts) + && electionPorts.equals(that.electionPorts); + } + + @Override + public int hashCode() { + return Objects.hash(serverId, clientPort, quorumPorts, electionPorts); + } + } + + } From 2eedf26879698725a52cdf093d708be084083d9b Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Wed, 9 Oct 2019 12:38:28 +0200 Subject: [PATCH 10/14] ZOOKEEPER-3188: fix PR commits; handle case when Leader can not bind to port on startup --- .../zookeeper/server/quorum/Leader.java | 21 +- .../QuorumPeerMainMultiAddressTest.java | 213 ++++-------------- .../quorum/QuorumServerConfigBuilder.java | 166 ++++++++++++++ 3 files changed, 222 insertions(+), 178 deletions(-) create mode 100644 zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumServerConfigBuilder.java diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java index 8af2a949473..8036e92456f 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java @@ -24,7 +24,6 @@ import java.io.DataInputStream; import java.io.DataOutputStream; import java.io.IOException; -import java.net.BindException; import java.net.InetSocketAddress; import java.net.ServerSocket; import java.net.Socket; @@ -40,6 +39,7 @@ import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; @@ -290,15 +290,20 @@ public boolean isQuorumSynced(QuorumVerifier qv) { addresses = self.getQuorumAddress().getAllAddresses(); } - for (InetSocketAddress address : addresses) { - serverSockets.add(createServerSocket(address, self.shouldUsePortUnification(), self.isSslQuorum())); + addresses.stream() + .map(address -> createServerSocket(address, self.shouldUsePortUnification(), self.isSslQuorum())) + .filter(Optional::isPresent) + .map(Optional::get) + .forEach(serverSockets::add); + + if (serverSockets.isEmpty()) { + throw new IOException("Leader failed to initialize any of the following sockets: " + addresses); } this.zk = zk; } - ServerSocket createServerSocket(InetSocketAddress address, boolean portUnification, boolean sslQuorum) - throws IOException { + Optional createServerSocket(InetSocketAddress address, boolean portUnification, boolean sslQuorum) { ServerSocket serverSocket; try { if (portUnification || sslQuorum) { @@ -308,11 +313,11 @@ ServerSocket createServerSocket(InetSocketAddress address, boolean portUnificati } serverSocket.setReuseAddress(true); serverSocket.bind(address); - return serverSocket; - } catch (BindException e) { + return Optional.of(serverSocket); + } catch (IOException e) { LOG.error("Couldn't bind to " + address.toString(), e); - throw e; } + return Optional.empty(); } /** diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java index e204d08a027..9e263ffd979 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java @@ -20,21 +20,17 @@ import static org.junit.Assert.assertEquals; import java.io.IOException; -import java.util.ArrayList; import java.util.Arrays; -import java.util.HashMap; import java.util.List; -import java.util.Map; -import java.util.stream.Collectors; import org.apache.zookeeper.CreateMode; import org.apache.zookeeper.DummyWatcher; import org.apache.zookeeper.KeeperException; -import org.apache.zookeeper.PortAssignment; import org.apache.zookeeper.ZooDefs; import org.apache.zookeeper.ZooKeeper; import org.apache.zookeeper.admin.ZooKeeperAdmin; import org.apache.zookeeper.test.ClientBase; import org.apache.zookeeper.test.ReconfigTest; +import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -59,18 +55,27 @@ public void setUp() throws Exception { ClientBase.setupTestEnv(); System.setProperty("zookeeper.DigestAuthenticationProvider.superDigest", "super:D/InIHSb7yEEbrWz8b9l71RjZJU="/* password is 'test'*/); QuorumPeerConfig.setReconfigEnabled(true); + + // just to get rid of the unrelated 'InstanceAlreadyExistsException' in the logs + System.setProperty("zookeeper.jmx.log4j.disable", "true"); + } + + @After + public void tearDown() throws Exception { + super.tearDown(); + System.clearProperty("zookeeper.jmx.log4j.disable"); } @Test public void shouldStartClusterWithMultipleAddresses() throws Exception { // we have three ZK servers, each server has two quorumPort and two electionPort registered - QuorumConfigBuilder quorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + QuorumServerConfigBuilder quorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2); // we launch the three servers, each server having the same configuration - QuorumConfigBuilder builderForServer1 = new QuorumConfigBuilder(quorumConfig); - QuorumConfigBuilder builderForServer2 = new QuorumConfigBuilder(quorumConfig); - QuorumConfigBuilder builderForServer3 = new QuorumConfigBuilder(quorumConfig); + QuorumServerConfigBuilder builderForServer1 = new QuorumServerConfigBuilder(quorumConfig); + QuorumServerConfigBuilder builderForServer2 = new QuorumServerConfigBuilder(quorumConfig); + QuorumServerConfigBuilder builderForServer3 = new QuorumServerConfigBuilder(quorumConfig); launchServers(Arrays.asList(builderForServer1, builderForServer2, builderForServer3)); checkIfZooKeeperQuorumWorks(quorumConfig); @@ -88,22 +93,22 @@ public void shouldStartClusterWithMultipleAddresses_IPv6() throws Exception { @Test public void shouldStartClusterWhenSomeAddressesAreUnreachable() throws Exception { // we have three ZK servers, each server has two quorumPort and two electionPort registered - QuorumConfigBuilder quorumConfig = new QuorumConfigBuilder(hostName, 3, 2); - - // we launch the three servers - // in the config of each server, we misconfigure one of the addresses for the other two servers - QuorumConfigBuilder builderForServer1 = new QuorumConfigBuilder(quorumConfig) + // in the config we misconfigure one of the addresses for each servers + QuorumServerConfigBuilder quorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2) + .changeHostName(FIRST_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST) .changeHostName(SECOND_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST) .changeHostName(THIRD_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST); - QuorumConfigBuilder builderForServer2 = new QuorumConfigBuilder(quorumConfig) - .changeHostName(FIRST_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST) - .changeHostName(THIRD_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST); - - QuorumConfigBuilder builderForServer3 = new QuorumConfigBuilder(quorumConfig) - .changeHostName(FIRST_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST) - .changeHostName(SECOND_SERVER, SECOND_ADDRESS, UNREACHABLE_HOST); + // we prepare the same initial config for all the three servers + QuorumServerConfigBuilder builderForServer1 = new QuorumServerConfigBuilder(quorumConfig); + QuorumServerConfigBuilder builderForServer2 = new QuorumServerConfigBuilder(quorumConfig); + QuorumServerConfigBuilder builderForServer3 = new QuorumServerConfigBuilder(quorumConfig); + // we test here: + // - if the Leader can bind to the correct address and not die with BindException or + // SocketException for trying to bind to a wrong address / port + // - if the ZK server can 'select' the correct address to connect when trying to form a quorum + // with the other servers launchServers(Arrays.asList(builderForServer1, builderForServer2, builderForServer3)); checkIfZooKeeperQuorumWorks(quorumConfig); @@ -121,7 +126,7 @@ public void shouldStartClusterWhenSomeAddressesAreUnreachable_IPv6() throws Exce @Test public void shouldReconfigIncrementallyByAddingMoreAddresses() throws Exception { // we have three ZK servers, each server has two quorumPort and two electionPort registered - QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + QuorumServerConfigBuilder initialQuorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2); // we launch the three servers, each server should use the same initial config launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); @@ -129,7 +134,7 @@ public void shouldReconfigIncrementallyByAddingMoreAddresses() throws Exception checkIfZooKeeperQuorumWorks(initialQuorumConfig); // we create a new config where we add a new address to each server with random available ports - QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(initialQuorumConfig) + QuorumServerConfigBuilder newQuorumConfig = new QuorumServerConfigBuilder(initialQuorumConfig) .addNewServerAddress(FIRST_SERVER); @@ -145,7 +150,7 @@ public void shouldReconfigIncrementallyByAddingMoreAddresses() throws Exception @Test public void shouldReconfigIncrementallyByDeletingSomeAddresses() throws Exception { // we have three ZK servers, each server has three quorumPort and three electionPort registered - QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 3); + QuorumServerConfigBuilder initialQuorumConfig = new QuorumServerConfigBuilder(hostName, 3, 3); // we launch the three servers, each server should use the same initial config launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); @@ -153,7 +158,7 @@ public void shouldReconfigIncrementallyByDeletingSomeAddresses() throws Exceptio checkIfZooKeeperQuorumWorks(initialQuorumConfig); // we create a new config where we delete a few address from each server - QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(initialQuorumConfig) + QuorumServerConfigBuilder newQuorumConfig = new QuorumServerConfigBuilder(initialQuorumConfig) .deleteLastServerAddress(FIRST_SERVER) .deleteLastServerAddress(SECOND_SERVER) .deleteLastServerAddress(SECOND_SERVER) @@ -170,7 +175,7 @@ public void shouldReconfigIncrementallyByDeletingSomeAddresses() throws Exceptio @Test public void shouldReconfigNonIncrementallyByChangingAllAddresses() throws Exception { // we have three ZK servers, each server has two quorumPort and two electionPort registered - QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + QuorumServerConfigBuilder initialQuorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2); // we launch the three servers, each server should use the same initial config launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); @@ -179,7 +184,7 @@ public void shouldReconfigNonIncrementallyByChangingAllAddresses() throws Except // we create a new config where no ports are the same // each server will have two new quorumPorts and two new electionPorts - QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + QuorumServerConfigBuilder newQuorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2); ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); @@ -197,7 +202,7 @@ public void shouldReconfigIncrementally_IPv6() throws Exception { hostName = IPV6_LOCALHOST; // we have three ZK servers, each server has two quorumPort and two electionPort registered - QuorumConfigBuilder initialQuorumConfig = new QuorumConfigBuilder(hostName, 3, 2); + QuorumServerConfigBuilder initialQuorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2); // we launch the three servers, each server should use the same initial config launchServers(Arrays.asList(initialQuorumConfig, initialQuorumConfig, initialQuorumConfig)); @@ -205,7 +210,7 @@ public void shouldReconfigIncrementally_IPv6() throws Exception { checkIfZooKeeperQuorumWorks(initialQuorumConfig); // we create a new config where we delete and add a few address for each server - QuorumConfigBuilder newQuorumConfig = new QuorumConfigBuilder(initialQuorumConfig) + QuorumServerConfigBuilder newQuorumConfig = new QuorumServerConfigBuilder(initialQuorumConfig) .deleteLastServerAddress(FIRST_SERVER) .deleteLastServerAddress(SECOND_SERVER) .deleteLastServerAddress(SECOND_SERVER) @@ -222,7 +227,7 @@ public void shouldReconfigIncrementally_IPv6() throws Exception { } - private Servers launchServers(List builders) throws IOException, InterruptedException { + private void launchServers(List builders) throws IOException, InterruptedException { numServers = builders.size(); @@ -232,10 +237,10 @@ private Servers launchServers(List builders) throws IOExcep servers.zk = new ZooKeeper[numServers]; for (int i = 0; i < numServers; i++) { - QuorumConfigBuilder quorumConfigBuilder = builders.get(i); - String quorumCfgSection = quorumConfigBuilder.build(); + QuorumServerConfigBuilder quorumServerConfigBuilder = builders.get(i); + String quorumCfgSection = quorumServerConfigBuilder.build(); LOG.info(String.format("starting server %d with quorum config:\n%s", i, quorumCfgSection)); - servers.clientPorts[i] = quorumConfigBuilder.getClientPort(i); + servers.clientPorts[i] = quorumServerConfigBuilder.getClientPort(i); servers.mt[i] = new MainThread(i, servers.clientPorts[i], quorumCfgSection); servers.mt[i].start(); servers.restartClient(i, this); @@ -244,12 +249,11 @@ private Servers launchServers(List builders) throws IOExcep waitForAll(servers, ZooKeeper.States.CONNECTED); for (int i = 0; i < numServers; i++) { - servers.zk[i].close(1000); + servers.zk[i].close(5000); } - return servers; } - private void checkIfZooKeeperQuorumWorks(QuorumConfigBuilder builder) throws IOException, + private void checkIfZooKeeperQuorumWorks(QuorumServerConfigBuilder builder) throws IOException, InterruptedException, KeeperException { zNodeId += 1; @@ -270,8 +274,8 @@ private void checkIfZooKeeperQuorumWorks(QuorumConfigBuilder builder) throws IOE } - private ZooKeeper connectToZkServer(QuorumConfigBuilder builder, int serverId) throws IOException, InterruptedException { - ServerAddress server = builder.getServerAddress(serverId, FIRST_ADDRESS); + private ZooKeeper connectToZkServer(QuorumServerConfigBuilder builder, int serverId) throws IOException, InterruptedException { + QuorumServerConfigBuilder.ServerAddress server = builder.getServerAddress(serverId, FIRST_ADDRESS); int clientPort = builder.getClientPort(serverId); ZooKeeper zk = new ZooKeeper(server.getHost() + ":" + clientPort, ClientBase.CONNECTION_TIMEOUT, this); waitForOne(zk, ZooKeeper.States.CONNECTED); @@ -279,7 +283,7 @@ private ZooKeeper connectToZkServer(QuorumConfigBuilder builder, int serverId) t } private ZooKeeperAdmin newZooKeeperAdmin( - QuorumConfigBuilder quorumConfig) throws IOException { + QuorumServerConfigBuilder quorumConfig) throws IOException { ZooKeeperAdmin zkAdmin = new ZooKeeperAdmin( hostName + ":" + quorumConfig.getClientPort(FIRST_SERVER), ClientBase.CONNECTION_TIMEOUT, @@ -289,135 +293,4 @@ private ZooKeeperAdmin newZooKeeperAdmin( } - private static class QuorumConfigBuilder { - - // map of (serverId -> clientPort) - private final Map clientIds = new HashMap<>(); - - // map of (serverId -> (ServerAddress=host,quorumPort,electionPort) ) - private final Map> serverAddresses = new HashMap<>(); - private final String hostName; - private final int numberOfServers; - - private QuorumConfigBuilder(String hostName, int numberOfServers, int numberOfServerAddresses) { - this.numberOfServers = numberOfServers; - this.hostName = hostName; - for (int serverId = 0; serverId < numberOfServers; serverId++) { - clientIds.put(serverId, PortAssignment.unique()); - - List addresses = new ArrayList<>(); - serverAddresses.put(serverId, addresses); - - for (int serverAddressId = 0; serverAddressId < numberOfServerAddresses; serverAddressId++) { - addresses.add(new ServerAddress(hostName)); - } - - } - } - - private QuorumConfigBuilder(QuorumConfigBuilder otherBuilder) { - this.numberOfServers = otherBuilder.clientIds.size(); - this.clientIds.putAll(otherBuilder.clientIds); - this.hostName = otherBuilder.hostName; - for (int i : otherBuilder.serverAddresses.keySet()) { - List clonedServerAddresses = otherBuilder.serverAddresses.get(i).stream() - .map(ServerAddress::clone).collect(Collectors.toList()); - this.serverAddresses.put(i, clonedServerAddresses); - } - } - - private int getClientPort(int serverId) { - return clientIds.get(serverId); - } - - private ServerAddress getServerAddress(int serverId, int addressId) { - return serverAddresses.get(serverId).get(addressId); - } - - private QuorumConfigBuilder changeHostName(int serverId, int addressId, String hostName) { - serverAddresses.get(serverId).get(addressId).setHost(hostName); - return this; - } - - private QuorumConfigBuilder changeQuorumPort(int serverId, int addressId, int quorumPort) { - serverAddresses.get(serverId).get(addressId).setQuorumPort(quorumPort); - return this; - } - - private QuorumConfigBuilder changeElectionPort(int serverId, int addressId, int electionPort) { - serverAddresses.get(serverId).get(addressId).setElectionPort(electionPort); - return this; - } - - private QuorumConfigBuilder addNewServerAddress(int serverId) { - serverAddresses.get(serverId).add(new ServerAddress(hostName)); - return this; - } - - private QuorumConfigBuilder deleteLastServerAddress(int serverId) { - serverAddresses.get(serverId).remove(serverAddresses.get(serverId).size() - 1); - return this; - } - - private String build() { - return String.join("\n", buildAsStringList()); - } - - private List buildAsStringList() { - List result = new ArrayList<>(numberOfServers); - - for (int serverId = 0; serverId < numberOfServers; serverId++) { - String s = serverAddresses.get(serverId).stream() - .map(ServerAddress::toString) - .collect(Collectors.joining("|")); - - result.add(String.format("server.%d=%s;%d", serverId, s, clientIds.get(serverId))); - } - - return result; - } - } - - private static class ServerAddress { - private String host; - private int quorumPort; - private int electionPort; - - private ServerAddress(String host) { - this(host, PortAssignment.unique(), PortAssignment.unique()); - - } - - private ServerAddress(String host, int quorumPort, int electionPort) { - this.host = host; - this.quorumPort = quorumPort; - this.electionPort = electionPort; - } - - private String getHost() { - return host; - } - - private void setHost(String host) { - this.host = host; - } - - private void setQuorumPort(int quorumPort) { - this.quorumPort = quorumPort; - } - - private void setElectionPort(int electionPort) { - this.electionPort = electionPort; - } - - @Override - public ServerAddress clone() { - return new ServerAddress(host, quorumPort, electionPort); - } - - @Override - public String toString() { - return String.format("%s:%d:%d", host, quorumPort, electionPort); - } - } } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumServerConfigBuilder.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumServerConfigBuilder.java new file mode 100644 index 00000000000..6bbb6e4e4f7 --- /dev/null +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumServerConfigBuilder.java @@ -0,0 +1,166 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.zookeeper.server.quorum; + +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.stream.Collectors; +import org.apache.zookeeper.PortAssignment; + + +/* + * Helper class to build / change Quorum Config String, like: + * server.1=127.0.0.1:11228:11231|127.0.0.1:11230:11229;11227 + * server.2=127.0.0.1:11338:11331|127.0.0.1:11330:11229;11337 + * + */ +public class QuorumServerConfigBuilder { + + // map of (serverId -> clientPort) + private final Map clientIds = new HashMap<>(); + + // map of (serverId -> (ServerAddress=host,quorumPort,electionPort) ) + private final Map> serverAddresses = new HashMap<>(); + private final String hostName; + private final int numberOfServers; + + public QuorumServerConfigBuilder(String hostName, int numberOfServers, int numberOfServerAddresses) { + this.numberOfServers = numberOfServers; + this.hostName = hostName; + for (int serverId = 0; serverId < numberOfServers; serverId++) { + clientIds.put(serverId, PortAssignment.unique()); + + List addresses = new ArrayList<>(); + serverAddresses.put(serverId, addresses); + + for (int serverAddressId = 0; serverAddressId < numberOfServerAddresses; serverAddressId++) { + addresses.add(new ServerAddress(hostName)); + } + + } + } + + public QuorumServerConfigBuilder(QuorumServerConfigBuilder otherBuilder) { + this.numberOfServers = otherBuilder.clientIds.size(); + this.clientIds.putAll(otherBuilder.clientIds); + this.hostName = otherBuilder.hostName; + for (int i : otherBuilder.serverAddresses.keySet()) { + List clonedServerAddresses = otherBuilder.serverAddresses.get(i).stream() + .map(ServerAddress::clone).collect(Collectors.toList()); + this.serverAddresses.put(i, clonedServerAddresses); + } + } + + public int getClientPort(int serverId) { + return clientIds.get(serverId); + } + + public ServerAddress getServerAddress(int serverId, int addressId) { + return serverAddresses.get(serverId).get(addressId); + } + + public QuorumServerConfigBuilder changeHostName(int serverId, int addressId, String hostName) { + serverAddresses.get(serverId).get(addressId).setHost(hostName); + return this; + } + + public QuorumServerConfigBuilder changeQuorumPort(int serverId, int addressId, int quorumPort) { + serverAddresses.get(serverId).get(addressId).setQuorumPort(quorumPort); + return this; + } + + public QuorumServerConfigBuilder changeElectionPort(int serverId, int addressId, int electionPort) { + serverAddresses.get(serverId).get(addressId).setElectionPort(electionPort); + return this; + } + + public QuorumServerConfigBuilder addNewServerAddress(int serverId) { + serverAddresses.get(serverId).add(new ServerAddress(hostName)); + return this; + } + + public QuorumServerConfigBuilder deleteLastServerAddress(int serverId) { + serverAddresses.get(serverId).remove(serverAddresses.get(serverId).size() - 1); + return this; + } + + public String build() { + return String.join("\n", buildAsStringList()); + } + + public List buildAsStringList() { + List result = new ArrayList<>(numberOfServers); + + for (int serverId = 0; serverId < numberOfServers; serverId++) { + String s = serverAddresses.get(serverId).stream() + .map(ServerAddress::toString) + .collect(Collectors.joining("|")); + + result.add(String.format("server.%d=%s;%d", serverId, s, clientIds.get(serverId))); + } + + return result; + } + + public static class ServerAddress { + private String host; + private int quorumPort; + private int electionPort; + + private ServerAddress(String host) { + this(host, PortAssignment.unique(), PortAssignment.unique()); + + } + + private ServerAddress(String host, int quorumPort, int electionPort) { + this.host = host; + this.quorumPort = quorumPort; + this.electionPort = electionPort; + } + + public String getHost() { + return host; + } + + private void setHost(String host) { + this.host = host; + } + + private void setQuorumPort(int quorumPort) { + this.quorumPort = quorumPort; + } + + private void setElectionPort(int electionPort) { + this.electionPort = electionPort; + } + + @Override + public ServerAddress clone() { + return new ServerAddress(host, quorumPort, electionPort); + } + + @Override + public String toString() { + return String.format("%s:%d:%d", host, quorumPort, electionPort); + } + } +} + From e232c55daf99e1abc3cfdac387248aaa5a4eeeb9 Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Mon, 4 Nov 2019 12:54:14 -0600 Subject: [PATCH 11/14] ZOOKEEPER-3188: fix flaky unit MultiAddress unit test --- .../QuorumPeerMainMultiAddressTest.java | 25 ++++++++++++------- .../apache/zookeeper/test/ReconfigTest.java | 1 + 2 files changed, 17 insertions(+), 9 deletions(-) diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java index 9e263ffd979..ae3557315d0 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumPeerMainMultiAddressTest.java @@ -140,7 +140,7 @@ public void shouldReconfigIncrementallyByAddingMoreAddresses() throws Exception ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); - // initiating a new incremental reconfig, by adding new ports to each server + // initiating a new incremental reconfig, by using the updated ports ReconfigTest.reconfig(zkAdmin, newQuorumConfig.buildAsStringList(), null, null, -1); checkIfZooKeeperQuorumWorks(newQuorumConfig); @@ -166,14 +166,14 @@ public void shouldReconfigIncrementallyByDeletingSomeAddresses() throws Exceptio ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); - // initiating a new incremental reconfig, by adding new ports to each server + // initiating a new incremental reconfig, by using the updated ports ReconfigTest.reconfig(zkAdmin, newQuorumConfig.buildAsStringList(), null, null, -1); checkIfZooKeeperQuorumWorks(newQuorumConfig); } @Test - public void shouldReconfigNonIncrementallyByChangingAllAddresses() throws Exception { + public void shouldReconfigNonIncrementally() throws Exception { // we have three ZK servers, each server has two quorumPort and two electionPort registered QuorumServerConfigBuilder initialQuorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2); @@ -182,14 +182,18 @@ public void shouldReconfigNonIncrementallyByChangingAllAddresses() throws Except checkIfZooKeeperQuorumWorks(initialQuorumConfig); - // we create a new config where no ports are the same - // each server will have two new quorumPorts and two new electionPorts - QuorumServerConfigBuilder newQuorumConfig = new QuorumServerConfigBuilder(hostName, 3, 2); - + // we create a new config where we delete and add a few address for each server + QuorumServerConfigBuilder newQuorumConfig = new QuorumServerConfigBuilder(initialQuorumConfig) + .deleteLastServerAddress(FIRST_SERVER) + .deleteLastServerAddress(SECOND_SERVER) + .deleteLastServerAddress(SECOND_SERVER) + .deleteLastServerAddress(THIRD_SERVER) + .addNewServerAddress(SECOND_SERVER) + .addNewServerAddress(THIRD_SERVER); ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); - // initiating a new non-incremental reconfig, by adding new ports to each server + // initiating a new non-incremental reconfig, by using the updated ports ReconfigTest.reconfig(zkAdmin, null, null, newQuorumConfig.buildAsStringList(), -1); checkIfZooKeeperQuorumWorks(newQuorumConfig); @@ -220,7 +224,7 @@ public void shouldReconfigIncrementally_IPv6() throws Exception { ZooKeeperAdmin zkAdmin = newZooKeeperAdmin(initialQuorumConfig); - // initiating a new incremental reconfig, by adding new ports to each server + // initiating a new incremental reconfig, by using the updated ports ReconfigTest.reconfig(zkAdmin, newQuorumConfig.buildAsStringList(), null, null, -1); checkIfZooKeeperQuorumWorks(newQuorumConfig); @@ -256,6 +260,7 @@ private void launchServers(List builders) throws IOEx private void checkIfZooKeeperQuorumWorks(QuorumServerConfigBuilder builder) throws IOException, InterruptedException, KeeperException { + LOG.info("starting to verify if Quorum works"); zNodeId += 1; String zNodePath = "/foo_" + zNodeId; ZooKeeper zk = connectToZkServer(builder, FIRST_SERVER); @@ -272,6 +277,8 @@ private void checkIfZooKeeperQuorumWorks(QuorumServerConfigBuilder builder) thro assertEquals(new String(zk.getData(zNodePath, null, null)), "foobar1"); zk.close(1000); + LOG.info("Quorum verification finished successfully"); + } private ZooKeeper connectToZkServer(QuorumServerConfigBuilder builder, int serverId) throws IOException, InterruptedException { diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java index 94de1f4d192..8642b368d9c 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/test/ReconfigTest.java @@ -94,6 +94,7 @@ public static String reconfig( long fromConfig) throws KeeperException, InterruptedException { byte[] config = null; String failure = null; + LOG.info("reconfig initiated by the test"); for (int j = 0; j < 30; j++) { try { config = zkAdmin.reconfigure(joiningServers, leavingServers, newMembers, fromConfig, new Stat()); From 0f95678ca7e68d22c5e547bdecac58b680537c2d Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Wed, 13 Nov 2019 11:30:03 -0800 Subject: [PATCH 12/14] ZOOKEEPER-3188: skip unreachable addresses when Learner connects to Leader --- .../zookeeper/server/quorum/Learner.java | 23 +++++++------ .../server/quorum/MultipleAddresses.java | 14 +++++++- .../server/quorum/QuorumCnxManager.java | 3 +- .../server/quorum/MultipleAddressesTest.java | 33 +++++++++++++++++++ 4 files changed, 61 insertions(+), 12 deletions(-) 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 75010ea9674..1efe1cbb9c1 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 @@ -255,14 +255,14 @@ protected void sockConnect(Socket sock, InetSocketAddress addr, int timeout) thr * Establish a connection with the LearnerMaster found by findLearnerMaster. * Followers only connect to Leaders, Observers can connect to any active LearnerMaster. * Retries until either initLimit time has elapsed or 5 tries have happened. - * @param addr - the address of the Peer to connect to. + * @param multiAddr - the address of the Peer to connect to. * @throws IOException - if the socket connection fails on the 5th attempt * if there is an authentication failure while connecting to leader */ - protected void connectToLeader(MultipleAddresses addr, String hostname) throws IOException { + protected void connectToLeader(MultipleAddresses multiAddr, String hostname) throws IOException { - this.leaderAddr = addr; - Set addresses = addr.getAllAddresses(); + this.leaderAddr = multiAddr; + Set addresses = multiAddr.getAllReachableAddresses(); ExecutorService executor = Executors.newFixedThreadPool(addresses.size()); CountDownLatch latch = new CountDownLatch(addresses.size()); AtomicReference socket = new AtomicReference<>(null); @@ -284,15 +284,14 @@ protected void connectToLeader(MultipleAddresses addr, String hostname) throws I } if (socket.get() == null) { - throw new IOException("Failed connect to " + addr); + throw new IOException("Failed connect to " + multiAddr); } else { sock = socket.get(); } self.authLearner.authenticate(sock, hostname); - leaderIs = BinaryInputArchive.getArchive(new BufferedInputStream( - sock.getInputStream())); + leaderIs = BinaryInputArchive.getArchive(new BufferedInputStream(sock.getInputStream())); bufferedOutput = new BufferedOutputStream(sock.getOutputStream()); leaderOs = BinaryOutputArchive.getArchive(bufferedOutput); } @@ -315,9 +314,13 @@ public void run() { Thread.currentThread().setName("LeaderConnector-" + address); Socket sock = connectToLeader(); - if (sock != null && sock.isConnected() && !socket.compareAndSet(null, sock)) { - LOG.info("Connection to the leader is already established, close the redundant connection"); - sock.close(); + if (sock != null && sock.isConnected()) { + if (socket.compareAndSet(null, sock)) { + LOG.info("Successfully connected to leader, using address: {}", address); + } else { + LOG.info("Connection to the leader is already established, close the redundant connection"); + sock.close(); + } } } catch (Exception e) { 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 e6d35a529bb..730b182ae65 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 @@ -120,13 +120,25 @@ public void addAddress(InetSocketAddress address) { * @throws NoRouteToHostException if none of the addresses are reachable */ public InetSocketAddress getReachableAddress() throws NoRouteToHostException { - // using parallelStream() + findAny() will help to minimize the time spent, but + // using parallelStream() + findAny() will help to minimize the time spent on network operations return addresses.parallelStream() .filter(this::checkIfAddressIsReachable) .findAny() .orElseThrow(() -> new NoRouteToHostException("No valid address among " + addresses)); } + /** + * Returns a set of all reachable addresses. If none is reachable than returns empty set. + * + * @return all addresses which are reachable. + */ + public Set getAllReachableAddresses() { + // using parallelStream() will help to minimize the time spent on network operations + return addresses.parallelStream() + .filter(this::checkIfAddressIsReachable) + .collect(Collectors.toSet()); + } + /** * 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/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java index f59cce4e343..3371102ca49 100644 --- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java +++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumCnxManager.java @@ -666,7 +666,8 @@ synchronized boolean connectOne(long sid, MultipleAddresses electionAddr) { sslSock.getSession().getCipherSuite()); } - LOG.debug("Connected to server {}", sid); + LOG.debug("Connected to server {} using election address: {}:{}", + sid, sock.getInetAddress(), sock.getPort()); // Sends connection request asynchronously if the quorum // sasl authentication is enabled. This is required because // sasl server authentication process may take few seconds to 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 3bab008a42a..99203204279 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 @@ -24,7 +24,9 @@ import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import java.util.stream.Collectors; import java.util.stream.IntStream; import org.apache.commons.collections.CollectionUtils; @@ -133,6 +135,37 @@ public void testRecreateSocketAddressesWithWrongAddresses() { Assert.assertEquals(address, multipleAddresses.getOne()); } + @Test + public void testAlwaysGetReachableAddress() throws Exception{ + InetSocketAddress reachableHost = new InetSocketAddress("127.0.0.1", 1234); + 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); + + MultipleAddresses multipleAddresses = new MultipleAddresses( + Arrays.asList(unreachableHost1, unreachableHost2, unreachableHost3, reachableHost)); + + // we call the getReachableAddress() function multiple times, to make sure we + // always got back a reachable address and not just a random one + for (int i = 0; i < 10; i++) { + Assert.assertEquals(reachableHost, multipleAddresses.getReachableAddress()); + } + } + + @Test + public void testGetAllReachableAddresses() 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.getAllReachableAddresses()); + } + @Test public void testEquals() { List addresses = getAddressList(); From 4b6bcea486f81ecd2517fd237413f5ac7c8afcff Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Sun, 17 Nov 2019 13:42:55 +0100 Subject: [PATCH 13/14] ZOOKEEPER-3188: MultiAddress unit tests for Quorum TLS and Kerberos/Digest authentication --- .../server/quorum/QuorumSSLTest.java | 76 ++++++++++++++++++- .../quorum/auth/QuorumAuthTestBase.java | 29 ++++--- .../quorum/auth/QuorumDigestAuthTest.java | 23 +++++- .../quorum/auth/QuorumKerberosAuthTest.java | 29 ++++++- .../auth/QuorumKerberosHostBasedAuthTest.java | 22 ++++++ 5 files changed, 163 insertions(+), 16 deletions(-) diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java index dcd9fcee6e9..09245c53d72 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/QuorumSSLTest.java @@ -378,6 +378,8 @@ private KeyPair createKeyPair() throws NoSuchProviderException, NoSuchAlgorithmE } private String generateQuorumConfiguration() { + StringBuilder sb = new StringBuilder(); + int portQp1 = PortAssignment.unique(); int portQp2 = PortAssignment.unique(); int portQp3 = PortAssignment.unique(); @@ -386,9 +388,35 @@ private String generateQuorumConfiguration() { int portLe2 = PortAssignment.unique(); int portLe3 = PortAssignment.unique(); - return "server.1=127.0.0.1:" + (portQp1) + ":" + (portLe1) + ";" + clientPortQp1 - + "\n" + "server.2=127.0.0.1:" + (portQp2) + ":" + (portLe2) + ";" + clientPortQp2 - + "\n" + "server.3=127.0.0.1:" + (portQp3) + ":" + (portLe3) + ";" + clientPortQp3; + sb.append(String.format("server.1=127.0.0.1:%d:%d;%d\n", portQp1, portLe1, clientPortQp1)); + sb.append(String.format("server.2=127.0.0.1:%d:%d;%d\n", portQp2, portLe2, clientPortQp2)); + sb.append(String.format("server.3=127.0.0.1:%d:%d;%d\n", portQp3, portLe3, clientPortQp3)); + + return sb.toString(); + } + + private String generateMultiAddressQuorumConfiguration() { + StringBuilder sb = new StringBuilder(); + + int portQp1a = PortAssignment.unique(); + int portQp1b = PortAssignment.unique(); + int portQp2a = PortAssignment.unique(); + int portQp2b = PortAssignment.unique(); + int portQp3a = PortAssignment.unique(); + int portQp3b = PortAssignment.unique(); + + int portLe1a = PortAssignment.unique(); + int portLe1b = PortAssignment.unique(); + int portLe2a = PortAssignment.unique(); + int portLe2b = PortAssignment.unique(); + int portLe3a = PortAssignment.unique(); + int portLe3b = PortAssignment.unique(); + + sb.append(String.format("server.1=127.0.0.1:%d:%d|127.0.0.1:%d:%d;%d\n", portQp1a, portLe1a, portQp1b, portLe1b, clientPortQp1)); + sb.append(String.format("server.2=127.0.0.1:%d:%d|127.0.0.1:%d:%d;%d\n", portQp2a, portLe2a, portQp2b, portLe2b, clientPortQp2)); + sb.append(String.format("server.3=127.0.0.1:%d:%d|127.0.0.1:%d:%d;%d\n", portQp3a, portLe3a, portQp3b, portLe3b, clientPortQp3)); + + return sb.toString(); } public void setSSLSystemProperties() { @@ -449,6 +477,30 @@ public void testQuorumSSL() throws Exception { assertFalse(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp3, CONNECTION_TIMEOUT)); } + + @Test + public void testQuorumSSLWithMultipleAddresses() throws Exception { + quorumConfiguration = generateMultiAddressQuorumConfiguration(); + + q1 = new MainThread(1, clientPortQp1, quorumConfiguration, SSL_QUORUM_ENABLED); + q2 = new MainThread(2, clientPortQp2, quorumConfiguration, SSL_QUORUM_ENABLED); + + q1.start(); + q2.start(); + + assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp1, CONNECTION_TIMEOUT)); + assertTrue(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp2, CONNECTION_TIMEOUT)); + + clearSSLSystemProperties(); + + // This server should fail to join the quorum as it is not using ssl. + q3 = new MainThread(3, clientPortQp3, quorumConfiguration); + q3.start(); + + assertFalse(ClientBase.waitForServerUp("127.0.0.1:" + clientPortQp3, CONNECTION_TIMEOUT)); + } + + @Test public void testRollingUpgrade() throws Exception { // Form a quorum without ssl @@ -544,6 +596,24 @@ public void testHostnameVerificationWithInvalidIpAddressAndInvalidHostname() thr testHostnameVerification(badhostnameKeystorePath, false); } + @Test + public void testHostnameVerificationForInvalidMultiAddressServerConfig() throws Exception { + quorumConfiguration = generateMultiAddressQuorumConfiguration(); + + String badhostnameKeystorePath = tmpDir + "/badhost.jks"; + X509Certificate badHostCert = buildEndEntityCert( + defaultKeyPair, + rootCertificate, + rootKeyPair.getPrivate(), + "bleepbloop", + "140.211.11.105", + null, + null); + writeKeystore(badHostCert, defaultKeyPair, badhostnameKeystorePath); + + testHostnameVerification(badhostnameKeystorePath, false); + } + @Test public void testHostnameVerificationWithInvalidIpAddressAndValidHostname() throws Exception { String badhostnameKeystorePath = tmpDir + "/badhost.jks"; diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java index db5d39da67f..3b52cc009d7 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumAuthTestBase.java @@ -64,12 +64,23 @@ public static void cleanupJaasConfig() { } } + protected String startQuorum(final int serverCount, Map authConfigs, + int authServerCount) throws IOException { + return this.startQuorum(serverCount, authConfigs, authServerCount, false); + } + + protected String startMultiAddressQuorum(final int serverCount, Map authConfigs, + int authServerCount) throws IOException { + return this.startQuorum(serverCount, authConfigs, authServerCount, true); + } + protected String startQuorum( final int serverCount, Map authConfigs, - int authServerCount) throws IOException { + int authServerCount, + boolean multiAddress) throws IOException { StringBuilder connectStr = new StringBuilder(); - final int[] clientPorts = startQuorum(serverCount, connectStr, authConfigs, authServerCount); + final int[] clientPorts = startQuorum(serverCount, connectStr, authConfigs, authServerCount, multiAddress); for (int i = 0; i < serverCount; i++) { assertTrue( "waiting for server " + i + " being up", @@ -78,17 +89,17 @@ protected String startQuorum( return connectStr.toString(); } - protected int[] startQuorum( - final int serverCount, - StringBuilder connectStr, - Map authConfigs, - int authServerCount) throws IOException { + protected int[] startQuorum(final int serverCount, StringBuilder connectStr, Map authConfigs, + int authServerCount, boolean multiAddress) throws IOException { final int[] clientPorts = new int[serverCount]; StringBuilder sb = new StringBuilder(); for (int i = 0; i < serverCount; i++) { clientPorts[i] = PortAssignment.unique(); - String server = String.format("server.%d=localhost:%d:%d:participant", i, PortAssignment.unique(), PortAssignment.unique()); - sb.append(server + "\n"); + String server = String.format("server.%d=localhost:%d:%d", i, PortAssignment.unique(), PortAssignment.unique()); + if (multiAddress) { + server = server + String.format("|localhost:%d:%d", PortAssignment.unique(), PortAssignment.unique()); + } + sb.append(server + ":participant\n"); connectStr.append("127.0.0.1:" + clientPorts[i]); if (i < serverCount - 1) { connectStr.append(","); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java index ed86607c4f1..7462979d7cd 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumDigestAuthTest.java @@ -92,6 +92,27 @@ public void testValidCredentials() throws Exception { zk.close(); } + /** + * Test to verify that server is able to start with valid credentials + * when using multiple Quorum / Election addresses + */ + @Test(timeout = 30000) + public void testValidCredentialsWithMultiAddresses() throws Exception { + Map authConfigs = new HashMap(); + authConfigs.put(QuorumAuth.QUORUM_SASL_AUTH_ENABLED, "true"); + authConfigs.put(QuorumAuth.QUORUM_SERVER_SASL_AUTH_REQUIRED, "true"); + authConfigs.put(QuorumAuth.QUORUM_LEARNER_SASL_AUTH_REQUIRED, "true"); + + String connectStr = startMultiAddressQuorum(3, authConfigs, 3); + CountdownWatcher watcher = new CountdownWatcher(); + ZooKeeper zk = new ZooKeeper(connectStr, ClientBase.CONNECTION_TIMEOUT, watcher); + watcher.waitForConnected(ClientBase.CONNECTION_TIMEOUT); + for (int i = 0; i < 10; i++) { + zk.create("/" + i, new byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + zk.close(); + } + /** * Test to verify that server is able to start with invalid credentials if * the configuration is set to quorum.auth.serverRequireSasl=false. @@ -126,7 +147,7 @@ public void testSaslRequiredInvalidCredentials() throws Exception { authConfigs.put(QuorumAuth.QUORUM_SERVER_SASL_AUTH_REQUIRED, "true"); authConfigs.put(QuorumAuth.QUORUM_LEARNER_SASL_AUTH_REQUIRED, "true"); int serverCount = 2; - final int[] clientPorts = startQuorum(serverCount, new StringBuilder(), authConfigs, serverCount); + final int[] clientPorts = startQuorum(serverCount, new StringBuilder(), authConfigs, serverCount, false); for (int i = 0; i < serverCount; i++) { boolean waitForServerUp = ClientBase.waitForServerUp("127.0.0.1:" + clientPorts[i], QuorumPeerTestBase.TIMEOUT); assertFalse("Shouldn't start server with invalid credentials", waitForServerUp); diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java index 092d04353bc..b9d662a16e2 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosAuthTest.java @@ -31,7 +31,7 @@ import org.apache.zookeeper.test.ClientBase.CountdownWatcher; import org.junit.After; import org.junit.AfterClass; -import org.junit.Before; +import org.junit.BeforeClass; import org.junit.Test; public class QuorumKerberosAuthTest extends KerberosSecurityTestcase { @@ -70,8 +70,8 @@ public class QuorumKerberosAuthTest extends KerberosSecurityTestcase { setupJaasConfig(jaasEntries); } - @Before - public void setUp() throws Exception { + @BeforeClass + public static void setUp() throws Exception { // create keytab keytabFile = new File(KerberosTestUtils.getKeytabFile()); String learnerPrincipal = KerberosTestUtils.getLearnerPrincipal(); @@ -119,4 +119,27 @@ public void testValidCredentials() throws Exception { zk.close(); } + /** + * Test to verify that server is able to start with valid credentials + * when using multiple Quorum / Election addresses + */ + @Test(timeout = 120000) + public void testValidCredentialsWithMultiAddresses() throws Exception { + String serverPrincipal = KerberosTestUtils.getServerPrincipal(); + serverPrincipal = serverPrincipal.substring(0, serverPrincipal.lastIndexOf("@")); + Map authConfigs = new HashMap(); + authConfigs.put(QuorumAuth.QUORUM_SASL_AUTH_ENABLED, "true"); + authConfigs.put(QuorumAuth.QUORUM_SERVER_SASL_AUTH_REQUIRED, "true"); + authConfigs.put(QuorumAuth.QUORUM_LEARNER_SASL_AUTH_REQUIRED, "true"); + authConfigs.put(QuorumAuth.QUORUM_KERBEROS_SERVICE_PRINCIPAL, serverPrincipal); + String connectStr = startMultiAddressQuorum(3, authConfigs, 3); + CountdownWatcher watcher = new CountdownWatcher(); + ZooKeeper zk = new ZooKeeper(connectStr, ClientBase.CONNECTION_TIMEOUT, watcher); + watcher.waitForConnected(ClientBase.CONNECTION_TIMEOUT); + for (int i = 0; i < 10; i++) { + zk.create("/" + i, new byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + zk.close(); + } + } diff --git a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java index 574bfe4eed9..93867550dcf 100644 --- a/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java +++ b/zookeeper-server/src/test/java/org/apache/zookeeper/server/quorum/auth/QuorumKerberosHostBasedAuthTest.java @@ -142,6 +142,28 @@ public void testValidCredentials() throws Exception { zk.close(); } + /** + * Test to verify that server is able to start with valid credentials + * when using multiple Quorum / Election addresses + */ + @Test(timeout = 120000) + public void testValidCredentialsWithMultiAddresses() throws Exception { + String serverPrincipal = hostServerPrincipal.substring(0, hostServerPrincipal.lastIndexOf("@")); + Map authConfigs = new HashMap(); + authConfigs.put(QuorumAuth.QUORUM_SASL_AUTH_ENABLED, "true"); + authConfigs.put(QuorumAuth.QUORUM_SERVER_SASL_AUTH_REQUIRED, "true"); + authConfigs.put(QuorumAuth.QUORUM_LEARNER_SASL_AUTH_REQUIRED, "true"); + authConfigs.put(QuorumAuth.QUORUM_KERBEROS_SERVICE_PRINCIPAL, serverPrincipal); + String connectStr = startMultiAddressQuorum(3, authConfigs, 3); + CountdownWatcher watcher = new CountdownWatcher(); + ZooKeeper zk = new ZooKeeper(connectStr, ClientBase.CONNECTION_TIMEOUT, watcher); + watcher.waitForConnected(ClientBase.CONNECTION_TIMEOUT); + for (int i = 0; i < 10; i++) { + zk.create("/" + i, new byte[0], Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); + } + zk.close(); + } + /** * Test to verify that the bad server connection to the quorum should be rejected. */ From 356882d46be95aef506c696dbda7171268cc1380 Mon Sep 17 00:00:00 2001 From: Mate Szalay-Beko Date: Tue, 19 Nov 2019 16:43:01 +0100 Subject: [PATCH 14/14] ZOOKEEPER-3188: document new configuration format for using multiple addresses --- .../main/resources/markdown/zookeeperAdmin.md | 25 +++++++++++++++++-- .../resources/markdown/zookeeperReconfig.md | 21 ++++++++++++++++ .../resources/markdown/zookeeperStarted.md | 5 ++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md index 272731c6e0f..ab48d0360c2 100644 --- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md +++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md @@ -202,7 +202,13 @@ ensemble: though about a few here: Every machine that is part of the ZooKeeper ensemble should know about every other machine in the ensemble. You accomplish this with - the series of lines of the form **server.id=host:port:port**. The parameters **host** and **port** are straightforward. You attribute the + the series of lines of the form **server.id=host:port:port**. + (The parameters **host** and **port** are straightforward, for each server + you need to specify first a Quorum port then a dedicated port for ZooKeeper leader + election). Since ZooKeeper 3.6.0 you can also [specify multiple addresses](#id_multi_address) + for each ZooKeeper server instance (this can increase availability when multiple physical + network interfaces can be used parallel in the cluster). + You attribute the server id to each machine by creating a file named *myid*, one for each server, which resides in that server's data directory, as specified by the configuration file @@ -1042,7 +1048,7 @@ of servers -- that is, when deploying clusters of servers. >Turning on leader selection is highly recommended when you have more than three ZooKeeper servers in an ensemble. -* *server.x=[hostname]:nnnnn[:nnnnn], etc* : +* *server.x=[hostname]:nnnnn[:nnnnn] etc* : (No Java system property) servers making up the ZooKeeper ensemble. When the server starts up, it determines which server it is by looking for the @@ -1057,6 +1063,21 @@ of servers -- that is, when deploying clusters of servers. The first followers use to connect to the leader, and the second is for leader election. If you want to test multiple servers on a single machine, then different ports can be used for each server. + + + + Since ZooKeeper 3.6.0 it is possible to specify **multiple addresses** for each + ZooKeeper server (see [ZOOKEEPER-3188](https://issues.apache.org/jira/projects/ZOOKEEPER/issues/ZOOKEEPER-3188)). + This helps to increase availability and adds network level + resiliency to ZooKeeper. When multiple physical network interfaces are used + for the servers, ZooKeeper is able to bind on all interfaces and runtime switching + to a working interface in case a network error. The different addresses can be specified + in the config using a pipe ('|') character. A valid configuration using multiple addresses looks like: + + server.1=zoo1-net1:2888:3888|zoo1-net2:2889:3889 + server.2=zoo2-net1:2888:3888|zoo2-net2:2889:3889 + server.3=zoo3-net1:2888:3888|zoo3-net2:2889:3889 + * *syncLimit* : (No Java system property) diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperReconfig.md b/zookeeper-docs/src/main/resources/markdown/zookeeperReconfig.md index 77a8fe555d5..afb85df0ad9 100644 --- a/zookeeper-docs/src/main/resources/markdown/zookeeperReconfig.md +++ b/zookeeper-docs/src/main/resources/markdown/zookeeperReconfig.md @@ -19,6 +19,7 @@ limitations under the License. * [Overview](#ch_reconfig_intro) * [Changes to Configuration Format](#ch_reconfig_format) * [Specifying the client port](#sc_reconfig_clientport) + * [Specifying multiple server addresses](#sc_multiaddress) * [The standaloneEnabled flag](#sc_reconfig_standaloneEnabled) * [The reconfigEnabled flag](#sc_reconfig_reconfigEnabled) * [Dynamic configuration file](#sc_reconfig_file) @@ -109,6 +110,26 @@ Examples of legal server statements: server.5 = 125.23.63.23:1234:1235;125.23.63.24:1236 server.5 = 125.23.63.23:1234:1235:participant;125.23.63.23:1236 + + + +### Specifying multiple server addresses + +Since ZooKeeper 3.6.0 it is possible to specify multiple addresses for each +ZooKeeper server (see [ZOOKEEPER-3188](https://issues.apache.org/jira/projects/ZOOKEEPER/issues/ZOOKEEPER-3188)). +This helps to increase availability and adds network level +resiliency to ZooKeeper. When multiple physical network interfaces are used +for the servers, ZooKeeper is able to bind on all interfaces and runtime switching +to a working interface in case a network error. The different addresses can be +specified in the config using a pipe ('|') character. + +Examples for a valid configurations using multiple addresses: + + server.2=zoo2-net1:2888:3888|zoo2-net2:2889:3889;2188 + server.2=zoo2-net1:2888:3888|zoo2-net2:2889:3889|zoo2-net3:2890:3890;2188 + server.2=zoo2-net1:2888:3888|zoo2-net2:2889:3889;zoo2-net1:2188 + server.2=zoo2-net1:2888:3888:observer|zoo2-net2:2889:3889:observer;2188 + ### The _standaloneEnabled_ flag diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperStarted.md b/zookeeper-docs/src/main/resources/markdown/zookeeperStarted.md index ee19a71175d..bb1e2f0c203 100644 --- a/zookeeper-docs/src/main/resources/markdown/zookeeperStarted.md +++ b/zookeeper-docs/src/main/resources/markdown/zookeeperStarted.md @@ -355,6 +355,11 @@ server have its own machine. It must be a completely separate physical server. Multiple virtual machines on the same physical host are still vulnerable to the complete failure of that host. +>If you have multiple network interfaces in your ZooKeeper machines, +you can also instruct ZooKeeper to bind on all of your interfaces and +automatically switch to a healthy interface in case of a network failure. +For details, see the [Configuration Parameters](zookeeperAdmin.html#id_multi_address). + ### Other Optimizations