Return-Path: X-Original-To: apmail-hbase-commits-archive@www.apache.org Delivered-To: apmail-hbase-commits-archive@www.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id C39A51079C for ; Wed, 26 Nov 2014 21:51:37 +0000 (UTC) Received: (qmail 87179 invoked by uid 500); 26 Nov 2014 21:51:37 -0000 Delivered-To: apmail-hbase-commits-archive@hbase.apache.org Received: (qmail 87122 invoked by uid 500); 26 Nov 2014 21:51:37 -0000 Mailing-List: contact commits-help@hbase.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@hbase.apache.org Delivered-To: mailing list commits@hbase.apache.org Received: (qmail 87113 invoked by uid 99); 26 Nov 2014 21:51:37 -0000 Received: from tyr.zones.apache.org (HELO tyr.zones.apache.org) (140.211.11.114) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 26 Nov 2014 21:51:37 +0000 Received: by tyr.zones.apache.org (Postfix, from userid 65534) id 4BC448AFC3F; Wed, 26 Nov 2014 21:51:37 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: enis@apache.org To: commits@hbase.apache.org Date: Wed, 26 Nov 2014 21:51:37 -0000 Message-Id: <32bd4604f7e145fb8c5e93903e7cea6a@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [1/2] hbase git commit: HBASE-12072 Standardize retry handling for master operations Repository: hbase Updated Branches: refs/heads/branch-1 c0988a3b9 -> 898e78661 http://git-wip-us.apache.org/repos/asf/hbase/blob/898e7866/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHBaseAdminNoCluster.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHBaseAdminNoCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHBaseAdminNoCluster.java index 526a3db..7dc614d 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHBaseAdminNoCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestHBaseAdminNoCluster.java @@ -18,8 +18,11 @@ package org.apache.hadoop.hbase.client; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; import java.io.IOException; +import java.util.ArrayList; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.HBaseConfiguration; @@ -31,10 +34,22 @@ import org.apache.hadoop.hbase.MasterNotRunningException; import org.apache.hadoop.hbase.PleaseHoldException; import org.apache.hadoop.hbase.TableName; import org.apache.hadoop.hbase.ZooKeeperConnectionException; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.BalanceRequest; import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.CreateTableRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.DispatchMergingRegionsRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.EnableCatalogJanitorRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.GetTableDescriptorsRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.GetTableNamesRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.IsCatalogJanitorEnabledRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.MoveRegionRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.OfflineRegionRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.RunCatalogScanRequest; +import org.apache.hadoop.hbase.protobuf.generated.MasterProtos.SetBalancerRunningRequest; import org.junit.Test; import org.junit.experimental.categories.Category; import org.mockito.Mockito; +import org.mockito.invocation.InvocationOnMock; +import org.mockito.stubbing.Answer; import org.mortbay.log.Log; import com.google.protobuf.RpcController; @@ -45,10 +60,10 @@ public class TestHBaseAdminNoCluster { /** * Verify that PleaseHoldException gets retried. * HBASE-8764 - * @throws IOException - * @throws ZooKeeperConnectionException - * @throws MasterNotRunningException - * @throws ServiceException + * @throws IOException + * @throws ZooKeeperConnectionException + * @throws MasterNotRunningException + * @throws ServiceException */ @Test public void testMasterMonitorCallableRetries() @@ -87,4 +102,219 @@ public class TestHBaseAdminNoCluster { if (connection != null) connection.close(); } } + + @Test + public void testMasterOperationsRetries() throws Exception { + + // Admin.listTables() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.listTables(); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .getTableDescriptors((RpcController)Mockito.any(), + (GetTableDescriptorsRequest)Mockito.any()); + } + }); + + // Admin.listTableNames() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.listTableNames(); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .getTableNames((RpcController)Mockito.any(), + (GetTableNamesRequest)Mockito.any()); + } + }); + + // Admin.getTableDescriptor() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.getTableDescriptor(TableName.valueOf("getTableDescriptor")); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .getTableDescriptors((RpcController)Mockito.any(), + (GetTableDescriptorsRequest)Mockito.any()); + } + }); + + // Admin.getTableDescriptorsByTableName() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.getTableDescriptorsByTableName(new ArrayList()); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .getTableDescriptors((RpcController)Mockito.any(), + (GetTableDescriptorsRequest)Mockito.any()); + } + }); + + // Admin.move() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.move(new byte[0], null); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .moveRegion((RpcController)Mockito.any(), + (MoveRegionRequest)Mockito.any()); + } + }); + + // Admin.offline() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.offline(new byte[0]); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .offlineRegion((RpcController)Mockito.any(), + (OfflineRegionRequest)Mockito.any()); + } + }); + + // Admin.setBalancerRunning() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.setBalancerRunning(true, true); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .setBalancerRunning((RpcController)Mockito.any(), + (SetBalancerRunningRequest)Mockito.any()); + } + }); + + // Admin.balancer() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.balancer(); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .balance((RpcController)Mockito.any(), + (BalanceRequest)Mockito.any()); + } + }); + + // Admin.enabledCatalogJanitor() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.enableCatalogJanitor(true); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .enableCatalogJanitor((RpcController)Mockito.any(), + (EnableCatalogJanitorRequest)Mockito.any()); + } + }); + + // Admin.runCatalogScan() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.runCatalogScan(); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .runCatalogScan((RpcController)Mockito.any(), + (RunCatalogScanRequest)Mockito.any()); + } + }); + + // Admin.isCatalogJanitorEnabled() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.isCatalogJanitorEnabled(); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .isCatalogJanitorEnabled((RpcController)Mockito.any(), + (IsCatalogJanitorEnabledRequest)Mockito.any()); + } + }); + + // Admin.mergeRegions() + testMasterOperationIsRetried(new MethodCaller() { + @Override + public void call(Admin admin) throws Exception { + admin.mergeRegions(new byte[0], new byte[0], true); + } + @Override + public void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception { + Mockito.verify(masterAdmin, Mockito.atLeast(count)) + .dispatchMergingRegions((RpcController)Mockito.any(), + (DispatchMergingRegionsRequest)Mockito.any()); + } + }); + } + + private static interface MethodCaller { + void call(Admin admin) throws Exception; + void verify(MasterKeepAliveConnection masterAdmin, int count) throws Exception; + } + + private void testMasterOperationIsRetried(MethodCaller caller) throws Exception { + Configuration configuration = HBaseConfiguration.create(); + // Set the pause and retry count way down. + configuration.setLong(HConstants.HBASE_CLIENT_PAUSE, 1); + final int count = 10; + configuration.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, count); + + ClusterConnection connection = mock(ClusterConnection.class); + when(connection.getConfiguration()).thenReturn(configuration); + MasterKeepAliveConnection masterAdmin = + Mockito.mock(MasterKeepAliveConnection.class, new Answer() { + @Override + public Object answer(InvocationOnMock invocation) throws Throwable { + if (invocation.getMethod().getName().equals("close")) { + return null; + } + throw new MasterNotRunningException(); // all methods will throw an exception + } + }); + Mockito.when(connection.getKeepAliveMasterService()).thenReturn(masterAdmin); + + Admin admin = null; + try { + admin = new HBaseAdmin(connection); + + try { + caller.call(admin); // invoke the HBaseAdmin method + fail(); + } catch (RetriesExhaustedException e) { + Log.info("Expected fail", e); + } + // Assert we were called 'count' times. + caller.verify(masterAdmin, count); + } finally { + if (admin != null) {admin.close();} + } + } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/hbase/blob/898e7866/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java index ac6c178..6fff9d7 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestReplicasClient.java @@ -28,7 +28,6 @@ import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.HRegionInfo; import org.apache.hadoop.hbase.HTableDescriptor; -import org.apache.hadoop.hbase.MasterNotRunningException; import org.apache.hadoop.hbase.MediumTests; import org.apache.hadoop.hbase.NotServingRegionException; import org.apache.hadoop.hbase.RegionLocations; @@ -181,15 +180,6 @@ public class TestReplicasClient { TestRegionServerNoMaster.stopMasterAndAssignMeta(HTU); Configuration c = new Configuration(HTU.getConfiguration()); c.setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 1); - Admin ha = new HBaseAdmin(c); - for (boolean masterRuns = true; masterRuns; ) { - Thread.sleep(100); - try { - masterRuns = false; - masterRuns = ha.isMasterRunning(); - } catch (MasterNotRunningException ignored) { - } - } LOG.info("Master has stopped"); } http://git-wip-us.apache.org/repos/asf/hbase/blob/898e7866/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java index bed999e..a039d54 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMaster.java @@ -45,6 +45,7 @@ import org.apache.hadoop.hbase.client.HTable; import org.apache.hadoop.hbase.protobuf.generated.ZooKeeperProtos; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.Pair; +import org.apache.hadoop.util.StringUtils; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -63,6 +64,8 @@ public class TestMaster { @BeforeClass public static void beforeAllTests() throws Exception { + // we will retry operations when PleaseHoldException is thrown + TEST_UTIL.getConfiguration().setInt(HConstants.HBASE_CLIENT_RETRIES_NUMBER, 3); // Start a cluster of two regionservers. TEST_UTIL.startMiniCluster(2); admin = TEST_UTIL.getHBaseAdmin(); @@ -174,7 +177,7 @@ public class TestMaster { admin.move(tableRegions.get(0).getEncodedNameAsBytes(), null); fail("Region should not be moved since master is not initialized"); } catch (IOException ioe) { - assertTrue(ioe instanceof PleaseHoldException); + assertTrue(StringUtils.stringifyException(ioe).contains("PleaseHoldException")); } finally { master.initialized = true; TEST_UTIL.deleteTable(tableName); http://git-wip-us.apache.org/repos/asf/hbase/blob/898e7866/hbase-server/src/test/java/org/apache/hadoop/hbase/migration/TestNamespaceUpgrade.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/migration/TestNamespaceUpgrade.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/migration/TestNamespaceUpgrade.java index 046290d..d030b72 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/migration/TestNamespaceUpgrade.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/migration/TestNamespaceUpgrade.java @@ -242,12 +242,8 @@ public class TestNamespaceUpgrade { TEST_UTIL.waitFor(30000, new Waiter.Predicate() { @Override public boolean evaluate() throws IOException { - try { - return TEST_UTIL.getHBaseAdmin().getCompactionState(newTableName) == - AdminProtos.GetRegionInfoResponse.CompactionState.NONE; - } catch (InterruptedException e) { - throw new IOException(e); - } + return TEST_UTIL.getHBaseAdmin().getCompactionState(newTableName) == + AdminProtos.GetRegionInfoResponse.CompactionState.NONE; } }); } http://git-wip-us.apache.org/repos/asf/hbase/blob/898e7866/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java index 34c9ec1..b867da6 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionMergeTransactionOnCluster.java @@ -59,6 +59,8 @@ import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.util.PairOfSameType; +import org.apache.hadoop.util.StringUtils; +import org.apache.zookeeper.KeeperException; import org.junit.AfterClass; import org.junit.BeforeClass; import org.junit.Test; @@ -261,8 +263,9 @@ public class TestRegionMergeTransactionOnCluster { admin.mergeRegions(a.getEncodedNameAsBytes(), b.getEncodedNameAsBytes(), false); fail("Offline regions should not be able to merge"); } catch (IOException ie) { + System.out.println(ie); assertTrue("Exception should mention regions not online", - ie.getMessage().contains("regions not online") + StringUtils.stringifyException(ie).contains("regions not online") && ie instanceof MergeRegionException); } try { @@ -271,7 +274,7 @@ public class TestRegionMergeTransactionOnCluster { fail("A region should not be able to merge with itself, even forcifully"); } catch (IOException ie) { assertTrue("Exception should mention regions not online", - ie.getMessage().contains("region to itself") + StringUtils.stringifyException(ie).contains("region to itself") && ie instanceof MergeRegionException); } try { http://git-wip-us.apache.org/repos/asf/hbase/blob/898e7866/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java ---------------------------------------------------------------------- diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java index e9212ad..ba1b1bb 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestSplitTransactionOnCluster.java @@ -40,7 +40,6 @@ import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Abortable; import org.apache.hadoop.hbase.CoordinatedStateManager; import org.apache.hadoop.hbase.Coprocessor; -import org.apache.hadoop.hbase.HBaseIOException; import org.apache.hadoop.hbase.HBaseTestingUtility; import org.apache.hadoop.hbase.HColumnDescriptor; import org.apache.hadoop.hbase.HConstants; @@ -1268,7 +1267,7 @@ public class TestSplitTransactionOnCluster { */ private int ensureTableRegionNotOnSameServerAsMeta(final Admin admin, final HRegionInfo hri) - throws HBaseIOException, MasterNotRunningException, + throws IOException, MasterNotRunningException, ZooKeeperConnectionException, InterruptedException { // Now make sure that the table region is not on same server as that hosting // hbase:meta We don't want hbase:meta replay polluting our test when we later crash http://git-wip-us.apache.org/repos/asf/hbase/blob/898e7866/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java ---------------------------------------------------------------------- diff --git a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java index c30d96a..f1539ef 100644 --- a/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java +++ b/hbase-thrift/src/main/java/org/apache/hadoop/hbase/thrift/ThriftServerRunner.java @@ -685,8 +685,6 @@ public class ThriftServerRunner implements Runnable { public void compact(ByteBuffer tableNameOrRegionName) throws IOError { try{ getHBaseAdmin().compact(getBytes(tableNameOrRegionName)); - } catch (InterruptedException e) { - throw new IOError(e.getMessage()); } catch (IOException e) { LOG.warn(e.getMessage(), e); throw new IOError(e.getMessage()); @@ -697,9 +695,6 @@ public class ThriftServerRunner implements Runnable { public void majorCompact(ByteBuffer tableNameOrRegionName) throws IOError { try{ getHBaseAdmin().majorCompact(getBytes(tableNameOrRegionName)); - } catch (InterruptedException e) { - LOG.warn(e.getMessage(), e); - throw new IOError(e.getMessage()); } catch (IOException e) { LOG.warn(e.getMessage(), e); throw new IOError(e.getMessage());