Repository: hbase Updated Branches: refs/heads/branch-2 3849db8f1 -> a917a4e79
HBASE-19753 Miscellany of fixes for hbase-zookeeper tests to make them more robust First, we add test resources to CLASSPATH when tests run. W/o it, there was no logging of hbase-zookeeper test output (not sure why I have to add this here and not over in hbase-server; research turns up nothing so far). M hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKMainServer.java Improve fail log message. M hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java M hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKNodeTracker.java Wait until ZK is connected before progressing. On my slow zk, it could be a while post construction before zk connected. Using an unconnected zk caused test to fail. M hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMainServer.java Change session timeout to default 30s from 1s which was way too short. M hbase-zookeeper/src/test/resources/log4j.properties Set zk logs to DEBUG level in this module at least. Adds a ZooKeeperHelper class that has utility to help interacting w/ ZK. Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/a917a4e7 Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/a917a4e7 Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/a917a4e7 Branch: refs/heads/branch-2 Commit: a917a4e796d39f8f0e17be4262b0ff088d733d28 Parents: 3849db8 Author: Michael Stack <st...@apache.org> Authored: Wed Jan 10 14:25:39 2018 -0800 Committer: Michael Stack <st...@apache.org> Committed: Thu Jan 11 11:22:56 2018 -0800 ---------------------------------------------------------------------- .../hbase/zookeeper/ReadOnlyZKClient.java | 18 +++-- .../hadoop/hbase/zookeeper/ZooKeeperHelper.java | 71 ++++++++++++++++++++ hbase-zookeeper/pom.xml | 3 + .../hadoop/hbase/zookeeper/ZKMainServer.java | 16 ++--- .../hbase/zookeeper/TestReadOnlyZKClient.java | 10 +-- .../hbase/zookeeper/TestZKMainServer.java | 3 +- .../hbase/zookeeper/TestZKNodeTracker.java | 6 +- .../src/test/resources/log4j.properties | 2 +- 8 files changed, 103 insertions(+), 26 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java ---------------------------------------------------------------------- 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 24c7112..82c011b 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; } http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperHelper.java ---------------------------------------------------------------------- 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 0000000..dd26ed5 --- /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; + } +} http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-zookeeper/pom.xml ---------------------------------------------------------------------- diff --git a/hbase-zookeeper/pom.xml b/hbase-zookeeper/pom.xml index 67d3730..aff824b 100644 --- a/hbase-zookeeper/pom.xml +++ b/hbase-zookeeper/pom.xml @@ -96,6 +96,9 @@ <plugin> <artifactId>maven-surefire-plugin</artifactId> <configuration> + <additionalClasspathElements> + <additionalClasspathElement>src/test/resources</additionalClasspathElement> + </additionalClasspathElements> <properties> <property> <name>listener</name> http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-zookeeper/src/main/java/org/apache/hadoop/hbase/zookeeper/ZKMainServer.java ---------------------------------------------------------------------- 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 3a96015..2488682 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); } /** http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestReadOnlyZKClient.java ---------------------------------------------------------------------- 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 1f83536..c478121 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()); http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKMainServer.java ---------------------------------------------------------------------- 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 bc1c240..d8db8ae 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(); http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-zookeeper/src/test/java/org/apache/hadoop/hbase/zookeeper/TestZKNodeTracker.java ---------------------------------------------------------------------- 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 3778ca0..9afa9d1 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); http://git-wip-us.apache.org/repos/asf/hbase/blob/a917a4e7/hbase-zookeeper/src/test/resources/log4j.properties ---------------------------------------------------------------------- diff --git a/hbase-zookeeper/src/test/resources/log4j.properties b/hbase-zookeeper/src/test/resources/log4j.properties index c322699..f599ea6 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.
