HBASE-21355 HStore's storeSize is calculated repeatedly which causing the confusing region split

hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HStore.java

@@ -288,7 +288,10 @@ public class HStore implements Store {
     }
 
     this.storeEngine = StoreEngine.create(this, this.conf, this.comparator);
-    this.storeEngine.getStoreFileManager().loadFiles(loadStoreFiles());
+    List<StoreFile> storeFiles = loadStoreFiles();
+    this.storeSize.addAndGet(getStorefilesSize(storeFiles));
+    this.totalUncompressedBytes.addAndGet(getTotalUmcompressedBytes(storeFiles));
+    this.storeEngine.getStoreFileManager().loadFiles(storeFiles);
 
     // Initialize checksum type from name. The names are CRC32, CRC32C, etc.
     this.checksumType = getChecksumType(conf);
@@ -567,9 +570,6 @@ public class HStore implements Store {
           Future<StoreFile> future = completionService.take();
           StoreFile storeFile = future.get();
           if (storeFile != null) {
-            long length = storeFile.getReader().length();
-            this.storeSize.addAndGet(length);
-            this.totalUncompressedBytes.addAndGet(storeFile.getReader().getTotalUncompressedBytes());
             if (LOG.isDebugEnabled()) {
               LOG.debug("loaded " + storeFile.toStringDetailed());
             }
@@ -2323,20 +2323,35 @@ public class HStore implements Store {
     return this.totalUncompressedBytes.get();
   }
 
-  @Override
-  public long getStorefilesSize() {
+  private long getTotalUmcompressedBytes(Collection<StoreFile> files) {
     long size = 0;
-    for (StoreFile s: this.storeEngine.getStoreFileManager().getStorefiles()) {
-      StoreFile.Reader r = s.getReader();
-      if (r == null) {
-        LOG.warn("StoreFile " + s + " has a null Reader");
-        continue;
+    for (StoreFile sf : files) {
+      if (sf != null && sf.getReader() != null) {
+        size += sf.getReader().getTotalUncompressedBytes();
       }
-      size += r.length();
     }
     return size;
   }
 
+  private long getStorefilesSize(Collection<StoreFile> files) {
+    long size = 0;
+    for (StoreFile sf : files) {
+      if (sf != null) {
+        if (sf.getReader() == null) {
+          LOG.warn("StoreFile " + sf + " has a null Reader");
+          continue;
+        }
+        size += sf.getReader().length();
+      }
+    }
+    return size;
+  }
+
+  @Override
+  public long getStorefilesSize() {
+    return getStorefilesSize(storeEngine.getStoreFileManager().getStorefiles());
+  }
+
   @Override
   public long getStorefilesIndexSize() {
     long size = 0;

hbase-server/src/test/java/org/apache/hadoop/hbase/client/TestAdmin2.java

@@ -37,6 +37,8 @@ import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.HRegionInfo;
 import org.apache.hadoop.hbase.HRegionLocation;
 import org.apache.hadoop.hbase.HTableDescriptor;
+import org.apache.hadoop.hbase.ipc.HBaseRpcController;
+import org.apache.hadoop.hbase.regionserver.Store;
 import org.apache.hadoop.hbase.testclassification.LargeTests;
 import org.apache.hadoop.hbase.MasterNotRunningException;
 import org.apache.hadoop.hbase.MiniHBaseCluster;
@@ -825,4 +827,41 @@ public class TestAdmin2 {
     admin.enableTable(TABLENAME);
     TEST_UTIL.waitUntilAllRegionsAssigned(TableName.valueOf(TABLENAME), 10000);
   }
+
+  /**
+   * TestCase for HBASE-21355
+   */
+  @Test
+  public void testGetRegionInfo() throws Exception {
+    final TableName tableName = TableName.valueOf("testGetRegionInfo");
+    Table table = TEST_UTIL.createTable(tableName, Bytes.toBytes("f"));
+    for (int i = 0; i < 100; i++) {
+      table.put(new Put(Bytes.toBytes(i)).addColumn(Bytes.toBytes("f"), Bytes.toBytes("q"),
+        Bytes.toBytes(i)));
+    }
+    admin.flush(tableName);
+
+    HRegionServer rs = TEST_UTIL.getRSForFirstRegionInTable(table.getName());
+    List<Region> regions = rs.getOnlineRegions(tableName);
+    Assert.assertEquals(1, regions.size());
+
+    Region region = regions.get(0);
+    byte[] regionName = region.getRegionInfo().getRegionName();
+    Store store = region.getStore(Bytes.toBytes("f"));
+    long expectedStoreFilesSize = store.getStorefilesSize();
+    Assert.assertNotNull(store);
+    Assert.assertEquals(expectedStoreFilesSize, store.getSize());
+
+    ClusterConnection conn = (ClusterConnection) admin.getConnection();
+    HBaseRpcController controller = conn.getRpcControllerFactory().newController();
+    for (int i = 0; i < 10; i++) {
+      HRegionInfo ri =
+          ProtobufUtil.getRegionInfo(controller, conn.getAdmin(rs.getServerName()), regionName);
+      Assert.assertEquals(region.getRegionInfo(), ri);
+      Assert.assertNull(store.getSplitPoint());
+
+      // Make sure that the store size is still the actual file system's store size.
+      Assert.assertEquals(expectedStoreFilesSize, store.getSize());
+    }
+  }
 }