You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by zh...@apache.org on 2019/03/18 13:23:05 UTC

[hbase] branch branch-2.2 updated: HBASE-22040 Add mergeRegionsAsync with a List of region names method in AsyncAdmin

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

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


The following commit(s) were added to refs/heads/branch-2.2 by this push:
     new 8a5101a  HBASE-22040 Add mergeRegionsAsync with a List of region names method in AsyncAdmin
8a5101a is described below

commit 8a5101af2186443ae919805933a5fbb4085b1468
Author: zhangduo <zh...@apache.org>
AuthorDate: Mon Mar 18 20:48:38 2019 +0800

    HBASE-22040 Add mergeRegionsAsync with a List of region names method in AsyncAdmin
    
    Signed-off-by: Zheng Hu <op...@gmail.com>
---
 .../java/org/apache/hadoop/hbase/client/Admin.java | 28 +++++----
 .../org/apache/hadoop/hbase/client/AsyncAdmin.java | 19 +++++-
 .../hadoop/hbase/client/AsyncHBaseAdmin.java       |  5 +-
 .../org/apache/hadoop/hbase/client/HBaseAdmin.java | 37 +++--------
 .../hadoop/hbase/client/RawAsyncHBaseAdmin.java    | 72 +++++++++++-----------
 .../hadoop/hbase/master/MasterRpcServices.java     |  5 +-
 .../org/apache/hadoop/hbase/client/TestAdmin1.java | 46 ++++++++++++++
 .../hbase/client/TestAsyncRegionAdminApi2.java     | 58 ++++++++++++++---
 8 files changed, 178 insertions(+), 92 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
index 99d1d96..104a4f6 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/Admin.java
@@ -1297,29 +1297,31 @@ public interface Admin extends Abortable, Closeable {
 
   /**
    * Merge two regions. Asynchronous operation.
-   *
    * @param nameOfRegionA encoded or full name of region a
    * @param nameOfRegionB encoded or full name of region b
-   * @param forcible <code>true</code> if do a compulsory merge, otherwise we will only merge
-   *          two adjacent regions
-   * @throws IOException
+   * @param forcible <code>true</code> if do a compulsory merge, otherwise we will only merge two
+   *          adjacent regions
    */
-  Future<Void> mergeRegionsAsync(
-      byte[] nameOfRegionA,
-      byte[] nameOfRegionB,
-      boolean forcible) throws IOException;
+  default Future<Void> mergeRegionsAsync(byte[] nameOfRegionA, byte[] nameOfRegionB,
+      boolean forcible) throws IOException {
+    byte[][] nameofRegionsToMerge = new byte[2][];
+    nameofRegionsToMerge[0] = nameOfRegionA;
+    nameofRegionsToMerge[1] = nameOfRegionB;
+    return mergeRegionsAsync(nameofRegionsToMerge, forcible);
+  }
 
   /**
    * Merge regions. Asynchronous operation.
-   *
+   * <p/>
+   * You may get a {@code DoNotRetryIOException} if you pass more than two regions in but the master
+   * does not support merging more than two regions. At least till 2.2.0, we still only support
+   * merging two regions.
    * @param nameofRegionsToMerge encoded or full name of daughter regions
    * @param forcible <code>true</code> if do a compulsory merge, otherwise we will only merge
    *          adjacent regions
-   * @throws IOException
    */
-  Future<Void> mergeRegionsAsync(
-      byte[][] nameofRegionsToMerge,
-      boolean forcible) throws IOException;
+  Future<Void> mergeRegionsAsync(byte[][] nameofRegionsToMerge, boolean forcible)
+      throws IOException;
 
   /**
    * Split a table. The method will execute split action for each region in table.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
index 185d262..4f32bb5 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncAdmin.java
@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.client;
 
 import com.google.protobuf.RpcChannel;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.EnumSet;
 import java.util.List;
@@ -494,8 +495,22 @@ public interface AsyncAdmin {
    * @param forcible true if do a compulsory merge, otherwise we will only merge two adjacent
    *          regions
    */
-  CompletableFuture<Void> mergeRegions(byte[] nameOfRegionA, byte[] nameOfRegionB,
-      boolean forcible);
+  default CompletableFuture<Void> mergeRegions(byte[] nameOfRegionA, byte[] nameOfRegionB,
+      boolean forcible) {
+    return mergeRegions(Arrays.asList(nameOfRegionA, nameOfRegionB), forcible);
+  }
+
+  /**
+   * Merge regions.
+   * <p/>
+   * You may get a {@code DoNotRetryIOException} if you pass more than two regions in but the master
+   * does not support merging more than two regions. At least till 2.2.0, we still only support
+   * merging two regions.
+   * @param nameOfRegionsToMerge encoded or full name of daughter regions
+   * @param forcible true if do a compulsory merge, otherwise we will only merge two adjacent
+   *          regions
+   */
+  CompletableFuture<Void> mergeRegions(List<byte[]> nameOfRegionsToMerge, boolean forcible);
 
   /**
    * Split a table. The method will execute split action for each region in table.
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java
index 26eb5dc..29e74b9 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/AsyncHBaseAdmin.java
@@ -315,9 +315,8 @@ class AsyncHBaseAdmin implements AsyncAdmin {
   }
 
   @Override
-  public CompletableFuture<Void> mergeRegions(byte[] nameOfRegionA, byte[] nameOfRegionB,
-      boolean forcible) {
-    return wrap(rawAdmin.mergeRegions(nameOfRegionA, nameOfRegionB, forcible));
+  public CompletableFuture<Void> mergeRegions(List<byte[]> nameOfRegionsToMerge, boolean forcible) {
+    return wrap(rawAdmin.mergeRegions(nameOfRegionsToMerge, forcible));
   }
 
   @Override
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
index 83b0342..9c96f81 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/HBaseAdmin.java
@@ -111,6 +111,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting;
+import org.apache.hbase.thirdparty.com.google.common.base.Preconditions;
 import org.apache.hbase.thirdparty.com.google.protobuf.ServiceException;
 
 import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil;
@@ -1739,40 +1740,20 @@ public class HBaseAdmin implements Admin {
 
   /**
    * Merge two regions. Asynchronous operation.
-   * @param nameOfRegionA encoded or full name of region a
-   * @param nameOfRegionB encoded or full name of region b
-   * @param forcible true if do a compulsory merge, otherwise we will only merge
-   *          two adjacent regions
-   * @throws IOException
-   */
-  @Override
-  public Future<Void> mergeRegionsAsync(
-      final byte[] nameOfRegionA,
-      final byte[] nameOfRegionB,
-      final boolean forcible) throws IOException {
-    byte[][] nameofRegionsToMerge = new byte[2][];
-    nameofRegionsToMerge[0] = nameOfRegionA;
-    nameofRegionsToMerge[1] = nameOfRegionB;
-    return mergeRegionsAsync(nameofRegionsToMerge, forcible);
-  }
-
-  /**
-   * Merge two regions. Asynchronous operation.
    * @param nameofRegionsToMerge encoded or full name of daughter regions
    * @param forcible true if do a compulsory merge, otherwise we will only merge
    *          adjacent regions
-   * @throws IOException
    */
   @Override
-  public Future<Void> mergeRegionsAsync(
-      final byte[][] nameofRegionsToMerge,
-      final boolean forcible) throws IOException {
-    assert(nameofRegionsToMerge.length >= 2);
+  public Future<Void> mergeRegionsAsync(final byte[][] nameofRegionsToMerge, final boolean forcible)
+      throws IOException {
+    Preconditions.checkArgument(nameofRegionsToMerge.length >= 2, "Can not merge only %s region",
+      nameofRegionsToMerge.length);
     byte[][] encodedNameofRegionsToMerge = new byte[nameofRegionsToMerge.length][];
-    for(int i = 0; i < nameofRegionsToMerge.length; i++) {
-      encodedNameofRegionsToMerge[i] = HRegionInfo.isEncodedRegionName(nameofRegionsToMerge[i]) ?
-          nameofRegionsToMerge[i] :
-          Bytes.toBytes(HRegionInfo.encodeRegionName(nameofRegionsToMerge[i]));
+    for (int i = 0; i < nameofRegionsToMerge.length; i++) {
+      encodedNameofRegionsToMerge[i] =
+        RegionInfo.isEncodedRegionName(nameofRegionsToMerge[i]) ? nameofRegionsToMerge[i]
+          : Bytes.toBytes(RegionInfo.encodeRegionName(nameofRegionsToMerge[i]));
     }
 
     TableName tableName = null;
diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
index 5612864..8fd8d09 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RawAsyncHBaseAdmin.java
@@ -1162,13 +1162,12 @@ class RawAsyncHBaseAdmin implements AsyncAdmin {
     });
   }
 
-  private CompletableFuture<TableName> checkRegionsAndGetTableName(byte[] encodeRegionNameA,
-      byte[] encodeRegionNameB) {
+  private CompletableFuture<TableName> checkRegionsAndGetTableName(byte[][] encodedRegionNames) {
     AtomicReference<TableName> tableNameRef = new AtomicReference<>();
     CompletableFuture<TableName> future = new CompletableFuture<>();
-
-    checkAndGetTableName(encodeRegionNameA, tableNameRef, future);
-    checkAndGetTableName(encodeRegionNameB, tableNameRef, future);
+    for (byte[] encodedRegionName : encodedRegionNames) {
+      checkAndGetTableName(encodedRegionName, tableNameRef, future);
+    }
     return future;
   }
 
@@ -1218,41 +1217,42 @@ class RawAsyncHBaseAdmin implements AsyncAdmin {
   }
 
   @Override
-  public CompletableFuture<Void> mergeRegions(byte[] nameOfRegionA, byte[] nameOfRegionB,
-      boolean forcible) {
+  public CompletableFuture<Void> mergeRegions(List<byte[]> nameOfRegionsToMerge, boolean forcible) {
+    if (nameOfRegionsToMerge.size() < 2) {
+      return failedFuture(new IllegalArgumentException(
+        "Can not merge only " + nameOfRegionsToMerge.size() + " region"));
+    }
     CompletableFuture<Void> future = new CompletableFuture<>();
-    final byte[] encodeRegionNameA = toEncodeRegionName(nameOfRegionA);
-    final byte[] encodeRegionNameB = toEncodeRegionName(nameOfRegionB);
+    byte[][] encodedNameOfRegionsToMerge =
+      nameOfRegionsToMerge.stream().map(this::toEncodeRegionName).toArray(byte[][]::new);
 
-    addListener(checkRegionsAndGetTableName(encodeRegionNameA, encodeRegionNameB),
-      (tableName, err) -> {
-        if (err != null) {
-          future.completeExceptionally(err);
-          return;
-        }
+    addListener(checkRegionsAndGetTableName(encodedNameOfRegionsToMerge), (tableName, err) -> {
+      if (err != null) {
+        future.completeExceptionally(err);
+        return;
+      }
 
-        MergeTableRegionsRequest request = null;
-        try {
-          request = RequestConverter.buildMergeTableRegionsRequest(
-            new byte[][] { encodeRegionNameA, encodeRegionNameB }, forcible, ng.getNonceGroup(),
-            ng.newNonce());
-        } catch (DeserializationException e) {
-          future.completeExceptionally(e);
-          return;
-        }
+      MergeTableRegionsRequest request = null;
+      try {
+        request = RequestConverter.buildMergeTableRegionsRequest(encodedNameOfRegionsToMerge,
+          forcible, ng.getNonceGroup(), ng.newNonce());
+      } catch (DeserializationException e) {
+        future.completeExceptionally(e);
+        return;
+      }
 
-        addListener(
-          this.<MergeTableRegionsRequest, MergeTableRegionsResponse> procedureCall(tableName,
-            request, (s, c, req, done) -> s.mergeTableRegions(c, req, done),
-            (resp) -> resp.getProcId(), new MergeTableRegionProcedureBiConsumer(tableName)),
-          (ret, err2) -> {
-            if (err2 != null) {
-              future.completeExceptionally(err2);
-            } else {
-              future.complete(ret);
-            }
-          });
-      });
+      addListener(
+        this.<MergeTableRegionsRequest, MergeTableRegionsResponse> procedureCall(tableName, request,
+          (s, c, req, done) -> s.mergeTableRegions(c, req, done), (resp) -> resp.getProcId(),
+          new MergeTableRegionProcedureBiConsumer(tableName)),
+        (ret, err2) -> {
+          if (err2 != null) {
+            future.completeExceptionally(err2);
+          } else {
+            future.complete(ret);
+          }
+        });
+    });
     return future;
   }
 
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
index 0dab709..c0d42bb 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -790,7 +790,10 @@ public class MasterRpcServices extends RSRpcServices
 
     RegionStates regionStates = master.getAssignmentManager().getRegionStates();
 
-    assert(request.getRegionCount() == 2);
+    if (request.getRegionCount() != 2) {
+      throw new ServiceException(new DoNotRetryIOException(
+        "Only support merging 2 regions but " + request.getRegionCount() + " region passed"));
+    }
     RegionInfo[] regionsToMerge = new RegionInfo[request.getRegionCount()];
     for (int i = 0; i < request.getRegionCount(); i++) {
       final byte[] encodedNameOfRegion = request.getRegion(i).getValue().toByteArray();
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
index dfc3a2c..9484be6 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin1.java
@@ -29,8 +29,10 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HColumnDescriptor;
@@ -1437,6 +1439,50 @@ public class TestAdmin1 {
   }
 
   @Test
+  public void testMergeRegionsInvalidRegionCount()
+      throws IOException, InterruptedException, ExecutionException {
+    TableName tableName = TableName.valueOf(name.getMethodName());
+    TableDescriptor td = TableDescriptorBuilder.newBuilder(tableName)
+      .setColumnFamily(ColumnFamilyDescriptorBuilder.of("d")).build();
+    byte[][] splitRows = new byte[2][];
+    splitRows[0] = new byte[] { (byte) '3' };
+    splitRows[1] = new byte[] { (byte) '6' };
+    try {
+      TEST_UTIL.createTable(td, splitRows);
+      TEST_UTIL.waitTableAvailable(tableName);
+
+      List<RegionInfo> tableRegions = admin.getRegions(tableName);
+      // 0
+      try {
+        admin.mergeRegionsAsync(new byte[0][0], false).get();
+        fail();
+      } catch (IllegalArgumentException e) {
+        // expected
+      }
+      // 1
+      try {
+        admin.mergeRegionsAsync(new byte[][] { tableRegions.get(0).getEncodedNameAsBytes() }, false)
+          .get();
+        fail();
+      } catch (IllegalArgumentException e) {
+        // expected
+      }
+      // 3
+      try {
+        admin.mergeRegionsAsync(
+          tableRegions.stream().map(RegionInfo::getEncodedNameAsBytes).toArray(byte[][]::new),
+          false).get();
+        fail();
+      } catch (DoNotRetryIOException e) {
+        // expected
+      }
+    } finally {
+      admin.disableTable(tableName);
+      admin.deleteTable(tableName);
+    }
+  }
+
+  @Test
   public void testSplitShouldNotHappenIfSplitIsDisabledForTable()
       throws Exception {
     final TableName tableName = TableName.valueOf(name.getMethodName());
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
index 1642eec..bb0b813 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAsyncRegionAdminApi2.java
@@ -17,7 +17,22 @@
  */
 package org.apache.hadoop.hbase.client;
 
+import static org.apache.hadoop.hbase.TableName.META_TABLE_NAME;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Collectors;
 import org.apache.hadoop.hbase.AsyncMetaTableAccessor;
+import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionLocation;
@@ -33,15 +48,6 @@ import org.junit.experimental.categories.Category;
 import org.junit.runner.RunWith;
 import org.junit.runners.Parameterized;
 
-import java.util.ArrayList;
-import java.util.List;
-import java.util.Optional;
-
-import static org.apache.hadoop.hbase.TableName.META_TABLE_NAME;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertTrue;
-
 /**
  * Class to test asynchronous region admin operations.
  * @see TestAsyncRegionAdminApi This test and it used to be joined it was taking longer than our
@@ -178,6 +184,40 @@ public class TestAsyncRegionAdminApi2 extends TestAsyncAdminBase {
   }
 
   @Test
+  public void testMergeRegionsInvalidRegionCount() throws InterruptedException {
+    byte[][] splitRows = new byte[][] { Bytes.toBytes("3"), Bytes.toBytes("6") };
+    createTableWithDefaultConf(tableName, splitRows);
+    List<RegionInfo> regions = admin.getRegions(tableName).join();
+    // 0
+    try {
+      admin.mergeRegions(Collections.emptyList(), false).get();
+      fail();
+    } catch (ExecutionException e) {
+      // expected
+      assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
+    }
+    // 1
+    try {
+      admin.mergeRegions(regions.stream().limit(1).map(RegionInfo::getEncodedNameAsBytes)
+        .collect(Collectors.toList()), false).get();
+      fail();
+    } catch (ExecutionException e) {
+      // expected
+      assertThat(e.getCause(), instanceOf(IllegalArgumentException.class));
+    }
+    // 3
+    try {
+      admin.mergeRegions(
+        regions.stream().map(RegionInfo::getEncodedNameAsBytes).collect(Collectors.toList()), false)
+        .get();
+      fail();
+    } catch (ExecutionException e) {
+      // expected
+      assertThat(e.getCause(), instanceOf(DoNotRetryIOException.class));
+    }
+  }
+
+  @Test
   public void testSplitTable() throws Exception {
     initSplitMergeSwitch();
     splitTest(TableName.valueOf("testSplitTable"), 3000, false, null);