You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by GitBox <gi...@apache.org> on 2020/07/02 06:13:47 UTC
[GitHub] [hbase] bsglz opened a new pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
bsglz opened a new pull request #2011:
URL: https://github.com/apache/hbase/pull/2011
…er than only one 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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-654873701
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | 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 _ |
| +0 :ok: | mvndep | 0m 30s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 1s | master passed |
| +1 :green_heart: | compile | 2m 48s | master passed |
| +1 :green_heart: | shadedjars | 5m 44s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 18s | hbase-common in master failed. |
| -0 :warning: | javadoc | 0m 38s | hbase-server in master failed. |
| -0 :warning: | javadoc | 0m 15s | root in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| -1 :x: | mvninstall | 2m 20s | root in the patch failed. |
| -1 :x: | compile | 2m 1s | root in the patch failed. |
| -0 :warning: | javac | 2m 1s | root in the patch failed. |
| -1 :x: | shadedjars | 4m 37s | patch has 22 errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 17s | hbase-common in the patch failed. |
| -0 :warning: | javadoc | 0m 37s | hbase-server in the patch failed. |
| -0 :warning: | javadoc | 0m 15s | root in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 5m 16s | root in the patch failed. |
| | | 31m 49s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 44db201e7f58 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 / 34e807a8b5 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
| mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-root.txt |
| javac | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-compile-root.txt |
| shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-shadedjars.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/testReport/ |
| Max. process+thread count | 700 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] bsglz commented on a change in pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r453399710
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -38,10 +41,13 @@
*/
@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
public class ConstantSizeRegionSplitPolicy extends RegionSplitPolicy {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ConstantSizeRegionSplitPolicy.class);
private static final Random RANDOM = new Random();
private long desiredMaxFileSize;
private double jitterRate;
+ protected boolean overallHregionFiles;
Review comment:
Will fix.
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450752446
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
>Make the logic more clear.
If can not split, then we do not need to get tableRegionsCount and sizeToCheck in IncreasingToUpperBoundRegionSplitPolicy, and also avoid other comparsion in loop too.
That's a problem for IncreasingToUpperBoundRegionSplitPolicy. We shouldn't penalise a parent class because of subclasses implementation specifics. And you are already overriding this method on IncreasingToUpperBoundRegionSplitPolicy to do the extra loop anyways, so why keep this extra, needless loop here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bsglz commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-652945798
> TestFromClientSideWithCoprocessor5.testAppendWithoutWAL
This failure ut seems unrelated to this pr, it could pass in my local env.
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-652933322
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 45s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 6s | master passed |
| +1 :green_heart: | compile | 3m 18s | master passed |
| +1 :green_heart: | shadedjars | 6m 48s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 17s | root in master failed. |
| -0 :warning: | javadoc | 0m 18s | hbase-common in master failed. |
| -0 :warning: | javadoc | 0m 47s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 4s | the patch passed |
| +1 :green_heart: | compile | 3m 18s | the patch passed |
| +1 :green_heart: | javac | 3m 18s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 43s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 20s | hbase-common in the patch failed. |
| -0 :warning: | javadoc | 0m 51s | hbase-server in the patch failed. |
| -0 :warning: | javadoc | 0m 16s | root in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 233m 3s | root in the patch failed. |
| | | 271m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4855600bd10b 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / afe2eac7b6 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/testReport/ |
| Max. process+thread count | 3723 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-655260383
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 23s | 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 _ |
| +0 :ok: | mvndep | 0m 35s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 52s | master passed |
| +1 :green_heart: | checkstyle | 2m 16s | master passed |
| +0 :ok: | refguide | 5m 36s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 12m 38s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 23s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 20s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 5m 38s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 11s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 12m 56s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 34s | The patch does not generate ASF License warnings. |
| | | 72m 50s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle refguide xml |
| uname | Linux e89e3846d470 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 90f4ff7d7c |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-general-check/output/branch-site/book.html |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 122 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-654981402
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 41s | 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 _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 43s | master passed |
| +1 :green_heart: | checkstyle | 2m 44s | master passed |
| +0 :ok: | refguide | 6m 48s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 15m 26s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 39s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 47s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 7m 45s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 15m 10s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 15m 54s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 40s | The patch does not generate ASF License warnings. |
| | | 87m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle refguide xml |
| uname | Linux 94ec79e905a6 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 / 34e807a8b5 |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-general-check/output/branch-site/book.html |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 138 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] bsglz commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-657370727
@wchevreuil @Apache9
FYI, this PR should be merged to master only, I will push a new PR for branch-2(change DEFAULT_OVERALL_HREGION_FILES to false )
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450587742
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -94,4 +94,33 @@ long getDesiredMaxFileSize() {
public boolean positiveJitterRate() {
return this.jitterRate > 0;
}
+
+ /**
+ * @return true if region size exceed the sizeToCheck
+ */
+ protected boolean isExceedSize(long sizeToCheck, String extraLogStr) {
+ if (overallHregionFiles) {
+ long sumSize = 0;
+ for (HStore store : region.getStores()) {
+ sumSize += store.getSize();
+ }
+ if (sumSize > sizeToCheck) {
Review comment:
The extraLogStr exactly seems a bit strange, but there are several variables used by logging which calculated inside isExceedSize, if we want remove extraLogStr, I think there are some other ways.
1. Use a map as param instead of extraLogStr.
2. Change the return value from boolean to other obj such as map.
3. Make the logging more simpler.
I prefer the current way, WDYT?
Thanks. @wchevreuil
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-653046563
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 52s | 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 _ |
| +0 :ok: | mvndep | 0m 31s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 46s | master passed |
| +1 :green_heart: | checkstyle | 3m 16s | master passed |
| +0 :ok: | refguide | 7m 7s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 17m 53s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 51s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 35s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 5m 44s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 14s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 12m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
| | | 84m 19s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle refguide xml |
| uname | Linux 3c2b4c47bc0f 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / afe2eac7b6 |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-general-check/output/branch-site/book.html |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 122 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] Apache9 commented on a change in pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r453313685
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
##########
@@ -382,6 +382,13 @@
/** Default maximum file size */
public static final long DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024 * 1024L;
+ /** Conf key for if we should sum overall region files size when check to split */
+ public static final String OVERALL_HREGION_FILES =
Review comment:
Should we use 'hregion' or 'region'? I'm not sure, just ask.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -38,10 +41,13 @@
*/
@InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
public class ConstantSizeRegionSplitPolicy extends RegionSplitPolicy {
+ private static final Logger LOG =
+ LoggerFactory.getLogger(ConstantSizeRegionSplitPolicy.class);
private static final Random RANDOM = new Random();
private long desiredMaxFileSize;
private double jitterRate;
+ protected boolean overallHregionFiles;
Review comment:
overallHRegionFiles? The 'R' should be upper case.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionSplitPolicy.java
##########
@@ -75,7 +75,7 @@ protected void configureForRegion(HRegion region) {
*/
protected boolean canSplit() {
return !region.getRegionInfo().isMetaRegion() && region.isAvailable() &&
- !region.hasReferences();
+ region.getStores().stream().allMatch(HStore::canSplit);
Review comment:
This is an interesting change. Is behavior still the same?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -94,4 +90,33 @@ long getDesiredMaxFileSize() {
public boolean positiveJitterRate() {
return this.jitterRate > 0;
}
+
+ /**
+ * @return true if region size exceed the sizeToCheck
+ */
+ protected boolean isExceedSize(long sizeToCheck) {
Review comment:
Should we make this method final? I think it is used to be called by sub classes, not to be overridden?
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-654960264
: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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 44s | master passed |
| +1 :green_heart: | compile | 2m 25s | master passed |
| +1 :green_heart: | shadedjars | 5m 35s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 5s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 24s | the patch passed |
| +1 :green_heart: | compile | 2m 24s | the patch passed |
| +1 :green_heart: | javac | 2m 24s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 28s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 11s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 19m 57s | root in the patch failed. |
| | | 51m 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-2011/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3c3e465c705a 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 / 34e807a8b5 |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/testReport/ |
| Max. process+thread count | 1210 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] wchevreuil commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-656771603
restest build
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-653294523
Please take a look if you have time, thanks. @wchevreuil @Apache9
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-653019774
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 45s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 13s | master passed |
| +1 :green_heart: | compile | 2m 46s | master passed |
| +1 :green_heart: | shadedjars | 6m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 21s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 14s | the patch passed |
| +1 :green_heart: | compile | 2m 44s | the patch passed |
| +1 :green_heart: | javac | 2m 44s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 3s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 22s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 421m 12s | root in the patch failed. |
| | | 458m 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-2011/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux cb5e59a53ac2 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / afe2eac7b6 |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/testReport/ |
| Max. process+thread count | 4058 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-653025161
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 51s | 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 _ |
| +0 :ok: | mvndep | 0m 27s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 42s | master passed |
| +1 :green_heart: | compile | 3m 28s | master passed |
| +1 :green_heart: | shadedjars | 7m 39s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 5m 31s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 10s | the patch passed |
| +1 :green_heart: | compile | 3m 18s | the patch passed |
| +1 :green_heart: | javac | 3m 18s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 21s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 19s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 1m 12s | root in the patch failed. |
| | | 47m 11s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3262e17d5441 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / afe2eac7b6 |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/testReport/ |
| Max. process+thread count | 222 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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 a change in pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450842948
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
Although IncreasingToUpperBoundRegionSplitPolicy override the shouldSplit, but does not override isExceedSize.
IMO, even if there is only ConstantSizeRegionSplitPolicy, the ref check should be precondition of size check, since if any store has ref file, all stores are no need to check 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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-653114242
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 59s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 7s | master passed |
| +1 :green_heart: | compile | 3m 27s | master passed |
| +1 :green_heart: | shadedjars | 7m 11s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 20s | root in master failed. |
| -0 :warning: | javadoc | 0m 19s | hbase-common in master failed. |
| -0 :warning: | javadoc | 0m 47s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 9s | the patch passed |
| +1 :green_heart: | compile | 3m 33s | the patch passed |
| +1 :green_heart: | javac | 3m 33s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 57s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 19s | hbase-common in the patch failed. |
| -0 :warning: | javadoc | 0m 47s | hbase-server in the patch failed. |
| -0 :warning: | javadoc | 0m 15s | root in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 169m 16s | root in the patch passed. |
| | | 208m 31s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 7e9ef5f0498a 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 / afe2eac7b6 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/2/testReport/ |
| Max. process+thread count | 6580 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-652831765
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 30s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 37s | master passed |
| +1 :green_heart: | checkstyle | 1m 57s | master passed |
| +0 :ok: | refguide | 4m 51s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 11m 16s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 21s | the patch passed |
| -0 :warning: | checkstyle | 2m 1s | root: The patch generated 1 new + 5 unchanged - 0 fixed = 6 total (was 5) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 5m 0s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 11m 10s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 11m 39s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 42s | The patch does not generate ASF License warnings. |
| | | 64m 33s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle refguide xml |
| uname | Linux 3a94f1e13649 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 / afe2eac7b6 |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-general-check/output/branch-site/book.html |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-general-check/output/diff-checkstyle-root.txt |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/1/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 137 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-657371625
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +0 :ok: | mvndep | 0m 35s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | compile | 2m 45s | master passed |
| +1 :green_heart: | shadedjars | 5m 44s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 18s | hbase-common in master failed. |
| -0 :warning: | javadoc | 0m 38s | hbase-server in master failed. |
| -0 :warning: | javadoc | 0m 15s | root in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 56s | the patch passed |
| +1 :green_heart: | compile | 2m 44s | the patch passed |
| +1 :green_heart: | javac | 2m 44s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 39s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 17s | hbase-common in the patch failed. |
| -0 :warning: | javadoc | 0m 38s | hbase-server in the patch failed. |
| -0 :warning: | javadoc | 0m 15s | root in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 163m 52s | root in the patch passed. |
| | | 195m 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-2011/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux a4bcb8c1deac 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 / 724f0478ed |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/testReport/ |
| Max. process+thread count | 6784 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450139888
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
Move this check to the for loops inside _isExceedSize_, so that we don't have to do do an extra iteration for all stores again in case none returns false for _canSplit_.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -94,4 +94,33 @@ long getDesiredMaxFileSize() {
public boolean positiveJitterRate() {
return this.jitterRate > 0;
}
+
+ /**
+ * @return true if region size exceed the sizeToCheck
+ */
+ protected boolean isExceedSize(long sizeToCheck, String extraLogStr) {
+ if (overallHregionFiles) {
+ long sumSize = 0;
+ for (HStore store : region.getStores()) {
+ sumSize += store.getSize();
+ }
+ if (sumSize > sizeToCheck) {
Review comment:
We should just return this comparison and let each caller decide how to log it? That would discard the need for having an extra param just for the sake of logging.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bsglz commented on a change in pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450842948
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
Although IncreasingToUpperBoundRegionSplitPolicy override the shouldSplit, but does not override isExceedSize.
IMO, even if there is only ConstantSizeRegionSplitPolicy, the ref check should be precondition of size check, since if any store has ref file, all stores are no need to check size, especially the loop is cheap here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-657342284
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 21s | 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 _ |
| +0 :ok: | mvndep | 0m 29s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | checkstyle | 2m 17s | master passed |
| +0 :ok: | refguide | 5m 31s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 12m 49s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 43s | the patch passed |
| +1 :green_heart: | checkstyle | 2m 13s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 2s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 5m 36s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | hadoopcheck | 12m 16s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1. |
| +1 :green_heart: | spotbugs | 12m 55s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 36s | The patch does not generate ASF License warnings. |
| | | 72m 12s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle refguide xml |
| uname | Linux 57efe7bf4337 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 724f0478ed |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-general-check/output/branch-site/book.html |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-general-check/output/patch-site/book.html |
| Max. process+thread count | 122 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/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 a change in pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450842948
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
Although IncreasingToUpperBoundRegionSplitPolicy override the shouldSplit, but does not override isExceedSize.
IMO, even if there is only ConstantSizeRegionSplitPolicy, the ref check should be precondition of size check, since if any store has ref file, all stores are no need to check.
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r453401671
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RegionSplitPolicy.java
##########
@@ -75,7 +75,7 @@ protected void configureForRegion(HRegion region) {
*/
protected boolean canSplit() {
return !region.getRegionInfo().isMetaRegion() && region.isAvailable() &&
- !region.hasReferences();
+ region.getStores().stream().allMatch(HStore::canSplit);
Review comment:
Yeah, I think they are equivalent.
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-655374098
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 45s | 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 _ |
| +0 :ok: | mvndep | 0m 25s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 12s | master passed |
| +1 :green_heart: | compile | 2m 46s | master passed |
| +1 :green_heart: | shadedjars | 7m 17s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 31s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 28s | the patch passed |
| +1 :green_heart: | compile | 3m 1s | the patch passed |
| +1 :green_heart: | javac | 3m 1s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 10s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 45s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 335m 56s | root in the patch failed. |
| | | 376m 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-2011/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 09db1bb67116 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 / 90f4ff7d7c |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/testReport/ |
| Max. process+thread count | 5223 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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 a change in pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450846252
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -94,4 +94,33 @@ long getDesiredMaxFileSize() {
public boolean positiveJitterRate() {
return this.jitterRate > 0;
}
+
+ /**
+ * @return true if region size exceed the sizeToCheck
+ */
+ protected boolean isExceedSize(long sizeToCheck, String extraLogStr) {
+ if (overallHregionFiles) {
+ long sumSize = 0;
+ for (HStore store : region.getStores()) {
+ sumSize += store.getSize();
+ }
+ if (sumSize > sizeToCheck) {
Review comment:
Yeah, i am agree with you, will fix later.
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-654880233
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 43s | 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 _ |
| +0 :ok: | mvndep | 0m 32s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 11s | master passed |
| +1 :green_heart: | compile | 2m 38s | master passed |
| +1 :green_heart: | shadedjars | 6m 5s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 43s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 18s | Maven dependency ordering for patch |
| -1 :x: | mvninstall | 2m 18s | root in the patch failed. |
| -1 :x: | compile | 2m 3s | root in the patch failed. |
| -0 :warning: | javac | 2m 3s | root in the patch failed. |
| -1 :x: | shadedjars | 5m 11s | patch has 22 errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 3m 26s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 7m 5s | root in the patch failed. |
| | | 40m 40s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9bbc2f061e21 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 34e807a8b5 |
| Default Java | 1.8.0_232 |
| mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk8-hadoop3-check/output/patch-mvninstall-root.txt |
| compile | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-root.txt |
| javac | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk8-hadoop3-check/output/patch-compile-root.txt |
| shadedjars | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk8-hadoop3-check/output/patch-shadedjars.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/testReport/ |
| Max. process+thread count | 706 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] bsglz commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-652831980
This pr is for master, when it pass review, i can push another one for branch-2(set the option false as default) if necessary.
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-654955872
: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 _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 16s | master passed |
| +1 :green_heart: | compile | 2m 48s | master passed |
| +1 :green_heart: | shadedjars | 5m 51s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 19s | root in master failed. |
| -0 :warning: | javadoc | 0m 16s | hbase-common in master failed. |
| -0 :warning: | javadoc | 0m 41s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 12s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 4m 7s | the patch passed |
| +1 :green_heart: | compile | 2m 51s | the patch passed |
| +1 :green_heart: | javac | 2m 51s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 52s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 17s | hbase-common in the patch failed. |
| -0 :warning: | javadoc | 0m 47s | hbase-server in the patch failed. |
| -0 :warning: | javadoc | 0m 14s | root in the patch failed. |
||| _ Other Tests _ |
| -1 :x: | unit | 13m 41s | root in the patch failed. |
| | | 44m 39s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9a89cb4e96a6 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 / 34e807a8b5 |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/4/testReport/ |
| Max. process+thread count | 1127 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-654043581
@wchevreuil @Apache9 Ping.
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450583983
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
I moved the canSplit checking to the front purposely, it has benefits below.
1. Make the logic more clear.
2. If can not split, then we do not need to get tableRegionsCount and sizeToCheck in IncreasingToUpperBoundRegionSplitPolicy, and also avoid other comparsion in loop too.
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-655385037
Seems the ut failure unrelated to 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] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-657401366
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | 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 _ |
| +0 :ok: | mvndep | 0m 23s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 3m 40s | master passed |
| +1 :green_heart: | compile | 2m 18s | master passed |
| +1 :green_heart: | shadedjars | 5m 31s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 2m 56s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 13s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 3m 24s | the patch passed |
| +1 :green_heart: | compile | 2m 19s | the patch passed |
| +1 :green_heart: | javac | 2m 19s | the patch passed |
| +1 :green_heart: | shadedjars | 5m 32s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 2m 56s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 317m 8s | root in the patch failed. |
| | | 349m 23s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1ace3b54d2ef 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 / 724f0478ed |
| Default Java | 1.8.0_232 |
| unit | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/testReport/ |
| Max. process+thread count | 4767 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/6/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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r452592457
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
@wchevreuil WDYT?
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r453399865
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -94,4 +90,33 @@ long getDesiredMaxFileSize() {
public boolean positiveJitterRate() {
return this.jitterRate > 0;
}
+
+ /**
+ * @return true if region size exceed the sizeToCheck
+ */
+ protected boolean isExceedSize(long sizeToCheck) {
Review comment:
Will fix.
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
bsglz commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r453399499
##########
File path: hbase-common/src/main/java/org/apache/hadoop/hbase/HConstants.java
##########
@@ -382,6 +382,13 @@
/** Default maximum file size */
public static final long DEFAULT_MAX_FILE_SIZE = 10 * 1024 * 1024 * 1024L;
+ /** Conf key for if we should sum overall region files size when check to split */
+ public static final String OVERALL_HREGION_FILES =
Review comment:
I used region first, and changed to hregion when i saw the surround usage such as "HREGION_COMPACTIONDIR_NAME".
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450757746
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -94,4 +94,33 @@ long getDesiredMaxFileSize() {
public boolean positiveJitterRate() {
return this.jitterRate > 0;
}
+
+ /**
+ * @return true if region size exceed the sizeToCheck
+ */
+ protected boolean isExceedSize(long sizeToCheck, String extraLogStr) {
+ if (overallHregionFiles) {
+ long sumSize = 0;
+ for (HStore store : region.getStores()) {
+ sumSize += store.getSize();
+ }
+ if (sumSize > sizeToCheck) {
Review comment:
We are breaking isolation and cohesion principles, this method should log only the info it has locally, without requiring any additional param just for logging. The additional info available in eventual callers should then be logged by those callers, not passed through as params, here I suggest the log to be only:
`LOG.debug("ShouldSplit because region size is big enough "
+ "size={}, sizeToCheck={}", StringUtils.humanSize(sumSize),
StringUtils.humanSize(sizeToCheck))`
Then on IncreasingToUpperBoundRegionSplitPolicy you log the extra info on an additional debug message:
`LOG.debug(regionsWithCommonTable={}", tableRegionsCount)`
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
>Make the logic more clear.
If can not split, then we do not need to get tableRegionsCount and sizeToCheck in IncreasingToUpperBoundRegionSplitPolicy, and also avoid other comparsion in loop too.
That's a problem for IncreasingToUpperBoundRegionSplitPolicy. We shouldn't penalise a parent class because of subclasses implementation specifics. And you are already overriding this method on IncreasingToUpperBoundRegionSplitPolicy to do the extra loop anyways, so why keep this extra, needless loop here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-654889648
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 43s | 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 _ |
| +0 :ok: | mvndep | 0m 31s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 4m 14s | master passed |
| +1 :green_heart: | checkstyle | 2m 25s | master passed |
| +0 :ok: | refguide | 5m 39s | branch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| +1 :green_heart: | spotbugs | 14m 8s | master passed |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| -1 :x: | mvninstall | 2m 32s | root in the patch failed. |
| +1 :green_heart: | checkstyle | 2m 34s | the patch passed |
| +1 :green_heart: | whitespace | 0m 1s | The patch has no whitespace issues. |
| +1 :green_heart: | xml | 0m 1s | The patch has no ill-formed XML file. |
| +0 :ok: | refguide | 6m 0s | patch has no errors when building the reference guide. See footer for rendered docs, which you should manually inspect. |
| -1 :x: | hadoopcheck | 2m 16s | The patch causes 22 errors with Hadoop v3.1.2. |
| -1 :x: | hadoopcheck | 4m 33s | The patch causes 22 errors with Hadoop v3.2.1. |
| -1 :x: | spotbugs | 0m 31s | hbase-server in the patch failed. |
| -1 :x: | spotbugs | 5m 28s | root in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 31s | The patch does not generate ASF License warnings. |
| | | 53m 47s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | dupname asflicense spotbugs hadoopcheck hbaseanti checkstyle refguide xml |
| uname | Linux 0ce65ec5fc9f 4.15.0-74-generic #84-Ubuntu SMP Thu Dec 19 08:06:28 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 34e807a8b5 |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/branch-site/book.html |
| mvninstall | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/patch-mvninstall-root.txt |
| refguide | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/patch-site/book.html |
| hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/patch-javac-3.1.2.txt |
| hadoopcheck | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/patch-javac-3.2.1.txt |
| spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/patch-spotbugs-hbase-server.txt |
| spotbugs | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/3/artifact/yetus-general-check/output/patch-spotbugs-root.txt |
| Max. process+thread count | 123 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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] wchevreuil merged pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
wchevreuil merged pull request #2011:
URL: https://github.com/apache/hbase/pull/2011
----------------------------------------------------------------
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 #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#discussion_r450752446
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ConstantSizeRegionSplitPolicy.java
##########
@@ -68,22 +76,14 @@ protected void configureForRegion(HRegion region) {
@Override
protected boolean shouldSplit() {
- boolean foundABigStore = false;
-
+ // If any of the stores is unable to split (eg they contain reference files)
+ // then don't split
for (HStore store : region.getStores()) {
- // If any of the stores are unable to split (eg they contain reference files)
- // then don't split
- if ((!store.canSplit())) {
+ if (!store.canSplit()) {
Review comment:
>Make the logic more clear.
If can not split, then we do not need to get tableRegionsCount and sizeToCheck in IncreasingToUpperBoundRegionSplitPolicy, and also avoid other comparsion in loop too.
That's a problem for IncreasingToUpperBoundRegionSplitPolicy. We shouldn't penalise a parent class because of subclasses implementation specifics. And you are already overriding this method on IncreasingToUpperBoundRegionSplitPolicy to do the extra loop anyways, so why keep this extra, needless loop here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2011: HBASE-24664 Some changing of split region by overall region size rath…
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2011:
URL: https://github.com/apache/hbase/pull/2011#issuecomment-655332596
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 28s | 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 _ |
| +0 :ok: | mvndep | 0m 24s | Maven dependency ordering for branch |
| +1 :green_heart: | mvninstall | 5m 29s | master passed |
| +1 :green_heart: | compile | 3m 28s | master passed |
| +1 :green_heart: | shadedjars | 7m 25s | branch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 17s | root in master failed. |
| -0 :warning: | javadoc | 0m 19s | hbase-common in master failed. |
| -0 :warning: | javadoc | 0m 49s | hbase-server in master failed. |
||| _ Patch Compile Tests _ |
| +0 :ok: | mvndep | 0m 14s | Maven dependency ordering for patch |
| +1 :green_heart: | mvninstall | 5m 23s | the patch passed |
| +1 :green_heart: | compile | 3m 29s | the patch passed |
| +1 :green_heart: | javac | 3m 29s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 1s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 19s | hbase-common in the patch failed. |
| -0 :warning: | javadoc | 0m 49s | hbase-server in the patch failed. |
| -0 :warning: | javadoc | 0m 16s | root in the patch failed. |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 254m 15s | root in the patch passed. |
| | | 294m 2s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.12 Server=19.03.12 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2011 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4c99c52528eb 4.15.0-91-generic #92-Ubuntu SMP Fri Feb 28 11:09:48 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 90f4ff7d7c |
| Default Java | 2020-01-14 |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-root.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk11-hadoop3-check/output/branch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-common.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-hbase-server.txt |
| javadoc | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/artifact/yetus-jdk11-hadoop3-check/output/patch-javadoc-root.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/5/testReport/ |
| Max. process+thread count | 4415 (vs. ulimit of 12500) |
| modules | C: hbase-common hbase-server . U: . |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-2011/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