diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/AbstractRegionNormalizer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/AbstractRegionNormalizer.java index 9a9f5f228bca..8b17f77dc914 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/AbstractRegionNormalizer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/AbstractRegionNormalizer.java @@ -170,6 +170,12 @@ protected List 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 plans = new ArrayList<>(); for (int candidateIdx = 0; candidateIdx < tableRegions.size() - 1; candidateIdx++) { final RegionInfo hri = tableRegions.get(candidateIdx); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java index 7c33661d7c34..a2938a9f0b15 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/MergeNormalizationPlan.java +++ b/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); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizerOnCluster.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizerOnCluster.java index 6f7f69eb9a96..9199d3ff1447 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizerOnCluster.java +++ b/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.getTableDescriptor(TABLENAME)); - htd.setNormalizationEnabled(true); - admin.modifyTable(TABLENAME, 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 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 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 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.getTableDescriptor(tableName)); - htd.setNormalizationEnabled(true); - admin.modifyTable(tableName, htd); + final TableDescriptor td = TableDescriptorBuilder.newBuilder(admin.getDescriptor(tableName)) + .setNormalizationEnabled(true) + .build(); + + admin.modifyTable(td); admin.flush(tableName);