diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java index 24c7112a81f..82c011b1cdf 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java @@ -31,6 +31,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.ZooKeeperConnectionException; import org.apache.yetus.audience.InterfaceAudience; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.KeeperException.Code; @@ -249,8 +250,8 @@ public final class ReadOnlyZKClient implements Closeable { @Override public void exec(ZooKeeper zk) { - zk.getData(path, false, (rc, path, ctx, data, stat) -> onComplete(zk, rc, data, true), - null); + zk.getData(path, false, + (rc, path, ctx, data, stat) -> onComplete(zk, rc, data, true), null); } }); return future; @@ -284,8 +285,17 @@ public final class ReadOnlyZKClient implements Closeable { private ZooKeeper getZk() throws IOException { // may be closed when session expired if (zookeeper == null || !zookeeper.getState().isAlive()) { - zookeeper = new ZooKeeper(connectString, sessionTimeoutMs, e -> { - }); + zookeeper = new ZooKeeper(connectString, sessionTimeoutMs, e -> {}); + int timeout = 10000; + try { + // Before returning, try and ensure we are connected. Don't wait long in case + // we are trying to connect to a cluster that is down. If we fail to connect, + // just catch the exception and carry-on. The first usage will fail and we'll + // cleanup. + zookeeper = ZooKeeperHelper.ensureConnectedZooKeeper(zookeeper, timeout); + } catch (ZooKeeperConnectionException e) { + LOG.warn("Failed connecting after waiting " + timeout + "ms; " + zookeeper); + } } return zookeeper; } diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperHelper.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperHelper.java new file mode 100644 index 00000000000..dd26ed5f209 --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperHelper.java @@ -0,0 +1,71 @@ +/* + * 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.hadoop.hbase.zookeeper; + +import java.io.IOException; +import java.util.concurrent.TimeUnit; + +import org.apache.hadoop.hbase.ZooKeeperConnectionException; +import org.apache.hadoop.hbase.util.Threads; +import org.apache.hbase.thirdparty.com.google.common.base.Stopwatch; +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.zookeeper.ZooKeeper; + + +/** + * Methods that help working with ZooKeeper + */ +@InterfaceAudience.Private +public final class ZooKeeperHelper { + // This class cannot be instantiated + private ZooKeeperHelper() { + } + + /** + * Get a ZooKeeper instance and wait until it connected before returning. + * @param sessionTimeoutMs Used as session timeout passed to the created ZooKeeper AND as the + * timeout to wait on connection establishment. + */ + public static ZooKeeper getConnectedZooKeeper(String connectString, int sessionTimeoutMs) + throws IOException { + ZooKeeper zookeeper = new ZooKeeper(connectString, sessionTimeoutMs, e -> {}); + return ensureConnectedZooKeeper(zookeeper, sessionTimeoutMs); + } + + /** + * Ensure passed zookeeper is connected. + * @param timeout Time to wait on established Connection + */ + public static ZooKeeper ensureConnectedZooKeeper(ZooKeeper zookeeper, int timeout) + throws ZooKeeperConnectionException { + if (zookeeper.getState().isConnected()) { + return zookeeper; + } + Stopwatch stopWatch = Stopwatch.createStarted(); + // Make sure we are connected before we hand it back. + while(!zookeeper.getState().isConnected()) { + Threads.sleep(1); + if (stopWatch.elapsed(TimeUnit.MILLISECONDS) > timeout) { + throw new ZooKeeperConnectionException("Failed connect after waiting " + + stopWatch.elapsed(TimeUnit.MILLISECONDS) + "ms (zk session timeout); " + + zookeeper); + } + } + return zookeeper; + } +} diff --git a/hbase-zookeeper/pom.xml b/hbase-zookeeper/pom.xml index 08e215ca1c1..c5303dfde07 100644 --- a/hbase-zookeeper/pom.xml +++ b/hbase-zookeeper/pom.xml @@ -96,6 +96,9 @@ maven-surefire-plugin + + src/test/resources + listener diff --git a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKMainServer.java b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKMainServer.java index 3a96015ffad..2488682bcbc 100644 --- a/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKMainServer.java +++ b/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKMainServer.java @@ -1,4 +1,4 @@ -/** +/* * 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 @@ -19,7 +19,6 @@ package org.apache.hadoop.hbase.zookeeper; import java.io.IOException; -import java.util.concurrent.TimeUnit; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; @@ -28,7 +27,6 @@ import org.apache.yetus.audience.InterfaceAudience; import org.apache.zookeeper.KeeperException; import org.apache.zookeeper.ZooKeeperMain; -import org.apache.hbase.thirdparty.com.google.common.base.Stopwatch; /** * Tool for running ZookeeperMain from HBase by reading a ZooKeeper server @@ -52,15 +50,9 @@ public class ZKMainServer { super(args); // Make sure we are connected before we proceed. Can take a while on some systems. If we // run the command without being connected, we get ConnectionLoss KeeperErrorConnection... - Stopwatch stopWatch = Stopwatch.createStarted(); - while (!this.zk.getState().isConnected()) { - Thread.sleep(1); - if (stopWatch.elapsed(TimeUnit.SECONDS) > 10) { - throw new InterruptedException("Failed connect after waiting " + - stopWatch.elapsed(TimeUnit.SECONDS) + "seconds; state=" + this.zk.getState() + - "; " + this.zk); - } - } + // Make it 30seconds. We dont' have a config in this context and zk doesn't have + // a timeout until after connection. 30000ms is default for zk. + ZooKeeperHelper.ensureConnectedZooKeeper(this.zk, 30000); } /** diff --git a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java index 1f83536393f..c47812156fb 100644 --- a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java +++ b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java @@ -1,4 +1,4 @@ -/** +/* * 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 @@ -67,8 +67,8 @@ public class TestReadOnlyZKClient { public static void setUp() throws Exception { PORT = UTIL.startMiniZKCluster().getClientPort(); - ZooKeeper zk = new ZooKeeper("localhost:" + PORT, 10000, e -> { - }); + ZooKeeper zk = ZooKeeperHelper. + getConnectedZooKeeper("localhost:" + PORT, 10000); DATA = new byte[10]; ThreadLocalRandom.current().nextBytes(DATA); zk.create(PATH, DATA, ZooDefs.Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); @@ -137,8 +137,8 @@ public class TestReadOnlyZKClient { UTIL.getZkCluster().getZooKeeperServers().get(0).closeSession(sessionId); // should not reach keep alive so still the same instance assertSame(zk, RO_ZK.getZooKeeper()); - - assertArrayEquals(DATA, RO_ZK.get(PATH).get()); + byte [] got = RO_ZK.get(PATH).get(); + assertArrayEquals(DATA, got); assertNotNull(RO_ZK.getZooKeeper()); assertNotSame(zk, RO_ZK.getZooKeeper()); assertNotEquals(sessionId, RO_ZK.getZooKeeper().getSessionId()); diff --git a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMainServer.java b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMainServer.java index bc1c240e593..d8db8aedacd 100644 --- a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMainServer.java +++ b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMainServer.java @@ -70,7 +70,8 @@ public class TestZKMainServer { public void testCommandLineWorks() throws Exception { System.setSecurityManager(new NoExitSecurityManager()); HBaseZKTestingUtility htu = new HBaseZKTestingUtility(); - htu.getConfiguration().setInt(HConstants.ZK_SESSION_TIMEOUT, 1000); + // Make it long so for sure succeeds. + htu.getConfiguration().setInt(HConstants.ZK_SESSION_TIMEOUT, 30000); htu.startMiniZKCluster(); try { ZKWatcher zkw = htu.getZooKeeperWatcher(); diff --git a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKNodeTracker.java b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKNodeTracker.java index 3778ca0119c..9afa9d1595e 100644 --- a/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKNodeTracker.java +++ b/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKNodeTracker.java @@ -129,9 +129,9 @@ public class TestZKNodeTracker { // Create a completely separate zk connection for test triggers and avoid // any weird watcher interactions from the test - final ZooKeeper zkconn = - new ZooKeeper(ZKConfig.getZKQuorumServersString(TEST_UTIL.getConfiguration()), 60000, e -> { - }); + final ZooKeeper zkconn = ZooKeeperHelper. + getConnectedZooKeeper(ZKConfig.getZKQuorumServersString(TEST_UTIL.getConfiguration()), + 60000); // Add the node with data one zkconn.create(node, dataOne, Ids.OPEN_ACL_UNSAFE, CreateMode.PERSISTENT); diff --git a/hbase-zookeeper/src/test/resources/log4j.properties b/hbase-zookeeper/src/test/resources/log4j.properties index c322699ced2..f599ea636e9 100644 --- a/hbase-zookeeper/src/test/resources/log4j.properties +++ b/hbase-zookeeper/src/test/resources/log4j.properties @@ -55,7 +55,7 @@ log4j.appender.console.layout.ConversionPattern=%d{ISO8601} %-5p [%t] %C{2}(%L): #log4j.logger.org.apache.hadoop.fs.FSNamesystem=DEBUG log4j.logger.org.apache.hadoop=WARN -log4j.logger.org.apache.zookeeper=ERROR +log4j.logger.org.apache.zookeeper=DEBUG log4j.logger.org.apache.hadoop.hbase=DEBUG #These settings are workarounds against spurious logs from the minicluster.