HBASE-29356 Incorrect split behavior when region information is missing

Author: qqvpp

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

@@ -322,14 +322,24 @@ private double getAverageRegionSizeMb(final List<RegionInfo> tableRegions,
       avgRegionSize = targetRegionSize;
     } else {
       final int regionCount = tableRegions.size();
-      final long totalSizeMb = tableRegions.stream().mapToLong(this::getRegionSizeMB).sum();
+      // Count of regions whose size is known
+      int regionCountKnownSize = 0;
+      long totalSizeMb = 0;
+      for (RegionInfo regionInfo : tableRegions) {
+        long regionSize = getRegionSizeMB(regionInfo);
+        if(regionSize != -1) {
+          totalSizeMb += regionSize;
+          regionCountKnownSize++;
+        }
+      }
       if (targetRegionCount > 0) {
-        avgRegionSize = totalSizeMb / (double) targetRegionCount;
+        avgRegionSize = totalSizeMb / (double) targetRegionCount - (regionCount - regionCountKnownSize);
       } else {
-        avgRegionSize = totalSizeMb / (double) regionCount;
+        avgRegionSize = totalSizeMb / (double) regionCountKnownSize;
       }
-      LOG.debug("Table {}, total aggregated regions size: {} MB and average region size {} MB",
-        table, totalSizeMb, String.format("%.3f", avgRegionSize));
+      LOG.debug("Table {}, total aggregated regions size: {} MB and average region size {} MB, "
+          + "number of regions with unknown size {}",
+        table, totalSizeMb, String.format("%.3f", avgRegionSize), regionCount - regionCountKnownSize);
     }
 
     return avgRegionSize;
@@ -393,6 +403,12 @@ private List<NormalizationPlan> computeMergeNormalizationPlans(final NormalizeCo
       for (current = rangeStart; current < ctx.getTableRegions().size(); current++) {
         final RegionInfo regionInfo = ctx.getTableRegions().get(current);
         final long regionSizeMb = getRegionSizeMB(regionInfo);
+        if(regionSizeMb == -1) {
+          LOG.debug("For region {} in table {} cannot determine size, skipping check merging region",
+            regionInfo.getRegionNameAsString(), ctx.getTableName());
+          rangeStart = Math.max(current, rangeStart + 1);
+          continue;
+        }
         if (skipForMerge(configuration, ctx, regionInfo)) {
           // this region cannot participate in a range. resume the outer loop.
           rangeStart = Math.max(current, rangeStart + 1);
@@ -457,6 +473,11 @@ private List<NormalizationPlan> computeSplitNormalizationPlans(final NormalizeCo
         continue;
       }
       final long regionSizeMb = getRegionSizeMB(hri);
+      if (regionSizeMb == -1) {
+        LOG.debug("For region {} in table {} cannot determine size, skipping check splitting region",
+          hri.getRegionNameAsString(), ctx.getTableName());
+        continue;
+      }
       if (regionSizeMb > 2 * avgRegionSize) {
         LOG.info(
           "Table {}, large region {} has size {} MB, more than twice avg size {} MB, "
@@ -490,7 +511,12 @@ private static boolean isOldEnoughForMerge(final NormalizerConfiguration normali
    */
   private boolean isLargeEnoughForMerge(final NormalizerConfiguration normalizerConfiguration,
     final NormalizeContext ctx, final RegionInfo regionInfo) {
-    return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
+    long regionSizeMb = getRegionSizeMB(regionInfo);
+    if( regionSizeMb==-1) {
+      return false;
+    } else {
+      return regionSizeMb >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
+    }
   }
 
   /**

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

@@ -32,9 +32,11 @@
 import static org.hamcrest.Matchers.empty;
 import static org.hamcrest.Matchers.everyItem;
 import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.hasItem;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -676,13 +678,28 @@ private void setupMocksForNormalizer(Map<byte[], Integer> regionSizes,
     ServerName sn = ServerName.valueOf("localhost", 0, 0L);
     when(masterServices.getAssignmentManager().getRegionStates().getRegionsOfTable(any()))
       .thenReturn(regionInfoList);
-    when(masterServices.getAssignmentManager().getRegionStates().getRegionServerOfRegion(any()))
-      .thenReturn(sn);
+    for(RegionInfo regionInfo : regionInfoList) {
+      int regionSize = regionSizes.get(regionInfo.getRegionName());
+      // ensures that the getRegionSizeMB method returns -1, simulating the condition when the region size cannot be found
+      if(regionSize == -1) {
+        when(masterServices.getAssignmentManager().getRegionStates()
+          .getRegionServerOfRegion(regionInfo))
+          .thenReturn(null);
+      } else {
+        when(masterServices.getAssignmentManager().getRegionStates()
+          .getRegionServerOfRegion(regionInfo))
+          .thenReturn(sn);
+      }
+    }
     when(
       masterServices.getAssignmentManager().getRegionStates().getRegionState(any(RegionInfo.class)))
       .thenReturn(RegionState.createForTesting(null, RegionState.State.OPEN));
 
     for (Map.Entry<byte[], Integer> region : regionSizes.entrySet()) {
+      // skip regions where we don't know the size
+      if(region.getValue() == -1) {
+        continue;
+      }
       RegionMetrics regionLoad = Mockito.mock(RegionMetrics.class);
       when(regionLoad.getRegionName()).thenReturn(region.getKey());
       when(regionLoad.getStoreFileSize())
@@ -757,4 +774,25 @@ private static Map<byte[], Integer> createRegionSizesMap(final List<RegionInfo>
     }
     return ret;
   }
+
+  @Test
+  public void testSplitOfLargeRegionIfOneIsNotKnow() {
+    final TableName tableName = name.getTableName();
+    final List<RegionInfo> regionInfos = createRegionInfos(tableName, 5);
+    final Map<byte[], Integer> regionSizes = createRegionSizesMap(regionInfos,  8, 6, -1, 10, 30);
+
+    setupMocksForNormalizer(regionSizes, regionInfos);
+
+    assertThat(normalizer.computePlansForTable(tableDescriptor),
+      contains(new SplitNormalizationPlan(regionInfos.get(4), 30)));
+  }
+
+  @Test
+  public void testSplitOfAllUnknownSize() {
+    final TableName tableName = name.getTableName();
+    final List<RegionInfo> regionInfos = createRegionInfos(tableName, 4);
+    final Map<byte[], Integer> regionSizes = createRegionSizesMap(regionInfos, -1, -1, -1, -1);
+    setupMocksForNormalizer(regionSizes, regionInfos);
+    assertThat(normalizer.computePlansForTable(tableDescriptor), is(empty()));
+  }
 }