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.

Reply via email to