HBASE-28756 RegionSizeCalculator ignored the size of memstore, which leads Spark miss data (#6120)

Change-Id: I7449f2b776369777600c583d22b811e1931e6559
Signed-off-by: Pankaj <[email protected]>
Signed-off-by: Duo Zhang <[email protected]>

Author: Xin Sun

hbase-mapreduce/src/main/java/org/apache/hadoop/hbase/mapreduce/RegionSizeCalculator.java

@@ -82,8 +82,8 @@ private void init(RegionLocator regionLocator, Admin admin) throws IOException {
         regionLocator.getName())) {
 
         byte[] regionId = regionLoad.getRegionName();
-        long regionSizeBytes =
-          ((long) regionLoad.getStoreFileSize().get(Size.Unit.MEGABYTE)) * MEGABYTE;
+        long regionSizeBytes = ((long) (regionLoad.getStoreFileSize().get(Size.Unit.MEGABYTE)
+          + regionLoad.getMemStoreSize().get(Size.Unit.MEGABYTE))) * MEGABYTE;
 
         sizeMap.put(regionId, regionSizeBytes);
 

hbase-mapreduce/src/test/java/org/apache/hadoop/hbase/mapreduce/TestRegionSizeCalculator.java

@@ -61,19 +61,22 @@ public void testSimpleTestCase() throws Exception {
 
     RegionLocator regionLocator = mockRegionLocator("region1", "region2", "region3");
 
-    Admin admin = mockAdmin(mockRegion("region1", 123), mockRegion("region3", 1232),
-      mockRegion("region2", 54321));
+    Admin admin = mockAdmin(mockRegion("region1", 123, 321), mockRegion("region3", 1232, 2321),
+      mockRegion("region2", 54321, 12345), mockRegion("region4", 6789, 0),
+      mockRegion("region5", 0, 4567));
 
     RegionSizeCalculator calculator = new RegionSizeCalculator(regionLocator, admin);
 
-    assertEquals(123 * megabyte, calculator.getRegionSize(Bytes.toBytes("region1")));
-    assertEquals(54321 * megabyte, calculator.getRegionSize(Bytes.toBytes("region2")));
-    assertEquals(1232 * megabyte, calculator.getRegionSize(Bytes.toBytes("region3")));
+    assertEquals((123 + 321) * megabyte, calculator.getRegionSize(Bytes.toBytes("region1")));
+    assertEquals((54321 + 12345) * megabyte, calculator.getRegionSize(Bytes.toBytes("region2")));
+    assertEquals((1232 + 2321) * megabyte, calculator.getRegionSize(Bytes.toBytes("region3")));
+    assertEquals(6789 * megabyte, calculator.getRegionSize(Bytes.toBytes("region4")));
+    assertEquals(4567 * megabyte, calculator.getRegionSize(Bytes.toBytes("region5")));
 
     // if regionCalculator does not know about a region, it should return 0
     assertEquals(0, calculator.getRegionSize(Bytes.toBytes("otherTableRegion")));
 
-    assertEquals(3, calculator.getRegionSizeMap().size());
+    assertEquals(5, calculator.getRegionSizeMap().size());
   }
 
   /**
@@ -85,12 +88,12 @@ public void testLargeRegion() throws Exception {
 
     RegionLocator regionLocator = mockRegionLocator("largeRegion");
 
-    Admin admin = mockAdmin(mockRegion("largeRegion", Integer.MAX_VALUE));
+    Admin admin = mockAdmin(mockRegion("largeRegion", Integer.MAX_VALUE, Integer.MAX_VALUE));
 
     RegionSizeCalculator calculator = new RegionSizeCalculator(regionLocator, admin);
 
-    assertEquals(((long) Integer.MAX_VALUE) * megabyte,
-      calculator.getRegionSize("largeRegion".getBytes()));
+    assertEquals(((long) Integer.MAX_VALUE) * 2L * megabyte,
+      calculator.getRegionSize(Bytes.toBytes("largeRegion")));
   }
 
   /** When calculator is disabled, it should return 0 for each request. */
@@ -99,11 +102,11 @@ public void testDisabled() throws Exception {
     String regionName = "cz.goout:/index.html";
     RegionLocator table = mockRegionLocator(regionName);
 
-    Admin admin = mockAdmin(mockRegion(regionName, 999));
+    Admin admin = mockAdmin(mockRegion(regionName, 999, 888));
 
     // first request on enabled calculator
     RegionSizeCalculator calculator = new RegionSizeCalculator(table, admin);
-    assertEquals(999 * megabyte, calculator.getRegionSize(regionName.getBytes()));
+    assertEquals((999 + 888) * megabyte, calculator.getRegionSize(Bytes.toBytes(regionName)));
 
     // then disabled calculator.
     configuration.setBoolean(RegionSizeCalculator.ENABLE_REGIONSIZECALCULATOR, false);
@@ -116,7 +119,7 @@ public void testDisabled() throws Exception {
   public void testRegionWithNullServerName() throws Exception {
     RegionLocator regionLocator =
       mockRegionLocator(null, Collections.singletonList("someBigRegion"));
-    Admin admin = mockAdmin(mockRegion("someBigRegion", Integer.MAX_VALUE));
+    Admin admin = mockAdmin(mockRegion("someBigRegion", Integer.MAX_VALUE, Integer.MAX_VALUE));
     RegionSizeCalculator calculator = new RegionSizeCalculator(regionLocator, admin);
     assertEquals(0, calculator.getRegionSize(Bytes.toBytes("someBigRegion")));
   }
@@ -158,13 +161,15 @@ private Admin mockAdmin(RegionMetrics... regionLoadArray) throws Exception {
 
   /**
    * Creates mock of region with given name and size.
-   * @param fileSizeMb number of megabytes occupied by region in file store in megabytes
+   * @param fileSizeMb   number of megabytes occupied by region in file store in megabytes
+   * @param memStoreSize number of megabytes occupied by region in memstore in megabytes
    */
-  private RegionMetrics mockRegion(String regionName, int fileSizeMb) {
+  private RegionMetrics mockRegion(String regionName, int fileSizeMb, int memStoreSize) {
     RegionMetrics region = Mockito.mock(RegionMetrics.class);
     when(region.getRegionName()).thenReturn(regionName.getBytes());
     when(region.getNameAsString()).thenReturn(regionName);
     when(region.getStoreFileSize()).thenReturn(new Size(fileSizeMb, Size.Unit.MEGABYTE));
+    when(region.getMemStoreSize()).thenReturn(new Size(memStoreSize, Size.Unit.MEGABYTE));
     return region;
   }
 }