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 2021/02/01 08:03:38 UTC
[GitHub] [hbase] ZhaoBQ opened a new pull request #2917: HBASE-25534 Improve the configuration of Normalizer
ZhaoBQ opened a new pull request #2917:
URL: https://github.com/apache/hbase/pull/2917
Table can be enabled and disabled merge/split in TableDescriptor, we should judge before calculating the plan.
Part of the configuration can be set by table level. For example, hbase.normalizer.min.region.count can be set by "alter ‘table’, CONFIGURATION=>{'hbase.normalizer.min.region.count' => '5'}". If the table is not set, then use the global configuration which is set in hbase-site.xml.
----------------------------------------------------------------
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] ZhaoBQ commented on a change in pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#discussion_r570676737
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
final NormalizerConfiguration normalizerConfiguration,
+ final NormalizeContext ctx,
final RegionInfo regionInfo
) {
- return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb();
+ return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
- private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
+ private boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
Review comment:
Thanks for the wonderful answers, which benefited me a lot.
----------------------------------------------------------------
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] ZhaoBQ commented on pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773126001
Thanks @ndimiduk. Not sure about the static keyword. The rest is done.
----------------------------------------------------------------
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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773154573
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 12s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 0s | master passed |
| +1 :green_heart: | compile | 3m 28s | master passed |
| +1 :green_heart: | checkstyle | 1m 15s | master passed |
| +1 :green_heart: | spotbugs | 2m 17s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 56s | the patch passed |
| +1 :green_heart: | compile | 3m 18s | the patch passed |
| +1 :green_heart: | javac | 3m 18s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 11s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 23s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 17s | The patch does not generate ASF License warnings. |
| | | 51m 44s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 81806a227f62 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d6d67d1e9f |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-770764134
:confetti_ball: **+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 _ |
| +1 :green_heart: | mvninstall | 4m 40s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed |
| +1 :green_heart: | shadedjars | 7m 53s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 46s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 34s | the patch passed |
| +1 :green_heart: | compile | 1m 11s | the patch passed |
| +1 :green_heart: | javac | 1m 11s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 44s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 45s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 134m 55s | hbase-server in the patch passed. |
| | | 166m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f594626d82cb 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a04ea7ea44 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/testReport/ |
| Max. process+thread count | 3720 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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] ndimiduk commented on a change in pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#discussion_r570437845
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
final NormalizerConfiguration normalizerConfiguration,
+ final NormalizeContext ctx,
final RegionInfo regionInfo
) {
- return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb();
+ return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
- private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
+ private boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
Review comment:
As a general practice, when a method can be static, I prefer to make it so. If a method has no need for `this` or private state, why expose it? I find static methods are easier to reason about because you know there's no hidden effects of object member variables manipulated elsewhere. I suspect they're simpler for the JIT to optimize for the same reasons, but I don't have evidence of this.
----------------------------------------------------------------
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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773755748
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | master passed |
| +1 :green_heart: | compile | 3m 12s | master passed |
| +1 :green_heart: | checkstyle | 1m 4s | master passed |
| +1 :green_heart: | spotbugs | 2m 6s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 37s | the patch passed |
| +1 :green_heart: | compile | 3m 17s | the patch passed |
| +1 :green_heart: | javac | 3m 17s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 2s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 21m 10s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 43s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 52m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 2a0351aa7b67 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 / 3a690931f5 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773277575
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 11m 56s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 7m 16s | master passed |
| +1 :green_heart: | compile | 1m 35s | master passed |
| +1 :green_heart: | shadedjars | 9m 16s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 1m 5s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 51s | the patch passed |
| +1 :green_heart: | compile | 1m 36s | the patch passed |
| +1 :green_heart: | javac | 1m 36s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 40s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 51s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 210m 27s | hbase-server in the patch passed. |
| | | 260m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 5b7637f388f5 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d6d67d1e9f |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/testReport/ |
| Max. process+thread count | 2775 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773790895
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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 _ |
| +1 :green_heart: | mvninstall | 4m 6s | master passed |
| +1 :green_heart: | compile | 1m 7s | master passed |
| +1 :green_heart: | shadedjars | 6m 45s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 1s | the patch passed |
| +1 :green_heart: | compile | 1m 5s | the patch passed |
| +1 :green_heart: | javac | 1m 5s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 40s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 136m 7s | hbase-server in the patch passed. |
| | | 163m 43s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4ab43ebbd319 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3a690931f5 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/testReport/ |
| Max. process+thread count | 4910 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773154573
----------------------------------------------------------------
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] ndimiduk commented on a change in pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#discussion_r569816933
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -254,41 +255,38 @@ private boolean isMasterSwitchEnabled(final MasterSwitchType masterSwitchType) {
return masterServices.isSplitOrMergeEnabled(masterSwitchType);
}
- private boolean proceedWithSplitPlanning() {
- return isSplitEnabled() && isMasterSwitchEnabled(MasterSwitchType.SPLIT);
+ private boolean proceedWithSplitPlanning(TableDescriptor tableDescriptor) {
+ String value = tableDescriptor.getValue(SPLIT_ENABLED_KEY);
+ return (value == null ? isSplitEnabled() : Boolean.parseBoolean(value)) &&
+ isMasterSwitchEnabled(MasterSwitchType.SPLIT);
}
- private boolean proceedWithMergePlanning() {
- return isMergeEnabled() && isMasterSwitchEnabled(MasterSwitchType.MERGE);
+ private boolean proceedWithMergePlanning(TableDescriptor tableDescriptor) {
+ String value = tableDescriptor.getValue(MERGE_ENABLED_KEY);
+ return (value == null ? isMergeEnabled() : Boolean.parseBoolean(value)) &&
+ isMasterSwitchEnabled(MasterSwitchType.MERGE);
}
/**
* @param tableRegions regions of table to normalize
+ * @param tableDescriptor the TableDescriptor
* @return average region size depending on
* @see org.apache.hadoop.hbase.client.TableDescriptor#getNormalizerTargetRegionCount()
* Also make sure tableRegions contains regions of the same table
*/
- private double getAverageRegionSizeMb(final List<RegionInfo> tableRegions) {
+ private double getAverageRegionSizeMb(final List<RegionInfo> tableRegions,
+ final TableDescriptor tableDescriptor) {
if (isEmpty(tableRegions)) {
throw new IllegalStateException(
"Cannot calculate average size of a table without any regions.");
}
- TableName table = tableRegions.get(0).getTable();
- int targetRegionCount = -1;
- long targetRegionSize = -1;
+ TableName table = tableDescriptor.getTableName();
double avgRegionSize;
- try {
- TableDescriptor tableDescriptor = masterServices.getTableDescriptors().get(table);
- if (tableDescriptor != null) {
- targetRegionCount = tableDescriptor.getNormalizerTargetRegionCount();
- targetRegionSize = tableDescriptor.getNormalizerTargetRegionSize();
- LOG.debug("Table {} configured with target region count {}, target region size {}", table,
- targetRegionCount, targetRegionSize);
- }
- } catch (IOException e) {
- LOG.warn("TableDescriptor for {} unavailable, table-level target region count and size"
- + " configurations cannot be considered.", table, e);
- }
+ long targetRegionCount = tableDescriptor.getNormalizerTargetRegionCount();
Review comment:
nit: no need to promote this to a `long`.
##########
File path: hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java
##########
@@ -292,12 +293,28 @@ public void testHonorsSplitEnabled() {
createRegionSizesMap(regionInfos, 5, 5, 20, 5, 5);
setupMocksForNormalizer(regionSizes, regionInfos);
assertThat(
- normalizer.computePlansForTable(tableName),
+ normalizer.computePlansForTable(tableDescriptor),
contains(instanceOf(SplitNormalizationPlan.class)));
conf.setBoolean(SPLIT_ENABLED_KEY, false);
setupMocksForNormalizer(regionSizes, regionInfos);
- assertThat(normalizer.computePlansForTable(tableName), empty());
+ assertThat(normalizer.computePlansForTable(tableDescriptor), empty());
+ }
+
+ @Test
+ public void testHonorsSplitEnabledInTD() {
Review comment:
These tests `*EnabledInTD` are good for confirming that the split/merge will not happen when disabled in table descriptor. Please also add coverage of when split/merge is disabled in configuration but enabled in table descriptor. I believe this should be supported, given the new implementations of `proceedWith{Split,Merge}Planning`.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -316,10 +314,10 @@ private double getAverageRegionSizeMb(final List<RegionInfo> tableRegions) {
*/
private boolean skipForMerge(
final NormalizerConfiguration normalizerConfiguration,
- final RegionStates regionStates,
+ final NormalizeContext ctx,
Review comment:
good.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -592,5 +621,14 @@ public RegionStates getRegionStates() {
public double getAverageRegionSizeMb() {
return averageRegionSizeMb;
}
+
+ public <T> T getOrDefault(String key, Function<String, T> function, T defaultValue) {
Review comment:
nice.
Maybe this method returns an `Object`/`null` and the caller handles the null-check. I think this approach would be better than using an out-of-range value to indicate the lack of configuration.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
final NormalizerConfiguration normalizerConfiguration,
+ final NormalizeContext ctx,
final RegionInfo regionInfo
) {
- return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb();
+ return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
- private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
+ private boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
Review comment:
why make this an instance method?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
Review comment:
can be static now as well?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-770694061
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 5m 1s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 54s | master passed |
| +1 :green_heart: | compile | 3m 19s | master passed |
| +1 :green_heart: | checkstyle | 1m 4s | master passed |
| +1 :green_heart: | spotbugs | 2m 4s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 38s | the patch passed |
| +1 :green_heart: | compile | 3m 10s | the patch passed |
| +1 :green_heart: | javac | 3m 10s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 3s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 21m 46s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 51s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 57m 34s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 36dc8557f59a 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 / a04ea7ea44 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 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] ZhaoBQ commented on a change in pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#discussion_r570023884
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
final NormalizerConfiguration normalizerConfiguration,
+ final NormalizeContext ctx,
final RegionInfo regionInfo
) {
- return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb();
+ return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
- private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
+ private boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
Review comment:
I find skipForSplit, logTraceReason, isOldEnoughForMerge is static, but it does not need static. I am not sure what is the purpose of setting those to static.
----------------------------------------------------------------
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] ndimiduk merged pull request #2917: HBASE-25534 Honor TableDescriptor settings earlier in normalization
Posted by GitBox <gi...@apache.org>.
ndimiduk merged pull request #2917:
URL: https://github.com/apache/hbase/pull/2917
----------------------------------------------------------------
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] ZhaoBQ commented on a change in pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on a change in pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#discussion_r570023884
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
final NormalizerConfiguration normalizerConfiguration,
+ final NormalizeContext ctx,
final RegionInfo regionInfo
) {
- return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb();
+ return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
- private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
+ private boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
Review comment:
I find skipForSplit, logTraceReason, isOldEnoughForMerge is static, but it does not need static. I am not sure what is the purpose of setting those to static.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
final NormalizerConfiguration normalizerConfiguration,
+ final NormalizeContext ctx,
final RegionInfo regionInfo
) {
- return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb();
+ return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
- private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
+ private boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
Review comment:
Thanks for the wonderful answers, which benefited me a lot.
----------------------------------------------------------------
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] ZhaoBQ commented on pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ZhaoBQ commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773126001
Thanks @ndimiduk. Not sure about the static keyword. The rest is done.
----------------------------------------------------------------
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] ndimiduk commented on a change in pull request #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
ndimiduk commented on a change in pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#discussion_r570437845
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java
##########
@@ -468,12 +467,13 @@ private static boolean isOldEnoughForMerge(
*/
private boolean isLargeEnoughForMerge(
final NormalizerConfiguration normalizerConfiguration,
+ final NormalizeContext ctx,
final RegionInfo regionInfo
) {
- return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb();
+ return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
- private static boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
+ private boolean logTraceReason(final BooleanSupplier predicate, final String fmtWhenTrue,
Review comment:
As a general practice, when a method can be static, I prefer to make it so. If a method has no need for `this` or private state, why expose it? I find static methods are easier to reason about because you know there's no hidden effects of object member variables manipulated elsewhere. I suspect they're simpler for the JIT to optimize for the same reasons, but I don't have evidence of this.
----------------------------------------------------------------
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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773267322
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 20s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 57s | master passed |
| +1 :green_heart: | compile | 1m 0s | master passed |
| +1 :green_heart: | shadedjars | 7m 18s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 51s | the patch passed |
| +1 :green_heart: | compile | 1m 0s | the patch passed |
| +1 :green_heart: | javac | 1m 0s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 210m 52s | hbase-server in the patch passed. |
| | | 240m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2faccb24f117 4.15.0-101-generic #102-Ubuntu SMP Mon May 11 10:07:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d6d67d1e9f |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/testReport/ |
| Max. process+thread count | 2781 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-773792500
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 32s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 55s | master passed |
| +1 :green_heart: | compile | 1m 1s | master passed |
| +1 :green_heart: | shadedjars | 6m 44s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 31s | the patch passed |
| +1 :green_heart: | compile | 0m 56s | the patch passed |
| +1 :green_heart: | javac | 0m 56s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 36s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 141m 47s | hbase-server in the patch passed. |
| | | 168m 30s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9d2597a6cd0c 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 / 3a690931f5 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/testReport/ |
| Max. process+thread count | 4088 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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 #2917: HBASE-25534 Improve the configuration of Normalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2917:
URL: https://github.com/apache/hbase/pull/2917#issuecomment-770766821
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 5m 4s | 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 _ |
| +1 :green_heart: | mvninstall | 3m 55s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 6m 39s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 35s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 56s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 139m 24s | hbase-server in the patch passed. |
| | | 170m 52s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2917 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8a4a42a9c8ab 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 / a04ea7ea44 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/testReport/ |
| Max. process+thread count | 3725 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2917/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 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