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



##########
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:
       >Make the logic more clear.
   If can not split, then we do not need to get tableRegionsCount and 
sizeToCheck in IncreasingToUpperBoundRegionSplitPolicy, and also avoid other 
comparsion in loop too.
   
   That's a problem for IncreasingToUpperBoundRegionSplitPolicy. We shouldn't 
penalise a parent class because of subclasses implementation specifics. And you 
are already overriding this method on IncreasingToUpperBoundRegionSplitPolicy 
to do the extra loop anyways, so why keep this extra, needless loop here?




----------------------------------------------------------------
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