HBASE-7578 TestCatalogTracker hangs occasionally

git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1434436 13f79535-47bb-0310-9956-ffa450edef68

hbase-server/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java

@@ -122,11 +122,6 @@ public class CatalogTracker {
    */
   private ServerName metaLocation;
 
-  /*
-   * Timeout waiting on root or meta to be set.
-   */
-  private final int defaultTimeout;
-
   private boolean stopped = false;
 
   static final byte [] ROOT_REGION_NAME =
@@ -162,33 +157,13 @@ public class CatalogTracker {
    * @throws IOException 
    */
   public CatalogTracker(final ZooKeeperWatcher zk, final Configuration conf,
-      final Abortable abortable)
+      Abortable abortable)
   throws IOException {
-    this(zk, conf, abortable,
+    this(zk, conf, HConnectionManager.getConnection(conf), abortable);
-      conf.getInt("hbase.catalogtracker.default.timeout", 1000));
-  }
-
-  /**
-   * Constructs the catalog tracker.  Find current state of catalog tables.
-   * Begin active tracking by executing {@link #start()} post construction.
-   * @param zk If zk is null, we'll create an instance (and shut it down
-   * when {@link #stop()} is called) else we'll use what is passed.
-   * @param conf
-   * @param abortable If fatal exception we'll call abort on this.  May be null.
-   * If it is we'll use the Connection associated with the passed
-   * {@link Configuration} as our Abortable.
-   * @param defaultTimeout Timeout to use.  Pass zero for no timeout
-   * ({@link Object#wait(long)} when passed a <code>0</code> waits for ever).
-   * @throws IOException
-   */
-  public CatalogTracker(final ZooKeeperWatcher zk, final Configuration conf,
-      Abortable abortable, final int defaultTimeout)
-  throws IOException {
-    this(zk, conf, HConnectionManager.getConnection(conf), abortable, defaultTimeout);
   }
 
   public CatalogTracker(final ZooKeeperWatcher zk, final Configuration conf,
-      HConnection connection, Abortable abortable, final int defaultTimeout)
+      HConnection connection, Abortable abortable)
   throws IOException {
     this.connection = connection;
     if (abortable == null) {
@@ -226,7 +201,6 @@ public class CatalogTracker {
         ct.resetMetaLocation();
       }
     };
-    this.defaultTimeout = defaultTimeout;
   }
 
   /**
@@ -363,24 +337,6 @@ public class CatalogTracker {
     return getCachedConnection(waitForRoot(timeout));
   }
 
-  /**
-   * Gets a connection to the server hosting root, as reported by ZooKeeper,
-   * waiting for the default timeout specified on instantiation.
-   * @see #waitForRoot(long) for additional information
-   * @return connection to server hosting root
-   * @throws NotAllMetaRegionsOnlineException if timed out waiting
-   * @throws IOException
-   * @deprecated Use #getRootServerConnection(long)
-   */
-  public AdminProtocol waitForRootServerConnectionDefault()
-  throws NotAllMetaRegionsOnlineException, IOException {
-    try {
-      return getRootServerConnection(this.defaultTimeout);
-    } catch (InterruptedException e) {
-      throw new NotAllMetaRegionsOnlineException("Interrupted");
-    }
-  }
-
   /**
    * Gets a connection to the server currently hosting <code>.META.</code> or
    * null if location is not currently available.
@@ -470,10 +426,10 @@ public class CatalogTracker {
    */
   public ServerName waitForMeta(long timeout)
   throws InterruptedException, IOException, NotAllMetaRegionsOnlineException {
-    long stop = System.currentTimeMillis() + timeout;
+    long stop = timeout == 0 ? Long.MAX_VALUE : System.currentTimeMillis() + timeout;
     long waitTime = Math.min(50, timeout);
     synchronized (metaAvailable) {
-      while(!stopped && (timeout == 0 || System.currentTimeMillis() < stop)) {
+      while(!stopped && System.currentTimeMillis() < stop) {
         if (getMetaServerConnection() != null) {
           return metaLocation;
         }
@@ -502,25 +458,6 @@ public class CatalogTracker {
     return getCachedConnection(waitForMeta(timeout));
   }
 
-  /**
-   * Gets a connection to the server hosting meta, as reported by ZooKeeper,
-   * waiting up to the specified timeout for availability.
-   * Used in tests.
-   * @see #waitForMeta(long) for additional information
-   * @return connection to server hosting meta
-   * @throws NotAllMetaRegionsOnlineException if timed out or interrupted
-   * @throws IOException
-   * @deprecated Does not retry; use an HTable instance instead.
-   */
-  public AdminProtocol waitForMetaServerConnectionDefault()
-  throws NotAllMetaRegionsOnlineException, IOException {
-    try {
-      return getCachedConnection(waitForMeta(defaultTimeout));
-    } catch (InterruptedException e) {
-      throw new NotAllMetaRegionsOnlineException("Interrupted");
-    }
-  }
-
   /**
    * Called when we figure current meta is off (called from zk callback).
    */

hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java

@@ -541,8 +541,7 @@ Server {
    */
   private void initializeZKBasedSystemTrackers() throws IOException,
       InterruptedException, KeeperException {
-    this.catalogTracker = createCatalogTracker(this.zooKeeper, this.conf,
+    this.catalogTracker = createCatalogTracker(this.zooKeeper, this.conf, this);
-        this, conf.getInt("hbase.master.catalog.timeout", 600000));
     this.catalogTracker.start();
 
     this.balancer = LoadBalancerFactory.getLoadBalancer(conf);
@@ -585,9 +584,9 @@ Server {
    * @throws IOException
    */
   CatalogTracker createCatalogTracker(final ZooKeeperWatcher zk,
-      final Configuration conf, Abortable abortable, final int defaultTimeout)
+      final Configuration conf, Abortable abortable)
   throws IOException {
-    return new CatalogTracker(zk, conf, abortable, defaultTimeout);
+    return new CatalogTracker(zk, conf, abortable);
   }
 
   // Check if we should stop every 100ms

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java

@@ -769,8 +769,7 @@ public class  HRegionServer implements ClientProtocol,
     blockAndCheckIfStopped(this.clusterStatusTracker);
 
     // Create the catalog tracker and start it;
-    this.catalogTracker = new CatalogTracker(this.zooKeeper, this.conf,
+    this.catalogTracker = new CatalogTracker(this.zooKeeper, this.conf, this);
-      this, this.conf.getInt("hbase.regionserver.catalog.timeout", 600000));
     catalogTracker.start();
 
     // Retrieve clusterId

hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java

@@ -115,7 +115,7 @@ public class TestCatalogTracker {
   private CatalogTracker constructAndStartCatalogTracker(final HConnection c)
   throws IOException, InterruptedException {
     CatalogTracker ct = new CatalogTracker(this.watcher, UTIL.getConfiguration(),
-      c, this.abortable, 0);
+      c, this.abortable);
     ct.start();
     return ct;
   }
@@ -234,10 +234,8 @@ public class TestCatalogTracker {
           @Override
           public void run() {
             try {
-              metaSet.set(ct.waitForMetaServerConnectionDefault() !=  null);
+              metaSet.set(ct.waitForMetaServerConnection(100000) !=  null);
-            } catch (NotAllMetaRegionsOnlineException e) {
+            } catch (Exception e) {
-              throw new RuntimeException(e);
-            } catch (IOException e) {
               throw new RuntimeException(e);
             }
           }

hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java

@@ -195,7 +195,7 @@ public class TestMetaReaderEditorNoCluster {
         when(connection).getClient(Mockito.anyString(), Mockito.anyInt());
 
       // Now start up the catalogtracker with our doctored Connection.
-      ct = new CatalogTracker(zkw, null, connection, ABORTABLE, 0);
+      ct = new CatalogTracker(zkw, null, connection, ABORTABLE);
       ct.start();
       // Scan meta for user tables and verify we got back expected answer.
       NavigableMap<HRegionInfo, Result> hris = MetaReader.getServerUserRegions(ct, sn);

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java

@@ -116,7 +116,7 @@ public class TestCatalogJanitor {
       this.ct = Mockito.mock(CatalogTracker.class);
       AdminProtocol hri = Mockito.mock(AdminProtocol.class);
       Mockito.when(this.ct.getConnection()).thenReturn(this.connection);
-      Mockito.when(ct.waitForMetaServerConnectionDefault()).thenReturn(hri);
+      Mockito.when(ct.waitForMetaServerConnection(Mockito.anyLong())).thenReturn(hri);
     }
 
     @Override

hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java

@@ -198,7 +198,7 @@ public class TestMasterNoCluster {
 
       @Override
       CatalogTracker createCatalogTracker(ZooKeeperWatcher zk,
-          Configuration conf, Abortable abortable, int defaultTimeout)
+          Configuration conf, Abortable abortable)
       throws IOException {
         // Insert a mock for the connection used by the CatalogTracker.  Any
         // regionserver should do.  Use TESTUTIL.getConfiguration rather than
@@ -207,7 +207,7 @@ public class TestMasterNoCluster {
         HConnection connection =
           HConnectionTestingUtility.getMockedConnectionAndDecorate(TESTUTIL.getConfiguration(),
             rs0, rs0, rs0.getServerName(), HRegionInfo.ROOT_REGIONINFO);
-        return new CatalogTracker(zk, conf, connection, abortable, defaultTimeout);
+        return new CatalogTracker(zk, conf, connection, abortable);
       }
     };
     master.start();
@@ -284,7 +284,7 @@ public class TestMasterNoCluster {
 
       @Override
       CatalogTracker createCatalogTracker(ZooKeeperWatcher zk,
-          Configuration conf, Abortable abortable, int defaultTimeout)
+          Configuration conf, Abortable abortable)
       throws IOException {
         // Insert a mock for the connection used by the CatalogTracker.   Use
         // TESTUTIL.getConfiguration rather than the conf from the master; the
@@ -293,7 +293,7 @@ public class TestMasterNoCluster {
         HConnection connection =
           HConnectionTestingUtility.getMockedConnectionAndDecorate(TESTUTIL.getConfiguration(),
             rs0, rs0, rs0.getServerName(), HRegionInfo.ROOT_REGIONINFO);
-        return new CatalogTracker(zk, conf, connection, abortable, defaultTimeout);
+        return new CatalogTracker(zk, conf, connection, abortable);
       }
     };
     master.start();