You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2018/01/12 02:08:31 UTC
[06/46] hbase git commit: HBASE-19753 Miscellany of fixes for
hbase-zookeeper tests to make them more robust
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/HBASE-19397-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.