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/05/02 21:27:21 UTC
[GitHub] [hbase] apurtell opened a new pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
apurtell opened a new pull request #3219:
URL: https://github.com/apache/hbase/pull/3219
RegionStates#getAssignmentsForBalancer is used by the HMaster to collect all regions of interest to the balancer for the next chore iteration. We check if a table is in disabled state to exclude regions that will not be of interest (because disabled regions are or will be offline) or are in a state where they shouldn't be mutated (like SPLITTING).
The current checks are not actually comprehensive. For example, splitting states are considered, but not merging.
Filter out regions not in OPEN or OPENING state when building the set of interesting regions for the balancer to consider. Only regions open (or opening) on the cluster are of interest to balancing calculations for the current iteration. Regions in all other states can be expected to not be of interest – either offline (OFFLINE, or FAILED_*), not subject to balancer decisions now (SPLITTING, SPLITTING_NEW, MERGING, MERGING_NEW), or will be offline shortly (CLOSING) – until at least the next chore iteration.
--
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] bharathv commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r624900154
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +570,25 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
- if (isTableDisabled(tableStateManager, node.getTable())) {
Review comment:
nit: DisableTableProcedure first sets the table state to DISABLED and then force unassigns the regions in a loop, so we may want to retain this check to avoid weird races when "disable table" and the "chore" is running to totally avoid the table when it is marked DISABLED but regions are still a WIP.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +570,25 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
- if (isTableDisabled(tableStateManager, node.getTable())) {
- continue;
- }
- if (node.getRegionInfo().isSplitParent()) {
+ // When balancing, we are only interested in OPEN or OPENING regions and expected
+ // to be online at that server until possibly the next balancer iteration or unless
+ // we decide to move it. Other states are not interesting as the region will either
+ // be closing, or splitting/merging, or will not be deployed.
+ if (!(node.isInState(State.OPEN)||node.isInState(State.OPENING))) {
Review comment:
nit: spaces missing on either side of ||
--
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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-830915350
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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 59s | master passed |
| +1 :green_heart: | compile | 3m 14s | master passed |
| +1 :green_heart: | checkstyle | 1m 4s | master passed |
| +1 :green_heart: | spotbugs | 2m 5s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 36s | the patch passed |
| +1 :green_heart: | compile | 3m 8s | the patch passed |
| +1 :green_heart: | javac | 3m 8s | 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 | 18m 13s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 15s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 17s | The patch does not generate ASF License warnings. |
| | | 47m 2s | |
| 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-3219/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux c5d905e439b7 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 00fec24c90 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| 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-3219/1/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831518568
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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 10s | master passed |
| +1 :green_heart: | compile | 1m 11s | master passed |
| +1 :green_heart: | shadedjars | 8m 13s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 24s | the patch passed |
| +1 :green_heart: | compile | 1m 13s | the patch passed |
| +1 :green_heart: | javac | 1m 13s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 37s | 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 | 146m 29s | hbase-server in the patch passed. |
| | | 178m 11s | |
| 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-3219/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 36de140616e5 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 / 7640134e3e |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3219/2/testReport/ |
| Max. process+thread count | 4004 (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-3219/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] apurtell commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r625257713
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +571,37 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
+ // DisableTableProcedure first sets the table state to DISABLED and then force unassigns
+ // the regions in a loop. The balancer should ignore all regions for tables in DISABLED
+ // state because even if still currently open we expect them to be offlined very soon.
if (isTableDisabled(tableStateManager, node.getTable())) {
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Ignoring " + node + " because table is disabled");
Review comment:
Is one better than the other? I'd say no because it's easy to edit the string and not change the arguments, when you have these printf style log lines. I've done that, I've seen others do it. For your future consideration.
--
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] apurtell commented on pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
apurtell commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831418343
Updated after feedback
--
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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831601253
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 26s | 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 37s | master passed |
| +1 :green_heart: | compile | 1m 0s | master passed |
| +1 :green_heart: | shadedjars | 8m 9s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 37s | the patch passed |
| +1 :green_heart: | compile | 1m 4s | the patch passed |
| +1 :green_heart: | javac | 1m 4s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 12s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 148m 44s | hbase-server in the patch passed. |
| | | 178m 20s | |
| 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-3219/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 17de50684198 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 / 7640134e3e |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3219/3/testReport/ |
| Max. process+thread count | 4595 (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-3219/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] apurtell commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r625240325
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +570,25 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
- if (isTableDisabled(tableStateManager, node.getTable())) {
Review comment:
Ok ,will do
--
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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-830960493
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 3m 49s | 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 28s | master passed |
| +1 :green_heart: | compile | 1m 10s | master passed |
| +1 :green_heart: | shadedjars | 8m 8s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 15s | the patch passed |
| +1 :green_heart: | compile | 1m 10s | the patch passed |
| +1 :green_heart: | javac | 1m 10s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 6s | 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 | 140m 14s | hbase-server in the patch passed. |
| | | 174m 42s | |
| 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-3219/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3c72f42a2983 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 / 00fec24c90 |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3219/1/testReport/ |
| Max. process+thread count | 3991 (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-3219/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] Apache-HBase commented on pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831598724
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 33s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed |
| +1 :green_heart: | shadedjars | 8m 2s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 17s | the patch passed |
| +1 :green_heart: | compile | 1m 13s | the patch passed |
| +1 :green_heart: | javac | 1m 13s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 8s | 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 | 138m 31s | hbase-server in the patch passed. |
| | | 170m 2s | |
| 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-3219/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 3ee0a95a7154 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 / 7640134e3e |
| Default Java | AdoptOpenJDK-11.0.10+9 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3219/3/testReport/ |
| Max. process+thread count | 3906 (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-3219/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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831519452
:confetti_ball: **+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 | 3m 47s | master passed |
| +1 :green_heart: | compile | 1m 4s | master passed |
| +1 :green_heart: | shadedjars | 8m 7s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 37s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 36s | the patch passed |
| +1 :green_heart: | compile | 1m 1s | the patch passed |
| +1 :green_heart: | javac | 1m 1s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 12s | 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 | 149m 34s | hbase-server in the patch passed. |
| | | 179m 41s | |
| 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-3219/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 06d8919e373f 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 / 7640134e3e |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3219/2/testReport/ |
| Max. process+thread count | 4783 (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-3219/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] apurtell merged pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
apurtell merged pull request #3219:
URL: https://github.com/apache/hbase/pull/3219
--
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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831444487
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | 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 48s | master passed |
| +1 :green_heart: | compile | 3m 13s | master passed |
| +1 :green_heart: | checkstyle | 1m 2s | master passed |
| +1 :green_heart: | spotbugs | 1m 58s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 37s | the patch passed |
| +1 :green_heart: | compile | 3m 9s | the patch passed |
| +1 :green_heart: | javac | 3m 9s | the patch passed |
| +1 :green_heart: | checkstyle | 1m 1s | the patch passed |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 17m 50s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 13s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 46m 13s | |
| 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-3219/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 3356dcb3d800 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 7640134e3e |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| 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-3219/2/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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] apurtell commented on pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
apurtell commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-830875094
Tested in an integration cluster test scenario (see https://github.com/apache/hbase/pull/3208) but let's see what the unit test results in the CR report looks like.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r624968217
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +570,25 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
- if (isTableDisabled(tableStateManager, node.getTable())) {
Review comment:
yeah, I think this makes sense. +1 to retaining this check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] apurtell commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
apurtell commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r625257713
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +571,37 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
+ // DisableTableProcedure first sets the table state to DISABLED and then force unassigns
+ // the regions in a loop. The balancer should ignore all regions for tables in DISABLED
+ // state because even if still currently open we expect them to be offlined very soon.
if (isTableDisabled(tableStateManager, node.getTable())) {
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Ignoring " + node + " because table is disabled");
Review comment:
Is one better than the other? I'd say no because it's easy to edit the string and not change the arguments with printf style log lines. I've done that, I've seen others do it. For your future consideration.
--
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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831548044
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 23s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +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 3s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 34s | the patch passed |
| +1 :green_heart: | compile | 3m 11s | the patch passed |
| +1 :green_heart: | javac | 3m 11s | 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 | 17m 57s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 48m 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-3219/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 536024d0fbcd 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 / 7640134e3e |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| 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-3219/3/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=4.2.2 |
| 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] bharathv commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r624900245
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +570,25 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
- if (isTableDisabled(tableStateManager, node.getTable())) {
- continue;
- }
- if (node.getRegionInfo().isSplitParent()) {
+ // When balancing, we are only interested in OPEN or OPENING regions and expected
+ // to be online at that server until possibly the next balancer iteration or unless
+ // we decide to move it. Other states are not interesting as the region will either
+ // be closing, or splitting/merging, or will not be deployed.
+ if (!(node.isInState(State.OPEN)||node.isInState(State.OPENING))) {
Review comment:
nit: spaces missing on either side of ||
or can be simplified to !node.isInState(State.OPEN, State.OPENING)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache9 commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache9 commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r625069534
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +570,25 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
- if (isTableDisabled(tableStateManager, node.getTable())) {
Review comment:
Yes, let's add it back. It is added in HBASE-23377, https://github.com/apache/hbase/pull/908
--
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 #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-830970289
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 35s | master passed |
| +1 :green_heart: | compile | 1m 8s | master passed |
| +1 :green_heart: | shadedjars | 9m 17s | 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 56s | the patch passed |
| +1 :green_heart: | compile | 1m 6s | the patch passed |
| +1 :green_heart: | javac | 1m 6s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 39s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 151m 59s | hbase-server in the patch passed. |
| | | 184m 46s | |
| 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-3219/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3219 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1e75301071ff 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 / 00fec24c90 |
| Default Java | AdoptOpenJDK-1.8.0_282-b08 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3219/1/testReport/ |
| Max. process+thread count | 5397 (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-3219/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] bharathv commented on a change in pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#discussion_r625261529
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/master/assignment/RegionStates.java
##########
@@ -571,23 +571,37 @@ public ServerName getRegionServerOfRegion(RegionInfo regionInfo) {
* wants to iterate this exported list. We need to synchronize on regions
* since all access to this.servers is under a lock on this.regions.
*
- * @return A clone of current assignments.
+ * @return A clone of current open or opening assignments.
*/
public Map<TableName, Map<ServerName, List<RegionInfo>>> getAssignmentsForBalancer(
TableStateManager tableStateManager, List<ServerName> onlineServers) {
final Map<TableName, Map<ServerName, List<RegionInfo>>> result = new HashMap<>();
for (RegionStateNode node : regionsMap.values()) {
+ // DisableTableProcedure first sets the table state to DISABLED and then force unassigns
+ // the regions in a loop. The balancer should ignore all regions for tables in DISABLED
+ // state because even if still currently open we expect them to be offlined very soon.
if (isTableDisabled(tableStateManager, node.getTable())) {
+ if (LOG.isTraceEnabled()) {
+ LOG.trace("Ignoring {} because table is disabled", node);
+ }
continue;
}
- if (node.getRegionInfo().isSplitParent()) {
+ // When balancing, we are only interested in OPEN or OPENING regions. These can be
+ // expected to remain online until the next balancer iteration or unless the balancer
+ // decides to move it. Regions in other states are not eligible for balancing, because
+ // they are closing, splitting, merging, or otherwise already in transition.
+ if (!node.isInState(State.OPEN, State.OPENING)) {
+ if (LOG.isTraceEnabled()) {
Review comment:
Doing both isTraceEnabled() check and parameterized logging seems wasteful, its usually either one or the other.. I think the recommendation from log4j is to do the latter for concise code and avoid unnecessary temporary string objects...
--
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] apurtell commented on pull request #3219: HBASE-25836 RegionStates#getAssignmentsForBalancer should only care about OPEN or OPENING regions
Posted by GitBox <gi...@apache.org>.
apurtell commented on pull request #3219:
URL: https://github.com/apache/hbase/pull/3219#issuecomment-831425818
Address @virajjasani 's comment
--
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