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/06/10 11:58:39 UTC

[GitHub] [hbase] bsglz opened a new pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

bsglz opened a new pull request #1883:
URL: https://github.com/apache/hbase/pull/1883


   … but count all store size


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651595433


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 31s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  4s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   5m 50s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 39s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 51s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 39s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   7m 26s |  hbase-server in the patch failed.  |
   |  |   |  32m 41s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 3b742d9a5b20 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 982bd5fadd |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/testReport/ |
   | Max. process+thread count | 536 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz edited a comment on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz edited a comment on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651576692


   > 2020-06-30 13:46:23,017 DEBUG [Time-limited test] util.ClassSize(421): Primitives=127, arrays=0, **references=56**, refSize 4, size=368, prealign_size=363
   > 
   > java.lang.AssertionError: 
   > Expected :368
   > Actual   :360
   
   Checked the log of TestHeapSize.testSizes, seems there are 56*references, but the FIXED_OVERHEAD in HRegion only calc 55?
   If change it to 56, this ut could pass in my local env, but not ture should do? @saintstack 


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



[GitHub] [hbase] bsglz commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r440882731



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       > Yes, and maybe we should raise a discussion on dev list about making _ IncreasingToUpperBoundRegionSplitPolicy_ deprecated?
   
   Agree. Could you raise it? Thanks.




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



[GitHub] [hbase] wchevreuil commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651722529


   > @wchevreuil
   > Thought it again, if this new splitPolicy is useable in branch-2.2 and branch-2.3, but disappear in branch-2.4 without deprecated cycle, is it break the compatibility guarantee?
   
   Yeah, we can't simply remove it between minor releases.
    
   > IMO, maybe we can do as below:
   > 1: In all actived branchs, add this new splitPolicy.
   
   Agreed!
   
   
   > 2: In minor branch set it as default policy and mark SteppingSpliyPolicy and IncreasingToUpperBoundRegionSplitPolicy as deprecated, at the same time fix the ConstantSizeRegionSplitPolicy.
   
   Just to confirm my understanding of your proposal: adding the new policy as default, deprecation of the overridden ones and fix of ConstantSizeRegionSplitPolicy should go on master and branch-2 branches, whilst branch-2.2 and branch-2.3 would only have the new policy added? If so, I'm +1 for that.
   
   


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-641977561


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 32s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  1s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 41s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  5s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   1m 59s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 21s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  4s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m  4s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 10s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  32m 13s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux c506ac5dabf6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d8247ebae3 |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-642111035


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   3m 23s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  7s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 15s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 37s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 50s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 40s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 42s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 50s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 213m 50s |  hbase-server in the patch passed.  |
   |  |   | 246m 27s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f669b1a4e252 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d8247ebae3 |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/testReport/ |
   | Max. process+thread count | 3039 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] wchevreuil commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r439392669



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       I wonder if this is how _IncreasingToUpperBoundRegionSplitPolicy_ should actually behave. For scenarios where _IncreasingToUpperBoundRegionSplitPolicy.getSizeToCheck()_ returns *hbase.hregion.max.filesize* , it doesn't make much sense to me to check on individual stores only, as regions with many stores may then grow much larger then *hbase.hregion.max.filesize*. 




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



[GitHub] [hbase] wchevreuil commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r440937343



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       Sure, will send it later.




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



[GitHub] [hbase] virajjasani commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r441468251



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);

Review comment:
       Can you use placeholders with `{}` for all arguments?




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



[GitHub] [hbase] virajjasani commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r441556282



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       @wchevreuil You are fine with the PR getting merged before raising the discussion over dev group? Or good to wait for the discussion?




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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651497104


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 38s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  2s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 32s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  3s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 15s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   2m 35s |  root in the patch failed.  |
   | -1 :x: |  compile  |   1m  8s |  hbase-server in the patch failed.  |
   | -0 :warning: |  javac  |   1m  8s |  hbase-server in the patch failed.  |
   | -1 :x: |  shadedjars  |   5m 26s |  patch has 48 errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   1m 14s |  hbase-server in the patch failed.  |
   |  |   |  25m 36s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f1dd1991c7c4 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / bfec964aef |
   | Default Java | 1.8.0_232 |
   | mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk8-hadoop3-check/output/patch-mvninstall-root.txt |
   | compile | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
   | javac | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-hbase-server.txt |
   | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/testReport/ |
   | Max. process+thread count | 88 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] wchevreuil commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-648078417


   @bsglz , sorry for the delay. Let's give one more day for the DISCUSS thread. 


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651518122


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 13s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  5s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  4s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 39s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 42s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  9s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  6s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  6s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 26s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 44s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   9m 51s |  hbase-server in the patch failed.  |
   |  |   |  37m 15s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 47630ae12c4c 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / bfec964aef |
   | Default Java | 1.8.0_232 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/testReport/ |
   | Max. process+thread count | 457 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651576692


   > 2020-06-30 13:46:23,017 DEBUG [Time-limited test] util.ClassSize(421): Primitives=127, arrays=0, **references=56**, refSize 4, size=368, prealign_size=363
   > 
   > java.lang.AssertionError: 
   > Expected :368
   > Actual   :360
   
   Checked the log of TestHeapSize.testSizes, seems there are 56*references, but the FIXED_OVERHEAD in HRegion only calc 55?


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



[GitHub] [hbase] bsglz commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r441502228



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);

Review comment:
       Fixed, thanks.




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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-645363559


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 51s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 27s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 20s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 17s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  4s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m 11s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  14m  9s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m 26s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  39m 40s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 6341da09f0dd 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / b17ba7b81d |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-646493243


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651494787


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 24s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m 19s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m 17s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   2m 15s |  root in the patch failed.  |
   | +1 :green_heart: |  checkstyle  |   1m 13s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | -1 :x: |  hadoopcheck  |   2m 22s |  The patch causes 48 errors with Hadoop v3.1.2.  |
   | -1 :x: |  hadoopcheck  |   4m 47s |  The patch causes 48 errors with Hadoop v3.2.1.  |
   | -1 :x: |  spotbugs  |   0m 39s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 11s |  The patch does not generate ASF License warnings.  |
   |  |   |  19m 55s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 14ea6ff82563 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / bfec964aef |
   | mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
   | hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-general-check/output/patch-javac-3.1.2.txt |
   | hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
   | spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz edited a comment on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz edited a comment on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-650316101


   > @bsglz , sorry for the delay. So majority of people on the DISCUSS thread opined towards #3. I think @busbey suggestion to fix IncreasingToUpperBoundRegionSplitPolicy for minor updates only is reasonable, so maybe we can merge this current one to branch-2.2 and branch-2.3, then for master and branch-2 we fix IncreasingToUpperBoundRegionSplitPolicy.
   
   Ok, will do it in a follow-on issue. Thanks.


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651598770


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 42s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   5m  9s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 17s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m  9s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 40s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 59s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  2s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 23s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 36s |  the patch passed  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   9m 30s |  hbase-server in the patch failed.  |
   |  |   |  38m 48s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 406a357b1d92 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 982bd5fadd |
   | Default Java | 1.8.0_232 |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/testReport/ |
   | Max. process+thread count | 449 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-652153958


   @wchevreuil  Filed a new issue for follow-on tasks, FYI. https://issues.apache.org/jira/browse/HBASE-24664


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-645444304


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 52s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 50s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   7m  4s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 45s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 26s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 10s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 43s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 41s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 145m 21s |  hbase-server in the patch passed.  |
   |  |   | 175m 14s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 18f8e4773c8e 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / b17ba7b81d |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/testReport/ |
   | Max. process+thread count | 4298 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz edited a comment on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz edited a comment on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651576692


   > 2020-06-30 13:46:23,017 DEBUG [Time-limited test] util.ClassSize(421): Primitives=127, arrays=0, **references=56**, refSize 4, size=368, prealign_size=363
   > 
   > java.lang.AssertionError: 
   > Expected :368
   > Actual   :360
   
   Checked the log of TestHeapSize.testSizes, seems there are 56*references, but the FIXED_OVERHEAD in HRegion only calc 55?
   If change it to 56, this ut could pass in my local env, but not sure should do? @saintstack 


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-649320798


   @wchevreuil  Seems there are different opinions, how about merge this first, and do more improvement in seperate jira when we reach consensus.
   


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-646579266


   @wchevreuil For now the default split policy is SteppingSplitPolicy, FYI.


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



[GitHub] [hbase] bsglz commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r440598275



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       @wchevreuil Maybe we'd better to introduce this new split policy as optional first, and make it as default in future if it works well?




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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-642115789


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   3m 23s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 16s |  master passed  |
   | +1 :green_heart: |  compile  |   0m 59s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 10s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 56s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 59s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 59s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 10s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 38s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 225m  2s |  hbase-server in the patch passed.  |
   |  |   | 254m  1s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 383c77ed7f2a 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / d8247ebae3 |
   | Default Java | 1.8.0_232 |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/testReport/ |
   | Max. process+thread count | 3210 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/1/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-647871195


   @wchevreuil @virajjasani Skimed the comments of the thread, seems the #3 is mostly supported, should we continue discussion or do as #3?


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



[GitHub] [hbase] wchevreuil commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-657418801


   We had decided to go with approach from HBASE-24664. 


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651517009


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 16s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 42s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 11s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 15s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  2s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  3s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  4s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 42s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 38s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   6m 58s |  hbase-server in the patch failed.  |
   |  |   |  33m 53s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f1d0e31728a3 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / bfec964aef |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/testReport/ |
   | Max. process+thread count | 527 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r439454248



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       It seems make sense when write to each store not evenly, but the parameter name exactly confused.
   Maybe we should introduce a new parameter _hbase.region.max.size_ for this new policy and rename the _hbase.hregion.max.filesize_ to _hbase.region.store.max.size_ for the exist policies  in follow-on issue. WDYT?
   Thanks.




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



[GitHub] [hbase] wchevreuil commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r440853944



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       > Thought it agian, split by store size cause the actual region split size uncontrollable and easy to misunderstand, as hbase do more and more better on many columnfamilies, the disadvantages will more and more obvious, so i think we should change the default behavior to split by all reigon size.
   
   That's my concern. It may cause regions to grow much beyond the desired _hbase.hregion.max.filesize_.
   
   > Maybe we'd better to introduce this new split policy as optional first, and make it as default in future if it works well?
   
   Yes, and maybe we should raise a discussion on dev list about making _ IncreasingToUpperBoundRegionSplitPolicy_ deprecated?




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



[GitHub] [hbase] wchevreuil commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-648309919


   > IMO if we change split policy to consider region's total size rather than HStore size, we should also correct the config name. We can follow deprecation cycle for the config. But with current hregion.max.filesize its more confusing.
   > Now this config make sense because after a major compaction all the files under a Store will become single large file. So based on this max possible size we decide the split (So we sum all file's size under a store).
   
   I don't think changing the config name is strictly required. Official docs are clear about the expected behaviour for hbase.hregion.max.filesize, and this policy current violates that:
   
   `hbase.hregion.max.filesize
   Description
   Maximum HFile size. If the sum of the sizes of a region’s HFiles has grown to exceed this value, the region is split in two.`
   
   This policy also breaks MAX_FILESIZE table descriptor expected behaviour, as per ableDescriptorBuilder javadoc:
   
   `Returns the maximum size upto which a region can grow to after which a
      region split is triggered. The region size is represented by the size of
      the biggest store file in that region.
     `
   


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-645471513


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   6m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 46s |  master passed  |
   | +1 :green_heart: |  compile  |   1m  9s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   6m 17s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 43s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 31s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m  9s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 20s |  patch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   0m 40s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  | 189m 38s |  hbase-server in the patch passed.  |
   |  |   | 223m 33s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.11 Server=19.03.11 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux cf967b2b1e13 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / b17ba7b81d |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/testReport/ |
   | Max. process+thread count | 3083 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/2/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-650316101


   > @bsglz , sorry for the delay. So majority of people on the DISCUSS thread opined towards #3. I think @busbey suggestion to fix IncreasingToUpperBoundRegionSplitPolicy for minor updates only is reasonable, so maybe we can merge this current one to branch-2.2 and branch-2.3, then for master and branch-2 we fix IncreasingToUpperBoundRegionSplitPolicy.
   
   Ok, will do it in a follow-on issue.


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



[GitHub] [hbase] bsglz commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r440882731



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       > Yes, and maybe we should raise a discussion on dev list about making _ IncreasingToUpperBoundRegionSplitPolicy_ deprecated?
   Agree. Could you raise it? Thanks.




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



[GitHub] [hbase] wchevreuil commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-646565902


   I have raised a discussion here: https://lists.apache.org/thread.html/r08a8103e2532eb667a0fcb4efa8a4117b3f82e6251bc4bd0bc157c26%40%3Cdev.hbase.apache.org%3E
   
   Let's wait for folks comments before we decide next steps for this PR. 


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



[GitHub] [hbase] wchevreuil closed pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil closed pull request #1883:
URL: https://github.com/apache/hbase/pull/1883


   


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



[GitHub] [hbase] wchevreuil commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-650299753


   @bsglz , sorry for the delay. So majority of people on the DISCUSS thread opined towards #3. I think @busbey suggestion to fix IncreasingToUpperBoundRegionSplitPolicy for minor updates only is reasonable, so maybe we can merge this current one to branch-2.2 and branch-2.3, then for master and branch-2 we fix IncreasingToUpperBoundRegionSplitPolicy. 


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-647984025


   BTW, ConstantSizeRegionSplitPolicy has the same problem, FYI.


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-645208777


   @wchevreuil Could you help to merge it if you have time, thanks.


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



[GitHub] [hbase] anoopsjohn commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-648289120


   IMO if we change split policy to consider region's total size rather than HStore size, we should also correct the config name. We can follow deprecation cycle for the config. But with current hregion.max.filesize its more confusing.
   Now this config make sense because after a major compaction all the files under a Store will become single large file. So based on this max possible size we decide the split (So we sum all file's size under a store).  


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651484447


   rebase


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651804881


   That's true. 
   Could you help to merge this pr to all active branch first, then i will do other things  in follow-on issue for master and branch-2? 
   Thanks.


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651516357


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 28s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 44s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  6s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   2m  0s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 24s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  3s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m 15s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m  8s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 15s |  The patch does not generate ASF License warnings.  |
   |  |   |  32m 25s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux 6d44980546d2 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / bfec964aef |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/4/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651595114


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 29s |  Docker mode activated.  |
   ||| _ Prechecks _ |
   | +1 :green_heart: |  dupname  |   0m  0s |  No case conflicting files found.  |
   | +1 :green_heart: |  hbaseanti  |   0m  0s |  Patch does not have any anti-patterns.  |
   | +1 :green_heart: |  @author  |   0m  0s |  The patch does not contain any @author tags.  |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 27s |  master passed  |
   | +1 :green_heart: |  checkstyle  |   1m  3s |  master passed  |
   | +1 :green_heart: |  spotbugs  |   1m 57s |  master passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 21s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   1m  5s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  11m  3s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   2m  7s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 13s |  The patch does not generate ASF License warnings.  |
   |  |   |  32m  5s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle |
   | uname | Linux a3f37da9f58d 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / 982bd5fadd |
   | Max. process+thread count | 94 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/5/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) spotbugs=3.1.12 |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-650492489


   @wchevreuil 
   Thought it again, if this new splitPolicy is useable in branch-2.2 and branch-2.3, but disappear in branch-2.4 without deprecated cycle, is it break the compatibility guarantee? 
   
   IMO, maybe we can do as below:
   1: In all actived branchs, add this new splitPolicy.
   2: In minor branch set it as default policy and mark SteppingSpliyPolicy and IncreasingToUpperBoundRegionSplitPolicy as deprecated, at the same time fix the ConstantSizeRegionSplitPolicy.
   
   What do you think? Thanks.


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-648079234


   > @bsglz , sorry for the delay. Let's give one more day for the DISCUSS thread.
   
   Ok, thanks.


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



[GitHub] [hbase] bsglz commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-646577229


   Ok, thanks.


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



[GitHub] [hbase] bsglz commented on a change in pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#discussion_r439707985



##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/SteppingAllStoresSizeSplitPolicy.java
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.regionserver;
+
+import org.apache.hadoop.hbase.procedure2.util.StringUtils;
+import org.apache.yetus.audience.InterfaceAudience;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+@InterfaceAudience.Private
+public class SteppingAllStoresSizeSplitPolicy extends SteppingSplitPolicy {
+  private static final Logger LOG =
+    LoggerFactory.getLogger(SteppingAllStoresSizeSplitPolicy.class);
+
+  /**
+   * @return true if sum of store's size exceed the sizeToCheck
+   */
+  @Override
+  protected boolean isExceedSize(int tableRegionsCount, long sizeToCheck) {
+    long sumSize = 0;
+    for (HStore store : region.getStores()) {
+      sumSize += store.getSize();
+    }
+    if (sumSize > sizeToCheck) {
+      LOG.debug("ShouldSplit because region size is big enough " +
+        " size=" + StringUtils.humanSize(sumSize) +
+        ", sizeToCheck=" + StringUtils.humanSize(sizeToCheck) +
+        ", regionsWithCommonTable=" + tableRegionsCount);
+      return true;
+    }
+    return false;
+  }

Review comment:
       Thought it agian, split by store size cause the actual region split size uncontrollable and easy to misunderstand, as hbase do more and more better on many columnfamilies, the disadvantages will more and more obvious, so i think we should change the default behavior to split by all reigon size.  
   @wchevreuil @saintstack WDYT? 
   Thanks.




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



[GitHub] [hbase] Apache-HBase commented on pull request #1883: HBASE-24530 Introduce a split policy similar with SteppingSplitPolicy…

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #1883:
URL: https://github.com/apache/hbase/pull/1883#issuecomment-651501439


   :broken_heart: **-1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   3m 33s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  3s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ master Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   6m 20s |  master passed  |
   | +1 :green_heart: |  compile  |   1m 36s |  master passed  |
   | +1 :green_heart: |  shadedjars  |   8m 14s |  branch has no errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   1m  0s |  hbase-server in master failed.  |
   ||| _ Patch Compile Tests _ |
   | -1 :x: |  mvninstall  |   3m 44s |  root in the patch failed.  |
   | -1 :x: |  compile  |   1m 31s |  hbase-server in the patch failed.  |
   | -0 :warning: |  javac  |   1m 31s |  hbase-server in the patch failed.  |
   | -1 :x: |  shadedjars  |   6m 32s |  patch has 48 errors when building our shaded downstream artifacts.  |
   | -0 :warning: |  javadoc  |   1m  5s |  hbase-server in the patch failed.  |
   ||| _ Other Tests _ |
   | -1 :x: |  unit  |   1m 31s |  hbase-server in the patch failed.  |
   |  |   |  36m 17s |   |
   
   
   | Subsystem | Report/Notes |
   |----------:|:-------------|
   | Docker | Client=19.03.9 Server=19.03.9 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/1883 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 0c0b4bf6e225 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | master / bfec964aef |
   | Default Java | 2020-01-14 |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
   | mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
   | compile | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
   | javac | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-hbase-server.txt |
   | shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
   | javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
   | unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
   |  Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/testReport/ |
   | Max. process+thread count | 84 (vs. ulimit of 12500) |
   | modules | C: hbase-server U: hbase-server |
   | Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-1883/3/console |
   | versions | git=2.17.1 maven=(cecedd343002696d0abb50b32b541b8a6ba2883f) |
   | Powered by | Apache Yetus 0.11.1 https://yetus.apache.org |
   
   
   This message was automatically generated.
   
   


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