You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hbase.apache.org by hu...@apache.org on 2020/07/14 22:48:34 UTC

[hbase] branch master updated: HBASE-24705 MetaFixer#fixHoles() does not include the case for read replicas (i.e, replica regions are not created) (#2062)

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

huaxiangsun pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/hbase.git


The following commit(s) were added to refs/heads/master by this push:
     new 1360bee  HBASE-24705 MetaFixer#fixHoles() does not include the case for read replicas (i.e, replica regions are not created) (#2062)
1360bee is described below

commit 1360bee7f95988f5b3058982a1d1d994cd929051
Author: huaxiangsun <hu...@apache.org>
AuthorDate: Tue Jul 14 15:48:19 2020 -0700

    HBASE-24705 MetaFixer#fixHoles() does not include the case for read replicas (i.e, replica regions are not created) (#2062)
    
    Signed-off-by: Viraj Jasani <vj...@apache.org>
---
 .../hadoop/hbase/client/RegionReplicaUtil.java     |  5 ++-
 .../org/apache/hadoop/hbase/master/MetaFixer.java  | 32 ++++++++++++++-----
 .../master/procedure/CreateTableProcedure.java     |  2 +-
 .../master/procedure/EnableTableProcedure.java     |  2 +-
 .../apache/hadoop/hbase/HBaseTestingUtility.java   | 13 ++++++++
 .../apache/hadoop/hbase/master/TestMetaFixer.java  | 37 +++++++++++++++-------
 6 files changed, 66 insertions(+), 25 deletions(-)

diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionReplicaUtil.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionReplicaUtil.java
index 3aefb0a..09150f1 100644
--- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionReplicaUtil.java
+++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RegionReplicaUtil.java
@@ -147,14 +147,13 @@ public class RegionReplicaUtil {
   /**
    * Create any replicas for the regions (the default replicas that was already created is passed to
    * the method)
-   * @param tableDescriptor descriptor to use
    * @param regions existing regions
    * @param oldReplicaCount existing replica count
    * @param newReplicaCount updated replica count due to modify table
    * @return the combined list of default and non-default replicas
    */
-  public static List<RegionInfo> addReplicas(final TableDescriptor tableDescriptor,
-      final List<RegionInfo> regions, int oldReplicaCount, int newReplicaCount) {
+  public static List<RegionInfo> addReplicas(final List<RegionInfo> regions, int oldReplicaCount,
+    int newReplicaCount) {
     if ((newReplicaCount - 1) <= 0) {
       return regions;
     }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java
index a928e68..cc8a3db 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetaFixer.java
@@ -32,10 +32,13 @@ import org.apache.hadoop.hbase.MetaTableAccessor;
 import org.apache.hadoop.hbase.TableName;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.RegionInfoBuilder;
+import org.apache.hadoop.hbase.client.RegionReplicaUtil;
+import org.apache.hadoop.hbase.client.TableDescriptor;
 import org.apache.hadoop.hbase.exceptions.MergeRegionException;
 import org.apache.hadoop.hbase.master.assignment.TransitRegionStateProcedure;
 import org.apache.hadoop.hbase.util.Bytes;
 import org.apache.hadoop.hbase.util.Pair;
+import org.apache.hadoop.hbase.util.ServerRegionReplicaUtil;
 import org.apache.yetus.audience.InterfaceAudience;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -174,22 +177,35 @@ class MetaFixer {
   private static List<RegionInfo> createMetaEntries(final MasterServices masterServices,
     final List<RegionInfo> newRegionInfos) {
 
-    final List<Either<RegionInfo, IOException>> addMetaEntriesResults = newRegionInfos.stream()
-      .map(regionInfo -> {
+    final List<Either<List<RegionInfo>, IOException>> addMetaEntriesResults = newRegionInfos.
+      stream().map(regionInfo -> {
         try {
-          MetaTableAccessor.addRegionToMeta(masterServices.getConnection(), regionInfo);
-          masterServices.getAssignmentManager()
-            .getRegionStates()
-            .updateRegionState(regionInfo, RegionState.State.CLOSED);
-          return Either.<RegionInfo, IOException>ofLeft(regionInfo);
+          TableDescriptor td = masterServices.getTableDescriptors().get(regionInfo.getTable());
+
+          // Add replicas if needed
+          // we need to create regions with replicaIds starting from 1
+          List<RegionInfo> newRegions = RegionReplicaUtil.addReplicas(
+            Collections.singletonList(regionInfo), 1, td.getRegionReplication());
+
+          // Add regions to META
+          MetaTableAccessor.addRegionsToMeta(masterServices.getConnection(), newRegions,
+            td.getRegionReplication());
+
+          // Setup replication for region replicas if needed
+          if (td.getRegionReplication() > 1) {
+            ServerRegionReplicaUtil.setupRegionReplicaReplication(
+              masterServices.getConfiguration());
+          }
+          return Either.<List<RegionInfo>, IOException>ofLeft(newRegions);
         } catch (IOException e) {
-          return Either.<RegionInfo, IOException>ofRight(e);
+          return Either.<List<RegionInfo>, IOException>ofRight(e);
         }
       })
       .collect(Collectors.toList());
     final List<RegionInfo> createMetaEntriesSuccesses = addMetaEntriesResults.stream()
       .filter(Either::hasLeft)
       .map(Either::getLeft)
+      .flatMap(List::stream)
       .collect(Collectors.toList());
     final List<IOException> createMetaEntriesFailures = addMetaEntriesResults.stream()
       .filter(Either::hasRight)
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
index 0cd9972..b48cef8 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CreateTableProcedure.java
@@ -369,7 +369,7 @@ public class CreateTableProcedure
 
     // Add replicas if needed
     // we need to create regions with replicaIds starting from 1
-    List<RegionInfo> newRegions = RegionReplicaUtil.addReplicas(tableDescriptor, regions, 1,
+    List<RegionInfo> newRegions = RegionReplicaUtil.addReplicas(regions, 1,
       tableDescriptor.getRegionReplication());
 
     // Add regions to META
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java
index a71e467..67afa7f 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/EnableTableProcedure.java
@@ -145,7 +145,7 @@ public class EnableTableProcedure
             LOG.info("Number of replicas has increased. Assigning new region replicas." +
                 "The previous replica count was {}. The current replica count is {}.",
                 (currentMaxReplica + 1), configuredReplicaCount);
-            regionsOfTable = RegionReplicaUtil.addReplicas(tableDescriptor, regionsOfTable,
+            regionsOfTable = RegionReplicaUtil.addReplicas(regionsOfTable,
               currentMaxReplica + 1, configuredReplicaCount);
           }
           // Assign all the table regions. (including region replicas if added).
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
index e73dce5..0b5f816 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/HBaseTestingUtility.java
@@ -1531,6 +1531,19 @@ public class HBaseTestingUtility extends HBaseZKTestingUtility {
   }
 
   /**
+   * Create a table with multiple regions.
+   * @param tableName
+   * @param replicaCount replica count.
+   * @param families
+   * @return A Table instance for the created table.
+   * @throws IOException
+   */
+  public Table createMultiRegionTable(TableName tableName, int replicaCount, byte[][] families)
+    throws IOException {
+    return createTable(tableName, families, KEYS_FOR_HBA_CREATE_TABLE, replicaCount);
+  }
+
+  /**
    * Create a table.
    * @param tableName
    * @param families
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java
index bc69f93..fe329f3 100644
--- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java
+++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMetaFixer.java
@@ -83,10 +83,9 @@ public class TestMetaFixer {
     services.getAssignmentManager().getRegionStates().deleteRegion(ri);
   }
 
-  @Test
-  public void testPlugsHoles() throws Exception {
-    TableName tn = TableName.valueOf(this.name.getMethodName());
-    TEST_UTIL.createMultiRegionTable(tn, HConstants.CATALOG_FAMILY);
+  private void testPlugsHolesWithReadReplicaInternal(final TableName tn, final int replicaCount)
+    throws Exception {
+    TEST_UTIL.createMultiRegionTable(tn, replicaCount, new byte[][] { HConstants.CATALOG_FAMILY });
     List<RegionInfo> ris = MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), tn);
     MasterServices services = TEST_UTIL.getHBaseCluster().getMaster();
     int initialSize = services.getAssignmentManager().getRegionStates().getRegionStates().size();
@@ -94,12 +93,14 @@ public class TestMetaFixer {
     CatalogJanitor.Report report = services.getCatalogJanitor().getLastReport();
     assertTrue(report.isEmpty());
     int originalCount = ris.size();
-    // Remove first, last and middle region. See if hole gets plugged. Table has 26 regions.
-    deleteRegion(services, ris.get(ris.size() -1));
-    deleteRegion(services, ris.get(3));
-    deleteRegion(services, ris.get(0));
-    assertEquals(initialSize - 3,
-        services.getAssignmentManager().getRegionStates().getRegionStates().size());
+    // Remove first, last and middle region. See if hole gets plugged. Table has 26 * replicaCount regions.
+    for (int i = 0; i < replicaCount; i ++) {
+      deleteRegion(services, ris.get(3 * replicaCount + i));
+      deleteRegion(services, ris.get(i));
+      deleteRegion(services, ris.get(ris.size() - 1 - i));
+    }
+    assertEquals(initialSize - 3 * replicaCount,
+      services.getAssignmentManager().getRegionStates().getRegionStates().size());
     services.getCatalogJanitor().scan();
     report = services.getCatalogJanitor().getLastReport();
     assertEquals(report.toString(), 3, report.getHoles().size());
@@ -109,17 +110,29 @@ public class TestMetaFixer {
     report = services.getCatalogJanitor().getLastReport();
     assertTrue(report.toString(), report.isEmpty());
     assertEquals(initialSize,
-        services.getAssignmentManager().getRegionStates().getRegionStates().size());
+      services.getAssignmentManager().getRegionStates().getRegionStates().size());
 
     // wait for RITs to settle -- those are the fixed regions being assigned -- or until the
     // watchdog TestRule terminates the test.
     HBaseTestingUtility.await(50,
-      () -> isNotEmpty(services.getAssignmentManager().getRegionsInTransition()));
+      () -> services.getMasterProcedureExecutor().getActiveProcIds().size() == 0);
 
     ris = MetaTableAccessor.getTableRegions(TEST_UTIL.getConnection(), tn);
     assertEquals(originalCount, ris.size());
   }
 
+  @Test
+  public void testPlugsHoles() throws Exception {
+    TableName tn = TableName.valueOf(this.name.getMethodName());
+    testPlugsHolesWithReadReplicaInternal(tn, 1);
+  }
+
+  @Test
+  public void testPlugsHolesWithReadReplica() throws Exception {
+    TableName tn = TableName.valueOf(this.name.getMethodName());
+    testPlugsHolesWithReadReplicaInternal(tn, 3);
+  }
+
   /**
    * Just make sure running fixMeta does right thing for the case
    * of a single-region Table where the region gets dropped.