Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
Apache9 merged PR #7048: URL: https://github.com/apache/hbase/pull/7048 -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
Apache-HBase commented on PR #7048:
URL: https://github.com/apache/hbase/pull/7048#issuecomment-2933479140
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 50s | | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | | Unprocessed flag(s):
--brief-report-file --spotbugs-strict-precheck --author-ignore-list
--blanks-eol-ignore-file --blanks-tabs-ignore-file --quick-hadoopcheck |
_ Prechecks _ |
_ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 12s | | master passed |
| +1 :green_heart: | compile | 1m 19s | | master passed |
| +1 :green_heart: | javadoc | 0m 44s | | master passed |
| +1 :green_heart: | shadedjars | 6m 59s | | branch has no errors when
building our shaded downstream artifacts. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 48s | | the patch passed |
| +1 :green_heart: | compile | 1m 17s | | the patch passed |
| +1 :green_heart: | javac | 1m 17s | | the patch passed |
| +1 :green_heart: | javadoc | 0m 40s | | the patch passed |
| +1 :green_heart: | shadedjars | 6m 53s | | patch has no errors when
building our shaded downstream artifacts. |
_ Other Tests _ |
| +1 :green_heart: | unit | 303m 56s | | hbase-server in the patch
passed. |
| | | 336m 15s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base:
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7048/2/artifact/yetus-jdk17-hadoop3-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/hbase/pull/7048 |
| JIRA Issue | HBASE-29363 |
| Optional Tests | javac javadoc unit compile shadedjars |
| uname | Linux 0873e821399b 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May
23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e284ab95c94350b9e2efa5c321d961c524434651 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Test Results |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7048/2/testReport/
|
| Max. process+thread count | 4423 (vs. ulimit of 3) |
| modules | C: hbase-server U: hbase-server |
| Console output |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7048/2/console
|
| versions | git=2.34.1 maven=3.9.8 |
| Powered by | Apache Yetus 0.15.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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
Apache-HBase commented on PR #7048:
URL: https://github.com/apache/hbase/pull/7048#issuecomment-2932977810
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 59s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any
anti-patterns. |
_ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 18s | | master passed |
| +1 :green_heart: | compile | 3m 57s | | master passed |
| +1 :green_heart: | checkstyle | 0m 41s | | master passed |
| +1 :green_heart: | spotbugs | 1m 58s | | master passed |
| +1 :green_heart: | spotless | 1m 7s | | branch has no errors when
running spotless:check. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 55s | | the patch passed |
| +1 :green_heart: | compile | 4m 0s | | the patch passed |
| +1 :green_heart: | javac | 4m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 0m 47s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 6s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 13m 52s | | Patch does not cause any
errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 1m 0s | | patch has no errors when
running spotless:check. |
_ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 14s | | The patch does not
generate ASF License warnings. |
| | | 47m 32s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base:
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7048/2/artifact/yetus-general-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/hbase/pull/7048 |
| JIRA Issue | HBASE-29363 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell
detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux d6cf2281e710 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May
23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / e284ab95c94350b9e2efa5c321d961c524434651 |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 87 (vs. ulimit of 3) |
| modules | C: hbase-server U: hbase-server |
| Console output |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7048/2/console
|
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
charlesconnell commented on code in PR #7048:
URL: https://github.com/apache/hbase/pull/7048#discussion_r2121258013
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java:
##
@@ -90,6 +91,8 @@ public class CompactSplit implements CompactionRequester,
PropagatingConfigurati
private volatile ThreadPoolExecutor longCompactions;
private volatile ThreadPoolExecutor shortCompactions;
private volatile ThreadPoolExecutor splits;
+ // Used in unit testing
+ private int splitCounter;
Review Comment:
Yes, done
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java:
##
@@ -503,12 +509,13 @@ public int getSplitQueueSize() {
return splits.getQueue().size();
}
- private boolean shouldSplitRegion() {
+ private boolean shouldSplitRegion(RegionInfo ri) {
if (server.getNumberOfOnlineRegions() > 0.9 * regionSplitLimit) {
LOG.warn("Total number of regions is approaching the upper limit " +
regionSplitLimit + ". "
+ "Please consider taking a look at
http://hbase.apache.org/book.html#ops.regionmgt";);
}
-return (regionSplitLimit > server.getNumberOfOnlineRegions());
+return (regionSplitLimit > server.getNumberOfOnlineRegions()
+ && ri.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID);
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
Apache9 commented on code in PR #7048: URL: https://github.com/apache/hbase/pull/7048#discussion_r2119205197 ## hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java: ## @@ -90,6 +91,8 @@ public class CompactSplit implements CompactionRequester, PropagatingConfigurati private volatile ThreadPoolExecutor longCompactions; private volatile ThreadPoolExecutor shortCompactions; private volatile ThreadPoolExecutor splits; + // Used in unit testing + private int splitCounter; Review Comment: Is it possible to not introduce field in normal code which is only used for testing? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
Copilot commented on code in PR #7048:
URL: https://github.com/apache/hbase/pull/7048#discussion_r2119201485
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java:
##
@@ -503,12 +509,13 @@ public int getSplitQueueSize() {
return splits.getQueue().size();
}
- private boolean shouldSplitRegion() {
+ private boolean shouldSplitRegion(RegionInfo ri) {
if (server.getNumberOfOnlineRegions() > 0.9 * regionSplitLimit) {
LOG.warn("Total number of regions is approaching the upper limit " +
regionSplitLimit + ". "
+ "Please consider taking a look at
http://hbase.apache.org/book.html#ops.regionmgt";);
}
-return (regionSplitLimit > server.getNumberOfOnlineRegions());
+return (regionSplitLimit > server.getNumberOfOnlineRegions()
+ && ri.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID);
Review Comment:
[nitpick] Since this condition ensures that only primary replicas are
allowed to be split, consider adding a brief code comment or updating the
documentation to clarify this behavior for future maintainers.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
Apache-HBase commented on PR #7048:
URL: https://github.com/apache/hbase/pull/7048#issuecomment-2921228659
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Logfile | Comment |
|::|--:|:|::|:---:|
| +0 :ok: | reexec | 0m 40s | | Docker mode activated. |
_ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | | No case conflicting files
found. |
| +0 :ok: | codespell | 0m 0s | | codespell was not available. |
| +0 :ok: | detsecrets | 0m 0s | | detect-secrets was not available.
|
| +1 :green_heart: | @author | 0m 0s | | The patch does not contain
any @author tags. |
| +1 :green_heart: | hbaseanti | 0m 0s | | Patch does not have any
anti-patterns. |
_ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 0s | | master passed |
| +1 :green_heart: | compile | 4m 9s | | master passed |
| +1 :green_heart: | checkstyle | 0m 53s | | master passed |
| +1 :green_heart: | spotbugs | 2m 9s | | master passed |
| +1 :green_heart: | spotless | 0m 56s | | branch has no errors when
running spotless:check. |
_ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 57s | | the patch passed |
| +1 :green_heart: | compile | 4m 0s | | the patch passed |
| +1 :green_heart: | javac | 4m 0s | | the patch passed |
| +1 :green_heart: | blanks | 0m 0s | | The patch has no blanks
issues. |
| +1 :green_heart: | checkstyle | 0m 47s | | the patch passed |
| +1 :green_heart: | spotbugs | 2m 10s | | the patch passed |
| +1 :green_heart: | hadoopcheck | 13m 6s | | Patch does not cause any
errors with Hadoop 3.3.6 3.4.0. |
| +1 :green_heart: | spotless | 0m 48s | | patch has no errors when
running spotless:check. |
_ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 11s | | The patch does not
generate ASF License warnings. |
| | | 46m 7s | | |
| Subsystem | Report/Notes |
|--:|:-|
| Docker | ClientAPI=1.43 ServerAPI=1.43 base:
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7048/1/artifact/yetus-general-check/output/Dockerfile
|
| GITHUB PR | https://github.com/apache/hbase/pull/7048 |
| JIRA Issue | HBASE-29363 |
| Optional Tests | dupname asflicense javac spotbugs checkstyle codespell
detsecrets compile hadoopcheck hbaseanti spotless |
| uname | Linux f2e836b51668 5.4.0-1103-aws #111~18.04.1-Ubuntu SMP Tue May
23 20:04:10 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 11de4898f1682edde3258038f0fe6e2edad63c1b |
| Default Java | Eclipse Adoptium-17.0.11+9 |
| Max. process+thread count | 83 (vs. ulimit of 3) |
| modules | C: hbase-server U: hbase-server |
| Console output |
https://ci-hbase.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-7048/1/console
|
| versions | git=2.34.1 maven=3.9.8 spotbugs=4.7.3 |
| Powered by | Apache Yetus 0.15.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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Re: [PR] HBASE-29363: CompactSplit should not attempt to split secondary region replicas [hbase]
charlesconnell commented on code in PR #7048:
URL: https://github.com/apache/hbase/pull/7048#discussion_r2114737442
##
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/CompactSplit.java:
##
@@ -503,12 +509,13 @@ public int getSplitQueueSize() {
return splits.getQueue().size();
}
- private boolean shouldSplitRegion() {
+ private boolean shouldSplitRegion(RegionInfo ri) {
if (server.getNumberOfOnlineRegions() > 0.9 * regionSplitLimit) {
LOG.warn("Total number of regions is approaching the upper limit " +
regionSplitLimit + ". "
+ "Please consider taking a look at
http://hbase.apache.org/book.html#ops.regionmgt";);
}
-return (regionSplitLimit > server.getNumberOfOnlineRegions());
+return (regionSplitLimit > server.getNumberOfOnlineRegions()
+ && ri.getReplicaId() == RegionInfo.DEFAULT_REPLICA_ID);
Review Comment:
This is the key change, every other part of this PR is just unit testing
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
