From ec117ea67de7a9a3a313dcc780fd0ede37df1ba3 Mon Sep 17 00:00:00 2001 From: larsh Date: Wed, 16 Jan 2013 22:10:46 +0000 Subject: [PATCH] HBASE-7578 TestCatalogTracker hangs occasionally git-svn-id: https://svn.apache.org/repos/asf/hbase/trunk@1434436 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop/hbase/catalog/CatalogTracker.java | 73 ++----------------- .../apache/hadoop/hbase/master/HMaster.java | 7 +- .../hbase/regionserver/HRegionServer.java | 3 +- .../hbase/catalog/TestCatalogTracker.java | 8 +- .../TestMetaReaderEditorNoCluster.java | 2 +- .../hbase/master/TestCatalogJanitor.java | 2 +- .../hbase/master/TestMasterNoCluster.java | 8 +- 7 files changed, 18 insertions(+), 85 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java index 8a383e4b91f..da1f0c983f4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/catalog/CatalogTracker.java +++ b/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, - 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 0 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); + this(zk, conf, HConnectionManager.getConnection(conf), abortable); } 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 .META. 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). */ diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index 17d26fa4979..ee4e6fdb5c0 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/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, conf.getInt("hbase.master.catalog.timeout", 600000)); + this.catalogTracker = createCatalogTracker(this.zooKeeper, this.conf, this); 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 diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index 2c6ce076920..da7ade968aa 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/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, this.conf.getInt("hbase.regionserver.catalog.timeout", 600000)); + this.catalogTracker = new CatalogTracker(this.zooKeeper, this.conf, this); catalogTracker.start(); // Retrieve clusterId diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java index 046671cde0a..5bab3ac39b2 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestCatalogTracker.java +++ b/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); - } catch (NotAllMetaRegionsOnlineException e) { - throw new RuntimeException(e); - } catch (IOException e) { + metaSet.set(ct.waitForMetaServerConnection(100000) != null); + } catch (Exception e) { throw new RuntimeException(e); } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java index 7db46b6e996..8845de58e71 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/catalog/TestMetaReaderEditorNoCluster.java +++ b/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 hris = MetaReader.getServerUserRegions(ct, sn); diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java index 271145a908e..aa9d6704cfd 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestCatalogJanitor.java +++ b/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 diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java index a458242894b..12302aa8382 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterNoCluster.java +++ b/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();