From f0c1b71cb5f5ccf67c814affbff19f1acdad9455 Mon Sep 17 00:00:00 2001 From: Michael Stack Date: Wed, 10 Jan 2018 14:25:39 -0800 Subject: [PATCH] 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. Later in this test we kill a session... so makes sense that when we try to use the old zk, we could get a ConnectionLoss. Catch it. Allow some time for transition to new zk to happen before calling asserts. 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. --- .../hadoop/hbase/zookeeper/ReadOnlyZKClient.java | 3 +- .../hadoop/hbase/zookeeper/ZooKeeperHelper.java | 48 ++++++++++++++++++++++ hbase-zookeeper/pom.xml | 3 ++ .../hadoop/hbase/zookeeper/ZKMainServer.java | 9 ++-- .../hbase/zookeeper/TestReadOnlyZKClient.java | 8 ++-- .../hadoop/hbase/zookeeper/TestZKMainServer.java | 3 +- .../hadoop/hbase/zookeeper/TestZKNodeTracker.java | 6 +-- .../src/test/resources/log4j.properties | 2 +- 8 files changed, 68 insertions(+), 14 deletions(-) create mode 100644 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/ReadOnlyZKClient.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ReadOnlyZKClient.java index 24c7112a81..70af6159fe 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 @@ -284,8 +284,7 @@ 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 = ZooKeeperHelper.getConnectedZooKeeper(connectString, sessionTimeoutMs); } 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 0000000000..524e28d0d7 --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/zookeeper/ZooKeeperHelper.java @@ -0,0 +1,48 @@ +/** + * 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 org.apache.hadoop.hbase.util.Threads; +import org.apache.yetus.audience.InterfaceAudience; +import org.apache.zookeeper.ZooKeeper; + + +/** + * Methods that help working with ZooKeeper + */ +@InterfaceAudience.Private +public class ZooKeeperHelper { + // This class cannot be instantiated + private ZooKeeperHelper() { + } + + /** + * Get a ZooKeeper instance and wait until it connected before returning. + */ + public static ZooKeeper getConnectedZooKeeper(String connectString, int sessionTimeoutMs) + throws IOException { + ZooKeeper zookeeper = new ZooKeeper(connectString, sessionTimeoutMs, e -> {}); + // Make sure we are connected before we hand it back. + while(!zookeeper.getState().isConnected()) { + Threads.sleep(1); + } + return zookeeper; + } +} diff --git a/hbase-zookeeper/pom.xml b/hbase-zookeeper/pom.xml index 67d373013f..aff824bcb0 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 3a96015ffa..d9da0e597e 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 @@ -53,12 +53,15 @@ public class ZKMainServer { // 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(); + // 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. + long timeout = 30000; while (!this.zk.getState().isConnected()) { Thread.sleep(1); - if (stopWatch.elapsed(TimeUnit.SECONDS) > 10) { + if (stopWatch.elapsed(TimeUnit.MILLISECONDS) > timeout) { throw new InterruptedException("Failed connect after waiting " + - stopWatch.elapsed(TimeUnit.SECONDS) + "seconds; state=" + this.zk.getState() + - "; " + this.zk); + stopWatch.elapsed(TimeUnit.MILLISECONDS) + "ms (zk session timeout); " + + this.zk); } } } 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 1f83536393..ab5d4665e0 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 @@ -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 bc1c240e59..d8db8aedac 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 3778ca0119..9afa9d1595 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 c322699ced..f599ea636e 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. -- 2.11.0 (Apple Git-81)