HBASE-24376 MergeNormalizer is merging non-adjacent regions and causi…

hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/AbstractRegionNormalizer.java

@@ -170,6 +170,12 @@ protected List<NormalizationPlan> getMergeNormalizationPlan(TableName table) {
  + "number of regions: {}",
  table, avgRegionSize, table, tableRegions.size());
 
+ // The list of regionInfo from getRegionsOfTable() is ordered by regionName.
+ // regionName does not necessary guarantee the order by STARTKEY (let's say 'aa1', 'aa1!',
+ // in order by regionName, it will be 'aa1!' followed by 'aa1').
+ // This could result in normalizer merging non-adjacent regions into one and creates overlaps.
+ // In order to avoid that, sort the list by RegionInfo.COMPARATOR.
+ tableRegions.sort(RegionInfo.COMPARATOR);
  final List<NormalizationPlan> plans = new ArrayList<>();
  for (int candidateIdx = 0; candidateIdx < tableRegions.size() - 1; candidateIdx++) {
  final RegionInfo hri = tableRegions.get(candidateIdx);

hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java

@@ -69,8 +69,11 @@ public String toString() {
  public void execute(Admin admin) {
  LOG.info("Executing merging normalization plan: " + this);
  try {
+ // Do not use force=true as corner cases can happen, non adjacent regions,
+ // merge with a merged child region with no GC done yet, it is going to
+ // cause all different issues.
  admin.mergeRegionsAsync(firstRegion.getEncodedNameAsBytes(),
- secondRegion.getEncodedNameAsBytes(), true);
+ secondRegion.getEncodedNameAsBytes(), false);
  } catch (IOException ex) {
  LOG.error("Error during region merge: ", ex);
  }

hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizerOnCluster.java

@@ -18,6 +18,7 @@
 package org.apache.hadoop.hbase.master.normalizer;
 
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
 
 import java.io.IOException;
 import java.util.Collections;
@@ -26,7 +27,6 @@
 import org.apache.hadoop.hbase.HBaseClassTestRule;
 import org.apache.hadoop.hbase.HBaseTestingUtility;
 import org.apache.hadoop.hbase.HConstants;
-import org.apache.hadoop.hbase.HTableDescriptor;
 import org.apache.hadoop.hbase.MetaTableAccessor;
 import org.apache.hadoop.hbase.MiniHBaseCluster;
 import org.apache.hadoop.hbase.NamespaceDescriptor;
@@ -35,6 +35,8 @@
 import org.apache.hadoop.hbase.client.Put;
 import org.apache.hadoop.hbase.client.RegionInfo;
 import org.apache.hadoop.hbase.client.Table;
+import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.client.TableDescriptorBuilder;
 import org.apache.hadoop.hbase.master.HMaster;
 import org.apache.hadoop.hbase.master.TableNamespaceManager;
 import org.apache.hadoop.hbase.master.normalizer.NormalizationPlan.PlanType;
@@ -93,7 +95,6 @@ public static void afterAllTests() throws Exception {
  }
 
  @Test
- @SuppressWarnings("deprecation")
  public void testRegionNormalizationSplitOnCluster() throws Exception {
  testRegionNormalizationSplitOnCluster(false);
  testRegionNormalizationSplitOnCluster(true);
@@ -141,9 +142,11 @@ void testRegionNormalizationSplitOnCluster(boolean limitedByQuota) throws Except
  region.flush(true);
  }
 
- HTableDescriptor htd = new HTableDescriptor(admin.getDescriptor(TABLENAME));
- htd.setNormalizationEnabled(true);
- admin.modifyTable(htd);
+ final TableDescriptor td = TableDescriptorBuilder.newBuilder(admin.getDescriptor(TABLENAME))
+ .setNormalizationEnabled(true)
+ .build();
+
+ admin.modifyTable(td);
 
  admin.flush(TABLENAME);
 
@@ -179,8 +182,71 @@ void testRegionNormalizationSplitOnCluster(boolean limitedByQuota) throws Except
  admin.deleteTable(TABLENAME);
  }
 
+ // This test is to make sure that normalizer is only going to merge adjacent regions.
+ @Test
+ public void testNormalizerCannotMergeNonAdjacentRegions() throws Exception {
+ final TableName tableName = TableName.valueOf(name.getMethodName());
+ MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
+ HMaster m = cluster.getMaster();
+
+ // create 5 regions with sizes to trigger merge of small regions
+ final byte[][] keys = {
+ Bytes.toBytes("aa"),
+ Bytes.toBytes("aa1"),
+ Bytes.toBytes("aa1!"),
+ Bytes.toBytes("aa2")
+ };
+
+ try (Table ht = TEST_UTIL.createTable(tableName, FAMILYNAME, keys)) {
+ // Need to get sorted list of regions here, the order is
+ // [, "aa"), ["aa", "aa1"), ["aa1", "aa1!"), ["aa1!", "aa2"), ["aa2", )
+ List<HRegion> generatedRegions = TEST_UTIL.getHBaseCluster().getRegions(tableName);
+ generatedRegions.sort(Comparator.comparing(HRegion::getRegionInfo, RegionInfo.COMPARATOR));
+
+ // Region ["aa", "aa1") and ["aa1!", "aa2") are not adjacent, they are not supposed to
+ // merged.
+ HRegion region = generatedRegions.get(0);
+ generateTestData(region, 3);
+ region.flush(true);
+
+ region = generatedRegions.get(1);
+ generateTestData(region, 1);
+ region.flush(true);
+
+ region = generatedRegions.get(2);
+ generateTestData(region, 3);
+ region.flush(true);
+
+ region = generatedRegions.get(3);
+ generateTestData(region, 1);
+ region.flush(true);
+
+ region = generatedRegions.get(4);
+ generateTestData(region, 5);
+ region.flush(true);
+
+ final TableDescriptor td = TableDescriptorBuilder.newBuilder(admin.getDescriptor(tableName))
+ .setNormalizationEnabled(true)
+ .build();
+ admin.modifyTable(td);
+ admin.flush(tableName);
+
+ assertEquals(5, MetaTableAccessor.getRegionCount(TEST_UTIL.getConnection(), tableName));
+
+ Thread.sleep(5000); // to let region load to update
+
+ // Compute the plan, no merge plan returned as they are not adjacent.
+ final List<NormalizationPlan> plans = m.getRegionNormalizer().computePlanForTable(tableName);
+ assertNull(plans);
+ } finally {
+ if (admin.tableExists(tableName)) {
+ admin.disableTable(tableName);
+ admin.deleteTable(tableName);
+ }
+ }
+ }
+
  @Test
- @SuppressWarnings("deprecation")
  public void testRegionNormalizationMergeOnCluster() throws Exception {
  final TableName tableName = TableName.valueOf(name.getMethodName());
  MiniHBaseCluster cluster = TEST_UTIL.getHBaseCluster();
@@ -190,7 +256,7 @@ public void testRegionNormalizationMergeOnCluster() throws Exception {
  try (Table ht = TEST_UTIL.createMultiRegionTable(tableName, FAMILYNAME, 5)) {
  // Need to get sorted list of regions here
  List<HRegion> generatedRegions = TEST_UTIL.getHBaseCluster().getRegions(tableName);
-Collections.sort(generatedRegions, Comparator.comparing(HRegion::getRegionInfo, RegionInfo.COMPARATOR));
+generatedRegions.sort(Comparator.comparing(HRegion::getRegionInfo, RegionInfo.COMPARATOR));
 
  HRegion region = generatedRegions.get(0);
  generateTestData(region, 1);
@@ -213,9 +279,11 @@ public void testRegionNormalizationMergeOnCluster() throws Exception {
  region.flush(true);
  }
 
- HTableDescriptor htd = new HTableDescriptor(admin.getDescriptor(tableName));
- htd.setNormalizationEnabled(true);
- admin.modifyTable(htd);
+ final TableDescriptor td = TableDescriptorBuilder.newBuilder(admin.getDescriptor(tableName))
+ .setNormalizationEnabled(true)
+ .build();
+
+ admin.modifyTable(td);
 
  admin.flush(tableName);