You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by st...@apache.org on 2020/02/28 21:51:28 UTC

[hbase] branch branch-2 updated: HBASE-23904 Procedure updating meta and Master shutdown are incompatible: CODE-BUG

This is an automated email from the ASF dual-hosted git repository.

stack pushed a commit to branch branch-2
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/branch-2 by this push:
     new 836e1a1  HBASE-23904 Procedure updating meta and Master shutdown are incompatible: CODE-BUG
836e1a1 is described below

commit 836e1a1caf1611cceb4054884d36788da021b155
Author: stack <st...@apache.org>
AuthorDate: Wed Feb 26 22:16:11 2020 -0800

    HBASE-23904 Procedure updating meta and Master shutdown are incompatible: CODE-BUG
    
    Restore behavior from before HBASE-21789 (hbase-2.2.0) where we convert
    all exceptions to IOEs, even RuntimeExceptions. Actual fix is this change (in case
    obscured by doc and lambda simplification):
    
         } catch (Throwable e) {
    -      Throwables.propagateIfPossible(e, IOException.class);
    +      // Throw if an IOE else wrap in an IOE EVEN IF IT IS a RuntimeException (e.g.
    +      // a RejectedExecutionException because the hosting exception is shutting down.
    +      // This is old behavior worth reexamining. Procedures doing merge or split
    +      // currently don't handle RuntimeExceptions coming up out of meta table edits.
    +      // Would have to work on this at least. See HBASE-23904.
    +      Throwables.throwIfInstanceOf(e, IOException.class);
---
 .../org/apache/hadoop/hbase/MetaTableAccessor.java | 68 +++++++++++-----------
 .../hbase/TestMetaTableAccessorNoCluster.java      | 19 ++++++
 2 files changed, 54 insertions(+), 33 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
index d74f771..ac79e8a 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/MetaTableAccessor.java
@@ -232,8 +232,7 @@ public class MetaTableAccessor {
    * @param connection connection we're using to access Meta
    * @return An {@link Table} for <code>hbase:meta</code>
    */
-  public static Table getMetaHTable(final Connection connection)
-  throws IOException {
+  public static Table getMetaHTable(final Connection connection) throws IOException {
     // We used to pass whole CatalogTracker in here, now we just pass in Connection
     if (connection == null) {
       throw new NullPointerException("No connection");
@@ -1733,44 +1732,47 @@ public class MetaTableAccessor {
   }
 
   /**
-   * Performs an atomic multi-mutate operation against the given table.
+   * Performs an atomic multi-mutate operation against the given table. Used by the likes of
+   * merge and split as these want to make atomic mutations across multiple rows.
+   * @throws IOException even if we encounter a RuntimeException, we'll still wrap it in an IOE.
    */
-  private static void multiMutate(final Table table, byte[] row, final List<Mutation> mutations)
+  @VisibleForTesting
+  static void multiMutate(final Table table, byte[] row, final List<Mutation> mutations)
       throws IOException {
     debugLogMutations(mutations);
-    Batch.Call<MultiRowMutationService, MutateRowsResponse> callable =
-      new Batch.Call<MultiRowMutationService, MutateRowsResponse>() {
-
-        @Override
-        public MutateRowsResponse call(MultiRowMutationService instance) throws IOException {
-          MutateRowsRequest.Builder builder = MutateRowsRequest.newBuilder();
-          for (Mutation mutation : mutations) {
-            if (mutation instanceof Put) {
-              builder.addMutationRequest(
-                ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.PUT, mutation));
-            } else if (mutation instanceof Delete) {
-              builder.addMutationRequest(
-                ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.DELETE, mutation));
-            } else {
-              throw new DoNotRetryIOException(
-                "multi in MetaEditor doesn't support " + mutation.getClass().getName());
-            }
-          }
-          ServerRpcController controller = new ServerRpcController();
-          CoprocessorRpcUtils.BlockingRpcCallback<MutateRowsResponse> rpcCallback =
-            new CoprocessorRpcUtils.BlockingRpcCallback<>();
-          instance.mutateRows(controller, builder.build(), rpcCallback);
-          MutateRowsResponse resp = rpcCallback.get();
-          if (controller.failedOnException()) {
-            throw controller.getFailedOn();
-          }
-          return resp;
+    Batch.Call<MultiRowMutationService, MutateRowsResponse> callable = instance -> {
+      MutateRowsRequest.Builder builder = MutateRowsRequest.newBuilder();
+      for (Mutation mutation : mutations) {
+        if (mutation instanceof Put) {
+          builder.addMutationRequest(
+            ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.PUT, mutation));
+        } else if (mutation instanceof Delete) {
+          builder.addMutationRequest(
+            ProtobufUtil.toMutation(ClientProtos.MutationProto.MutationType.DELETE, mutation));
+        } else {
+          throw new DoNotRetryIOException(
+            "multi in MetaEditor doesn't support " + mutation.getClass().getName());
         }
-      };
+      }
+      ServerRpcController controller = new ServerRpcController();
+      CoprocessorRpcUtils.BlockingRpcCallback<MutateRowsResponse> rpcCallback =
+        new CoprocessorRpcUtils.BlockingRpcCallback<>();
+      instance.mutateRows(controller, builder.build(), rpcCallback);
+      MutateRowsResponse resp = rpcCallback.get();
+      if (controller.failedOnException()) {
+        throw controller.getFailedOn();
+      }
+      return resp;
+    };
     try {
       table.coprocessorService(MultiRowMutationService.class, row, row, callable);
     } catch (Throwable e) {
-      Throwables.propagateIfPossible(e, IOException.class);
+      // Throw if an IOE else wrap in an IOE EVEN IF IT IS a RuntimeException (e.g.
+      // a RejectedExecutionException because the hosting exception is shutting down.
+      // This is old behavior worth reexamining. Procedures doing merge or split
+      // currently don't handle RuntimeExceptions coming up out of meta table edits.
+      // Would have to work on this at least. See HBASE-23904.
+      Throwables.throwIfInstanceOf(e, IOException.class);
       throw new IOException(e);
     }
   }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessorNoCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessorNoCluster.java
index b4f963c..ca03cb6 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessorNoCluster.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/TestMetaTableAccessorNoCluster.java
@@ -23,6 +23,7 @@ import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 import java.util.NavigableMap;
 import org.apache.hadoop.hbase.client.ClusterConnection;
@@ -30,7 +31,10 @@ import org.apache.hadoop.hbase.client.HConnectionTestingUtility;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.RegionInfoBuilder;
 import org.apache.hadoop.hbase.client.Result;
+import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.client.coprocessor.Batch;
 import org.apache.hadoop.hbase.ipc.HBaseRpcController;
+import org.apache.hadoop.hbase.protobuf.generated.MultiRowMutationProtos;
 import org.apache.hadoop.hbase.testclassification.MiscTests;
 import org.apache.hadoop.hbase.testclassification.SmallTests;
 import org.apache.hadoop.hbase.util.Bytes;
@@ -40,7 +44,9 @@ import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
 import org.junit.experimental.categories.Category;
+import org.mockito.Mock;
 import org.mockito.Mockito;
+import org.mockito.internal.matchers.Any;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 import org.slf4j.Logger;
@@ -88,6 +94,19 @@ public class TestMetaTableAccessorNoCluster {
     UTIL.shutdownMiniZKCluster();
   }
 
+  /**
+   * Expect a IOE to come out of multiMutate, even if down in the depths we throw
+   * a RuntimeException. See HBASE-23904
+   */
+  @Test (expected = IOException.class)
+  public void testMultiMutate() throws Throwable {
+    Table table = Mockito.mock(Table.class);
+    Mockito.when(table.coprocessorService(Mockito.any(),
+      Mockito.any(byte [].class), Mockito.any(byte [].class), Mockito.any(Batch.Call.class))).
+      thenThrow(new RuntimeException("FAIL TEST WITH RuntimeException!"));
+    MetaTableAccessor.multiMutate(table, HConstants.LAST_ROW, Collections.emptyList());
+  }
+
   @Test
   public void testGetHRegionInfo() throws IOException {
     assertNull(MetaTableAccessor.getRegionInfo(new Result()));