hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From e...@apache.org
Subject [1/2] hbase git commit: HBASE-12072 Standardize retry handling for master operations
Date Wed, 26 Nov 2014 21:51:37 GMT
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<TableName>());
+      }
+      @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<IOException>() {
         @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());


Mime
View raw message