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 2020/08/25 07:16:52 UTC

[hbase] branch branch-2 updated: HBASE-24942 MergeTableRegionsProcedure should not call clean merge region (#2301)

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

zhangduo 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 e61a346  HBASE-24942 MergeTableRegionsProcedure should not call clean merge region (#2301)
e61a346 is described below

commit e61a3460a771760fe2121ad4f7877ec98c40b148
Author: Duo Zhang <zh...@apache.org>
AuthorDate: Tue Aug 25 12:19:19 2020 +0800

    HBASE-24942 MergeTableRegionsProcedure should not call clean merge region (#2301)
    
    Signed-off-by: stack <st...@apache.org>
---
 .../org/apache/hadoop/hbase/MetaTableAccessor.java | 11 +++++++++--
 .../apache/hadoop/hbase/master/CatalogJanitor.java | 23 ++--------------------
 .../assignment/MergeTableRegionsProcedure.java     |  9 +++++----
 .../org/apache/hadoop/hbase/client/TestAdmin1.java |  2 +-
 .../hbase/client/TestAsyncRegionAdminApi2.java     |  2 +-
 5 files changed, 18 insertions(+), 29 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 dd0d1ff..e43987d 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
@@ -383,8 +383,15 @@ public class MetaTableAccessor {
   }
 
   /**
-   * @return Deserialized values of <qualifier,regioninfo> pairs taken from column values that match
-   *   the regex 'info:merge.*' in array of <code>cells</code>.
+   * Check whether the given {@code regionName} has any 'info:merge*' columns.
+   */
+  public static boolean hasMergeRegions(Connection conn, byte[] regionName) throws IOException {
+    return hasMergeRegions(getRegionResult(conn, regionName).rawCells());
+  }
+
+  /**
+   * @return Deserialized values of &lt;qualifier,regioninfo&gt; pairs taken from column values that
+   *         match the regex 'info:merge.*' in array of <code>cells</code>.
    */
   @Nullable
   public static Map<String, RegionInfo> getMergeRegionsWithName(Cell [] cells) {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
index 7567713..cef816f 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/CatalogJanitor.java
@@ -161,7 +161,8 @@ public class CatalogJanitor extends ScheduledChore {
    * garbage to collect.
    * @return How many items gc'd whether for merge or split.
    */
-  int scan() throws IOException {
+  @VisibleForTesting
+  public int scan() throws IOException {
     int gcs = 0;
     try {
       if (!alreadyRunning.compareAndSet(false, true)) {
@@ -412,26 +413,6 @@ public class CatalogJanitor extends ScheduledChore {
   }
 
   /**
-   * Checks if the specified region has merge qualifiers, if so, try to clean them.
-   * @return true if no info:merge* columns; i.e. the specified region doesn't have
-   *   any merge qualifiers.
-   */
-  public boolean cleanMergeQualifier(final RegionInfo region) throws IOException {
-    // Get merge regions if it is a merged region and already has merge qualifier
-    List<RegionInfo> parents = MetaTableAccessor.getMergeRegions(this.services.getConnection(),
-        region.getRegionName());
-    if (parents == null || parents.isEmpty()) {
-      // It doesn't have merge qualifier, no need to clean
-      return true;
-    }
-
-    // If a parent region is a merged child region and GC has not kicked in/finish its work yet,
-    // return false in this case to avoid kicking in a merge, trying later.
-    cleanMergeRegion(region, parents);
-    return false;
-  }
-
-  /**
    * Report made by ReportMakingVisitor
    */
   public static class Report {
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java
index 68db7b2..d9c44f9 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/MergeTableRegionsProcedure.java
@@ -27,6 +27,7 @@ import java.util.stream.Stream;
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.MetaMutationAnnotation;
+import org.apache.hadoop.hbase.MetaTableAccessor;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.UnknownRegionException;
@@ -472,12 +473,12 @@ public class MergeTableRegionsProcedure
       return false;
     }
 
-    CatalogJanitor catalogJanitor = env.getMasterServices().getCatalogJanitor();
     RegionStates regionStates = env.getAssignmentManager().getRegionStates();
-    for (RegionInfo ri: this.regionsToMerge) {
-      if (!catalogJanitor.cleanMergeQualifier(ri)) {
+    for (RegionInfo ri : this.regionsToMerge) {
+      if (MetaTableAccessor.hasMergeRegions(env.getMasterServices().getConnection(),
+        ri.getRegionName())) {
         String msg = "Skip merging " + RegionInfo.getShortNameToLog(regionsToMerge) +
-            ", because a parent, " + RegionInfo.getShortNameToLog(ri) + ", has a merge qualifier " +
+          ", because a parent, " + RegionInfo.getShortNameToLog(ri) + ", has a merge qualifier " +
           "(if a 'merge column' in parent, it was recently merged but still has outstanding " +
           "references to its parents that must be cleared before it can participate in merge -- " +
           "major compact it to hurry clearing of its references)";
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 0637e69..77e59cf 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
@@ -553,7 +553,7 @@ public class TestAdmin1 extends TestAdminBase {
       // Need to wait GC for merged child region is done.
       HMaster services = TEST_UTIL.getHBaseCluster().getMaster();
       CatalogJanitor cj = services.getCatalogJanitor();
-      cj.cleanMergeQualifier(mergedChildRegion);
+      assertTrue(cj.scan() > 0);
       // Wait until all procedures settled down
       while (!services.getMasterProcedureExecutor().getActiveProcIds().isEmpty()) {
         Thread.sleep(200);
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 ef73b20..9689fe4 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
@@ -189,7 +189,7 @@ public class TestAsyncRegionAdminApi2 extends TestAsyncAdminBase {
     // Need to wait GC for merged child region is done.
     HMaster services = TEST_UTIL.getHBaseCluster().getMaster();
     CatalogJanitor cj = services.getCatalogJanitor();
-    cj.cleanMergeQualifier(mergedChildRegion);
+    assertTrue(cj.scan() > 0);
     // Wait until all procedures settled down
     while (!services.getMasterProcedureExecutor().getActiveProcIds().isEmpty()) {
       Thread.sleep(200);