bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450587742



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -94,4 +94,33 @@ long getDesiredMaxFileSize() {
   public boolean positiveJitterRate() {
     return this.jitterRate > 0;
   }
+
+  /**
+   * @return true if region size exceed the sizeToCheck
+   */
+  protected boolean isExceedSize(long sizeToCheck, String extraLogStr) {
+    if (overallHregionFiles) {
+      long sumSize = 0;
+      for (HStore store : region.getStores()) {
+        sumSize += store.getSize();
+      }
+      if (sumSize > sizeToCheck) {

Review comment:
       The extraLogStr exactly seems a bit strange, but there are several 
variables used by logging which calculated inside isExceedSize, if we want 
remove extraLogStr, I think there are some other ways.
   1. Use a map as param instead of extraLogStr.
   2. Change the return value from boolean to other obj such as map.
   3. Make the logging more simpler.
   
   I prefer the current way, WDYT?
   Thanks. @wchevreuil 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to