You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/06 11:14:23 UTC

[GitHub] [hbase] wchevreuil commented on a change in pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…

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



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
 
   @Override
   protected boolean shouldSplit() {
-    boolean foundABigStore = false;
-
+    // If any of the stores is unable to split (eg they contain reference files)
+    // then don't split
     for (HStore store : region.getStores()) {
-      // If any of the stores are unable to split (eg they contain reference files)
-      // then don't split
-      if ((!store.canSplit())) {
+      if (!store.canSplit()) {

Review comment:
       Move this check to the for loops inside _isExceedSize_, so that we don't have to do do an extra iteration for all stores again in case none returns false for _canSplit_.

##########
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:
       We should just return this comparison and let each caller decide how to log it? That would discard the need for having an extra param just for the sake of logging.




----------------------------------------------------------------
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:
users@infra.apache.org