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 2022/11/18 18:31:17 UTC
[GitHub] [hbase] charlesconnell opened a new pull request, #4888: Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
charlesconnell opened a new pull request, #4888:
URL: https://github.com/apache/hbase/pull/4888
This commit adds two new settings:
- hbase.normalizer.merge.plans_size_limit.mb
- hbase.normalizer.split.plans_size_limit.mb
These settings limit the number of plans produced by a single run of SimpleRegionNormalizer, by forcing it to stop producing new plans once the cumulative region size limits are exceeded.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1322456575
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 58s | 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 | 2m 35s | master passed |
| +1 :green_heart: | compile | 2m 19s | master passed |
| +1 :green_heart: | checkstyle | 0m 35s | master passed |
| +1 :green_heart: | spotless | 0m 40s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 23s | the patch passed |
| +1 :green_heart: | compile | 2m 20s | the patch passed |
| +1 :green_heart: | javac | 2m 20s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 35s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 8m 41s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 41s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 9s | The patch does not generate ASF License warnings. |
| | | 30m 49s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux b1a30a09028f 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 78 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/3/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028043557
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -229,6 +231,8 @@ public List<NormalizationPlan> computePlansForTable(final TableDescriptor tableD
plans.addAll(mergePlans);
}
+ plans = truncateForSize(plans);
Review Comment:
Yea, to me the separation of concerns makes sense -- RegionNormalizer generates plans, RegionNormalizerWorker executes those plans based on limits (rate limit and max work limit). The default is unlimited, so even if someone is using their own RegionNormalizer and didn't want limits, they wouldn't get them. But we'd be making the configuration available to all users, which seems like a win with no real downside (given default).
That said, if we decided to keep the shuffling i think that makes sense for SimpleRegionNormalizer to do -- return a shuffled list, rather than have RegionNormalizerWorker shuffle. The shuffling seems ok for the simple normalizer but would probably be unexpected for other implementations.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault merged pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault merged PR #4888:
URL: https://github.com/apache/hbase/pull/4888
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028382029
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -79,6 +79,8 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
static final int DEFAULT_MERGE_MIN_REGION_AGE_DAYS = 3;
static final String MERGE_MIN_REGION_SIZE_MB_KEY = "hbase.normalizer.merge.min_region_size.mb";
static final int DEFAULT_MERGE_MIN_REGION_SIZE_MB = 0;
+ static final String CUMULATIVE_SIZE_LIMIT_MB_KEY = "hbase.normalizer.plans_size_limit.mb";
Review Comment:
i think we want to remove these from here, since they now are defined in RegionNormalizerWorker.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java:
##########
@@ -207,14 +214,34 @@ private List<NormalizationPlan> calculatePlans(final TableName tableName) {
return Collections.emptyList();
}
- final List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+ List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+
+ plans = truncateForSize(plans);
+
if (CollectionUtils.isEmpty(plans)) {
LOG.debug("No normalization required for table {}.", tableName);
return Collections.emptyList();
}
return plans;
}
+ private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> plans) {
Review Comment:
Thanks for moving. One more request here -- please add a LOG.debug when limit is reached. We should at least include the original number of plans, number of plans that will be submitted, and cumulativeSizeMb (maybe the plan that tipped it over the limit?). This will help operators diagnose issues where the normalizer seems to be doing less work than they'd expect. It might be nice to include the total size, which I'm realizing might help aid in tuning but of course that means removing the loop break I asked you to make.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java:
##########
@@ -73,6 +78,8 @@ class RegionNormalizerWorker implements PropagatingConfigurationObserver, Runnab
this.mergePlanCount = 0;
this.rateLimiter = loadRateLimiter(configuration);
this.defaultNormalizerTableLevel = extractDefaultNormalizerValue(configuration);
+ this.cumulativePlansSizeLimitMb =
+ configuration.getLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_SIZE_LIMIT_MB);
Review Comment:
make sure to give this a similar treatment to the rateLimiter -- it needs to get parsed in onConfigurationChange below so that the limit can be updated dynamically. We should add a log there as well (formatted similar to the rate limit one) to inform the user that the limit has been updated.
when making that change, i know you dont see it in the other normalizer-related fields but typically i notice that most things updated by onConfigurationChange end up being volatile. Might be good to make cumulativePlansSizeLimitMb volatile along those lines.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -574,6 +598,10 @@ public long getMergeMinRegionSizeMb(NormalizeContext context) {
}
return mergeMinRegionSizeMb;
}
+
+ public long getCumulativePlansSizeLimitMb() {
Review Comment:
can be private or removed i think?
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -502,6 +522,8 @@ private NormalizerConfiguration(final Configuration conf,
mergeMinRegionCount = parseMergeMinRegionCount(conf);
mergeMinRegionAge = parseMergeMinRegionAge(conf);
mergeMinRegionSizeMb = parseMergeMinRegionSizeMb(conf);
+ cumulativePlansSizeLimitMb =
Review Comment:
i realize you need to keep this here for the shuffling stuff. that's fine, once the imports are updated to point at the constants in RegionNormalizerWorker.
but i dont think we should call `logConfigurationUpdated` here anymore, since the logger class will be SimpleRegionNormalizer even though the config is mostly for the worker. in another comment i ask you to add a log in RegionNormalizerWorker which I think can suffice for this config.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1324006070
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 22s | 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 | 2m 27s | master passed |
| +1 :green_heart: | compile | 0m 46s | master passed |
| +1 :green_heart: | shadedjars | 3m 45s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 32s | the patch passed |
| +1 :green_heart: | compile | 0m 46s | the patch passed |
| +1 :green_heart: | javac | 0m 46s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 46s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 203m 0s | hbase-server in the patch passed. |
| | | 222m 39s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux c8217addc362 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/testReport/ |
| Max. process+thread count | 2713 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1027120506
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -464,6 +468,27 @@ private boolean isLargeEnoughForMerge(final NormalizerConfiguration normalizerCo
return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
+ private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> plans) {
+ if (
+ normalizerConfiguration.getCumulativePlansSizeLimitMb() != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB
+ ) {
+ // If we are going to truncate our list of plans, shuffle the split and merge plans together
+ // so that the merge plans, which are listed last, are not starved out.
+ List<NormalizationPlan> maybeTruncatedPlans = new ArrayList<>();
+ Collections.shuffle(plans);
+ long cumulativeSizeMb = 0;
+ for (NormalizationPlan plan : plans) {
+ cumulativeSizeMb += plan.getPlanSizeMb();
+ if (cumulativeSizeMb < normalizerConfiguration.getCumulativePlansSizeLimitMb()) {
+ maybeTruncatedPlans.add(plan);
Review Comment:
Could you move this down a line and instead have this if block return early? Will just cut a little cpu when there are very large plans
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java:
##########
@@ -607,6 +609,39 @@ public void testNormalizerCannotMergeNonAdjacentRegions() {
assertThat(plans, empty());
}
+ @Test
+ public void testMergeSizeLimit() {
Review Comment:
Can you add a test for the shuffle stuff? Just proving that the limit doesn't starve merges like we hope
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028426624
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java:
##########
@@ -73,6 +78,8 @@ class RegionNormalizerWorker implements PropagatingConfigurationObserver, Runnab
this.mergePlanCount = 0;
this.rateLimiter = loadRateLimiter(configuration);
this.defaultNormalizerTableLevel = extractDefaultNormalizerValue(configuration);
+ this.cumulativePlansSizeLimitMb =
+ configuration.getLong(CUMULATIVE_SIZE_LIMIT_MB_KEY, DEFAULT_CUMULATIVE_SIZE_LIMIT_MB);
Review Comment:
Added logging on change. Made cumulativePlansSizeLimitMb an AtomicLong because assignments to volatile longs aren't atomic.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1323218315
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | 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 | 2m 37s | master passed |
| +1 :green_heart: | compile | 0m 40s | master passed |
| +1 :green_heart: | shadedjars | 4m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 17s | the patch passed |
| +1 :green_heart: | compile | 0m 36s | the patch passed |
| +1 :green_heart: | javac | 0m 36s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 7s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 204m 14s | hbase-server in the patch failed. |
| | | 223m 56s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 4b165037fc53 5.4.0-1085-aws #92~18.04.1-Ubuntu SMP Wed Aug 31 17:21:08 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/testReport/ |
| Max. process+thread count | 2644 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028047124
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java:
##########
@@ -607,6 +609,39 @@ public void testNormalizerCannotMergeNonAdjacentRegions() {
assertThat(plans, empty());
}
+ @Test
+ public void testMergeSizeLimit() {
Review Comment:
We decided that it was important that the limit be applied approximately fairly to splits and merges. Given that decision, we decided to design our implementation for that goal in mind. Since HBase is an open source project, someone could easily accidentally break this decision down the line. The way to prevent that is be creating tests based on the goals of the code change. Since this was a specific goal, we should probably guard it with a test. The comment helps but isn't really a contract and is often overlooked.
There's a lot of mocking going on in here. It seems like we could think of a way to make a non-flaky test despite the randomness. We aren't testing for a perfect shuffle, we really just want to ensure that at least 1 merge gets into the plan to prove that we aren't starving that.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1320593362
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 22s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | 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 | 2m 44s | master passed |
| +1 :green_heart: | compile | 0m 46s | master passed |
| +1 :green_heart: | shadedjars | 3m 48s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 32s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 47s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 26s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 205m 8s | hbase-server in the patch passed. |
| | | 225m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux b29d112b436b 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7b0ac6451d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/testReport/ |
| Max. process+thread count | 2484 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1322664895
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | 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 | 2m 32s | master passed |
| +1 :green_heart: | compile | 0m 38s | master passed |
| +1 :green_heart: | shadedjars | 4m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 14s | the patch passed |
| +1 :green_heart: | compile | 0m 37s | the patch passed |
| +1 :green_heart: | javac | 0m 37s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 8s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 209m 11s | hbase-server in the patch passed. |
| | | 229m 21s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 947b2e31e7c7 5.4.0-1085-aws #92~18.04.1-Ubuntu SMP Wed Aug 31 17:21:08 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/3/testReport/ |
| Max. process+thread count | 2657 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/3/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1320928522
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 58s | 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 | 2m 28s | master passed |
| +1 :green_heart: | compile | 2m 23s | master passed |
| +1 :green_heart: | checkstyle | 0m 35s | master passed |
| +1 :green_heart: | spotless | 0m 41s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 33s | the patch passed |
| +1 :green_heart: | compile | 2m 22s | the patch passed |
| +1 :green_heart: | javac | 2m 22s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 35s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 8m 40s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 40s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 37s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 10s | The patch does not generate ASF License warnings. |
| | | 30m 20s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 8b570d2741c3 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7b0ac6451d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 77 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1320964329
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 3m 37s | Docker mode activated. |
| -0 :warning: | yetus | 0m 2s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 34s | master passed |
| +1 :green_heart: | compile | 0m 40s | master passed |
| +1 :green_heart: | shadedjars | 4m 15s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 22s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 4s | the patch passed |
| +1 :green_heart: | compile | 0m 35s | the patch passed |
| +1 :green_heart: | javac | 0m 35s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 59s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 20s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 202m 51s | hbase-server in the patch failed. |
| | | 225m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 935df3269310 5.4.0-1085-aws #92~18.04.1-Ubuntu SMP Wed Aug 31 17:21:08 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7b0ac6451d |
| Default Java | Temurin-1.8.0_352-b08 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/artifact/yetus-jdk8-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/testReport/ |
| Max. process+thread count | 2641 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028425833
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java:
##########
@@ -207,14 +214,34 @@ private List<NormalizationPlan> calculatePlans(final TableName tableName) {
return Collections.emptyList();
}
- final List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+ List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+
+ plans = truncateForSize(plans);
+
if (CollectionUtils.isEmpty(plans)) {
LOG.debug("No normalization required for table {}.", tableName);
return Collections.emptyList();
}
return plans;
}
+ private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> plans) {
Review Comment:
Done
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java:
##########
@@ -207,14 +214,34 @@ private List<NormalizationPlan> calculatePlans(final TableName tableName) {
return Collections.emptyList();
}
- final List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+ List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+
+ plans = truncateForSize(plans);
+
if (CollectionUtils.isEmpty(plans)) {
LOG.debug("No normalization required for table {}.", tableName);
return Collections.emptyList();
}
return plans;
}
+ private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> plans) {
Review Comment:
Done, and the break still works
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028540010
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java:
##########
@@ -207,14 +214,34 @@ private List<NormalizationPlan> calculatePlans(final TableName tableName) {
return Collections.emptyList();
}
- final List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+ List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+
+ plans = truncateForSize(plans);
+
if (CollectionUtils.isEmpty(plans)) {
LOG.debug("No normalization required for table {}.", tableName);
return Collections.emptyList();
}
return plans;
}
+ private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> plans) {
Review Comment:
Fixed
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028426768
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -574,6 +598,10 @@ public long getMergeMinRegionSizeMb(NormalizeContext context) {
}
return mergeMinRegionSizeMb;
}
+
+ public long getCumulativePlansSizeLimitMb() {
Review Comment:
Can be private
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1323016380
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 58s | 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 | 2m 51s | master passed |
| +1 :green_heart: | compile | 2m 26s | master passed |
| +1 :green_heart: | checkstyle | 0m 34s | master passed |
| +1 :green_heart: | spotless | 0m 44s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 28s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 29s | the patch passed |
| +1 :green_heart: | compile | 2m 19s | the patch passed |
| +1 :green_heart: | javac | 2m 19s | the patch passed |
| -0 :warning: | checkstyle | 0m 34s | hbase-server: The patch generated 1 new + 0 unchanged - 0 fixed = 1 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 8m 50s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 39s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 31m 17s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux c60ddf1a1047 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028426122
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -79,6 +79,8 @@ class SimpleRegionNormalizer implements RegionNormalizer, ConfigurationObserver
static final int DEFAULT_MERGE_MIN_REGION_AGE_DAYS = 3;
static final String MERGE_MIN_REGION_SIZE_MB_KEY = "hbase.normalizer.merge.min_region_size.mb";
static final int DEFAULT_MERGE_MIN_REGION_SIZE_MB = 0;
+ static final String CUMULATIVE_SIZE_LIMIT_MB_KEY = "hbase.normalizer.plans_size_limit.mb";
Review Comment:
Yep
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -502,6 +522,8 @@ private NormalizerConfiguration(final Configuration conf,
mergeMinRegionCount = parseMergeMinRegionCount(conf);
mergeMinRegionAge = parseMergeMinRegionAge(conf);
mergeMinRegionSizeMb = parseMergeMinRegionSizeMb(conf);
+ cumulativePlansSizeLimitMb =
Review Comment:
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1027132193
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -229,6 +231,8 @@ public List<NormalizationPlan> computePlansForTable(final TableDescriptor tableD
plans.addAll(mergePlans);
}
+ plans = truncateForSize(plans);
Review Comment:
The difference there is that the new setting would apply to HBase clusters using alternate `RegionNormalizer` implementations. I really don't know if that's a pro or a con.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1027132066
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java:
##########
@@ -607,6 +609,39 @@ public void testNormalizerCannotMergeNonAdjacentRegions() {
assertThat(plans, empty());
}
+ @Test
+ public void testMergeSizeLimit() {
Review Comment:
I can write a test but it will be flaky and fail sometimes through bad luck. `Collections.shuffle()` is not guaranteed to actually change item ordering.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1324008486
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 58s | 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 | 2m 56s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 4m 40s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 32s | the patch passed |
| +1 :green_heart: | compile | 0m 42s | the patch passed |
| +1 :green_heart: | javac | 0m 42s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 42s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 27s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 201m 52s | hbase-server in the patch passed. |
| | | 224m 22s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux adae94cce740 5.4.0-1085-aws #92~18.04.1-Ubuntu SMP Wed Aug 31 17:21:08 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/testReport/ |
| Max. process+thread count | 2662 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028344198
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/normalizer/TestSimpleRegionNormalizer.java:
##########
@@ -607,6 +609,39 @@ public void testNormalizerCannotMergeNonAdjacentRegions() {
assertThat(plans, empty());
}
+ @Test
+ public void testMergeSizeLimit() {
Review Comment:
Added a test
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -464,6 +468,27 @@ private boolean isLargeEnoughForMerge(final NormalizerConfiguration normalizerCo
return getRegionSizeMB(regionInfo) >= normalizerConfiguration.getMergeMinRegionSizeMb(ctx);
}
+ private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> plans) {
+ if (
+ normalizerConfiguration.getCumulativePlansSizeLimitMb() != DEFAULT_CUMULATIVE_SIZE_LIMIT_MB
+ ) {
+ // If we are going to truncate our list of plans, shuffle the split and merge plans together
+ // so that the merge plans, which are listed last, are not starved out.
+ List<NormalizationPlan> maybeTruncatedPlans = new ArrayList<>();
+ Collections.shuffle(plans);
+ long cumulativeSizeMb = 0;
+ for (NormalizationPlan plan : plans) {
+ cumulativeSizeMb += plan.getPlanSizeMb();
+ if (cumulativeSizeMb < normalizerConfiguration.getCumulativePlansSizeLimitMb()) {
+ maybeTruncatedPlans.add(plan);
Review Comment:
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1027121723
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -229,6 +231,8 @@ public List<NormalizationPlan> computePlansForTable(final TableDescriptor tableD
plans.addAll(mergePlans);
}
+ plans = truncateForSize(plans);
Review Comment:
My only other thought is whether it makes sense to do the truncating in the NormalizationWorker, like how it currently handles rate limiting
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1320593259
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 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 _ |
| +1 :green_heart: | mvninstall | 2m 41s | master passed |
| +1 :green_heart: | compile | 0m 39s | master passed |
| +1 :green_heart: | shadedjars | 4m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 14s | the patch passed |
| +1 :green_heart: | compile | 0m 37s | the patch passed |
| +1 :green_heart: | javac | 0m 37s | the patch passed |
| +1 :green_heart: | shadedjars | 4m 10s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 205m 7s | hbase-server in the patch passed. |
| | | 225m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2daf68e93a66 5.4.0-1085-aws #92~18.04.1-Ubuntu SMP Wed Aug 31 17:21:08 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7b0ac6451d |
| Default Java | Temurin-1.8.0_352-b08 |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/testReport/ |
| Max. process+thread count | 2638 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1320423397
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 50s | 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 | 2m 55s | master passed |
| +1 :green_heart: | compile | 2m 22s | master passed |
| +1 :green_heart: | checkstyle | 0m 37s | master passed |
| +1 :green_heart: | spotless | 0m 43s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 28s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 26s | the patch passed |
| +1 :green_heart: | compile | 2m 21s | the patch passed |
| +1 :green_heart: | javac | 2m 21s | the patch passed |
| -0 :warning: | checkstyle | 0m 36s | hbase-server: The patch generated 8 new + 0 unchanged - 0 fixed = 8 total (was 0) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 8m 48s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| -1 :x: | spotless | 0m 35s | patch has 64 errors when running spotless:check, run spotless:apply to fix. |
| +1 :green_heart: | spotbugs | 1m 35s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 33m 10s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux 4538a4d549b1 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7b0ac6451d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| checkstyle | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| spotless | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/artifact/yetus-general-check/output/patch-spotless.txt |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/1/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1320963975
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 22s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | 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 | 2m 33s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 3m 49s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 31s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 45s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 204m 0s | hbase-server in the patch failed. |
| | | 223m 30s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 6fd6d64af15a 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7b0ac6451d |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/testReport/ |
| Max. process+thread count | 2553 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/2/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1323218670
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 22s | Docker mode activated. |
| -0 :warning: | yetus | 0m 5s | 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 | 2m 31s | master passed |
| +1 :green_heart: | compile | 0m 47s | master passed |
| +1 :green_heart: | shadedjars | 3m 45s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 35s | the patch passed |
| +1 :green_heart: | compile | 0m 47s | the patch passed |
| +1 :green_heart: | javac | 0m 47s | the patch passed |
| +1 :green_heart: | shadedjars | 3m 45s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 24s | the patch passed |
||| _ Other Tests _ |
| -1 :x: | unit | 204m 43s | hbase-server in the patch failed. |
| | | 224m 26s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0c90c07c69b1 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| unit | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/artifact/yetus-jdk11-hadoop3-check/output/patch-unit-hbase-server.txt |
| Test Results | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/testReport/ |
| Max. process+thread count | 2556 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/4/console |
| versions | git=2.34.1 maven=3.8.6 |
| 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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028517667
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/RegionNormalizerWorker.java:
##########
@@ -207,14 +214,34 @@ private List<NormalizationPlan> calculatePlans(final TableName tableName) {
return Collections.emptyList();
}
- final List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+ List<NormalizationPlan> plans = regionNormalizer.computePlansForTable(tblDesc);
+
+ plans = truncateForSize(plans);
+
if (CollectionUtils.isEmpty(plans)) {
LOG.debug("No normalization required for table {}.", tableName);
return Collections.emptyList();
}
return plans;
}
+ private List<NormalizationPlan> truncateForSize(List<NormalizationPlan> plans) {
Review Comment:
Sorry I think you misunderstood my request. I wouldn't normally push further on a logging change, but I think getting the right data will be helpful here.
We're adding a configurable limit on total bytes processed. If someone is ever going to want to tune this, it'll be helpful to know how many total bytes are necessary to normalize. Even logging this over time will be useful to have a sense for throughput over time. So in addition to the list sizes, I think we should include the size we truncated at and the total size calculated by `calculatePlans`. That's why I was saying we might need to remove the `break`, since it'll require iterating all plans. I think it's still worth it since this isn't a super cpu intensive process.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] charlesconnell commented on a diff in pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
charlesconnell commented on code in PR #4888:
URL: https://github.com/apache/hbase/pull/4888#discussion_r1028344454
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/normalizer/SimpleRegionNormalizer.java:
##########
@@ -229,6 +231,8 @@ public List<NormalizationPlan> computePlansForTable(final TableDescriptor tableD
plans.addAll(mergePlans);
}
+ plans = truncateForSize(plans);
Review Comment:
Sure, I've moved the truncation into RegionNormalizerWorker
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1323731572
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 57s | 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 | 2m 48s | master passed |
| +1 :green_heart: | compile | 2m 24s | master passed |
| +1 :green_heart: | checkstyle | 0m 34s | master passed |
| +1 :green_heart: | spotless | 0m 43s | branch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 29s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 2m 29s | the patch passed |
| +1 :green_heart: | compile | 2m 19s | the patch passed |
| +1 :green_heart: | javac | 2m 19s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 35s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 8m 44s | Patch does not cause any errors with Hadoop 3.2.4 3.3.4. |
| +1 :green_heart: | spotless | 0m 41s | patch has no errors when running spotless:check. |
| +1 :green_heart: | spotbugs | 1m 33s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 31m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/4888 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti spotless checkstyle compile |
| uname | Linux ab5a72aeaafc 5.4.0-124-generic #140-Ubuntu SMP Thu Aug 4 02:23:37 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 9462933179 |
| Default Java | Eclipse Adoptium-11.0.17+8 |
| Max. process+thread count | 79 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-4888/5/console |
| versions | git=2.34.1 maven=3.8.6 spotbugs=4.7.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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bbeaudreault commented on pull request #4888: HBASE-27496 Optionally limit the cumulative size of normalization plans produced by SimpleRegionNormalizer
Posted by GitBox <gi...@apache.org>.
bbeaudreault commented on PR #4888:
URL: https://github.com/apache/hbase/pull/4888#issuecomment-1323564422
The test failures look unrelated but unfortunately we do have to fix the checkstyle issue. Running spotless:apply may fix it.
--
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.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org