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/12/22 07:22:29 UTC
[GitHub] [hbase] YutSean opened a new pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
YutSean opened a new pull request #3972:
URL: https://github.com/apache/hbase/pull/3972
RSGroupAdmin#getRSGroupOfServer
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999943288
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 4 new + 411 unchanged - 5 fixed = 415 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 29s | The patch does not generate ASF License warnings. |
| | | 4m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/11/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 961fa7dfe24c 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/11/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/11/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-1000745100
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 35s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 11 new + 411 unchanged - 5 fixed = 422 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 27s | The patch does not generate ASF License warnings. |
| | | 4m 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-3972/12/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux af82cccdfbaa 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/12/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/12/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999475091
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 1s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 7 new + 413 unchanged - 3 fixed = 420 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 30s | The patch does not generate ASF License warnings. |
| | | 4m 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-3972/8/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 7ed32b3f56b4 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/8/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/8/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] virajjasani commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r774009875
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,22 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ # If the rsgroup is nil, that means this server belongs to no rsgroup.
+ # It should be already offline.
+ # Here we directly log and exit.
+ return rsgroup unless rsgroup.nil?
+ $LOG.info('The server ' + hostname + 'belongs to no rsgroup. Exit regions moving.')
+ exit
Review comment:
This should be `exit 0`?
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999455685
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 1 new + 0 unchanged - 416 fixed = 1 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 25s | The patch does not generate ASF License warnings. |
| | | 4m 23s | |
| 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-3972/6/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 8c5c259b6168 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/6/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/6/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999384656
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 1 new + 415 unchanged - 1 fixed = 416 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 29s | The patch does not generate ASF License warnings. |
| | | 4m 31s | |
| 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-3972/4/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 9d14964f85da 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/4/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/4/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999498995
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 37s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 4 new + 411 unchanged - 5 fixed = 415 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 29s | The patch does not generate ASF License warnings. |
| | | 4m 32s | |
| 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-3972/10/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux ca9714e95f03 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/10/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/10/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] YutSean commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
YutSean commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r776709051
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,30 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(servers, rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ server_name = getServerName(servers, hostname, port)
+ regions = getRegions(getConfiguration(), server_name)
+ if !regions.nil? && regions.empty?
Review comment:
Checked the code. Currently, RSRpcService will return an empty list of regions as the RPC response or throw a RemoteServiceException. It should be not nil at the client side. However, I also found that the ProtobufUtil used at client side will return null when the response is null. In my opinion, it is a guarantee here. Could be left as it is currently. What do you think?
--
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] virajjasani commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r774895851
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,22 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ # If the rsgroup is nil, that means this server belongs to no rsgroup.
+ # It should be already offline.
+ # Here we directly log and exit.
+ return rsgroup unless rsgroup.nil?
+ $LOG.info('The server ' + hostname + 'belongs to no rsgroup. Exit regions moving.')
Review comment:
From the overall system viewpoint, this might be seen as a normal behaviour that region_mover exited gracefully as expected, however from region_mover's own viewpoint, yes it makes sense to convert it to WARN level because the same behaviour is possible only when either the given server is in zombie state or maybe some other issues persist in RSGroup APIs.
--
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] YutSean commented on pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
YutSean commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999528975
@virajjasani It seems that some of the rules of rubocop are strange.
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-1003015190
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 5m 14s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 7 new + 411 unchanged - 5 fixed = 418 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 28s | The patch does not generate ASF License warnings. |
| | | 9m 8s | |
| 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-3972/15/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 31be244f5dea 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / e8a8523bb0 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/15/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/15/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] virajjasani commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r774894839
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,22 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ # If the rsgroup is nil, that means this server belongs to no rsgroup.
+ # It should be already offline.
+ # Here we directly log and exit.
+ return rsgroup unless rsgroup.nil?
+ $LOG.info('The server ' + hostname + 'belongs to no rsgroup. Exit regions moving.')
+ exit 0
Review comment:
> It's okay to exit with 0 if the server has no rsgroup AND has no regions.
> I don't know whether a call to `getOnlineRegions` would succeed.
Hmm, I agree with first statement and that second statement would also be true.
How about we check something like this?
```
rsgroup = rsgroupAdmin.getRSGroupOfServer
if rsgroup.nil?
try
regions = admin.getOnlineRegions(server)
except
LOG.warn(.....)
exit 0
if not regions.nil? and regions.size == 0:
exit 0
else
LOG.warn('Server is not part of any RSGroup and still hosting some regions')
exit 5
```
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999348735
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 4m 43s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 4 new + 415 unchanged - 1 fixed = 419 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 28s | The patch does not generate ASF License warnings. |
| | | 8m 53s | |
| 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-3972/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux cbbc3ec79e17 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/1/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/1/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] d-c-manning commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r774737883
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,22 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ # If the rsgroup is nil, that means this server belongs to no rsgroup.
+ # It should be already offline.
+ # Here we directly log and exit.
+ return rsgroup unless rsgroup.nil?
+ $LOG.info('The server ' + hostname + 'belongs to no rsgroup. Exit regions moving.')
+ exit 0
Review comment:
The code and syntax looks good to me
>Here, I see one minor issue: if we go with this change, still the unload region would not make a graceful exit:
Can we instead call rsgroupAdmin.getRSGroupOfServer outside of getSameRSGroupServers method and if it returns nil, we can log a message and then exit gracefully?
I do question this design decision, though. If a server has regions on it, but fails to find any target servers to unload to, then that seems like a reason to exit with an error code.
So then I would propose that we explicitly check for this case. It's okay to exit with 0 if the server has no rsgroup AND has no regions. This would obviously require additional refactoring of the code, since currently we don't check for regions until after we obtain the rsgroup of the server. And in the case mentioned in HBASE-26596, I don't know whether a call to `getOnlineRegions` would succeed.
This is only my opinion, so feel free to ignore if you feel differently 🙂
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,22 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ # If the rsgroup is nil, that means this server belongs to no rsgroup.
+ # It should be already offline.
+ # Here we directly log and exit.
+ return rsgroup unless rsgroup.nil?
+ $LOG.info('The server ' + hostname + 'belongs to no rsgroup. Exit regions moving.')
Review comment:
This feels like it justifies at least a `warn` level log
--
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] virajjasani commented on pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-1000058359
@d-c-manning would you like to take a look?
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999452079
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 4 new + 415 unchanged - 1 fixed = 419 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 28s | The patch does not generate ASF License warnings. |
| | | 4m 35s | |
| 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-3972/5/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux da7d028ffeea 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/5/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/5/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-1000750362
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 6s | The patch generated 7 new + 411 unchanged - 5 fixed = 418 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 29s | The patch does not generate ASF License warnings. |
| | | 4m 29s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/13/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux b28cc10608c2 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/13/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/13/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] virajjasani commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r776597674
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,29 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ regions = getRegions(getConfiguration(), hostname)
Review comment:
As of today, we don't have any particular UT to validate region_mover functionality on branch-1 (one of the primary reasons to move region_mover logic to RegionMover class in branch-2), hence we need to validate it locally by running region_mover.rb (with RSGroup enabled for this change at least).
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-1002368750
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 40s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 7 new + 411 unchanged - 5 fixed = 418 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 28s | The patch does not generate ASF License warnings. |
| | | 4m 37s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/14/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux ca5cb3c53502 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / e8a8523bb0 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/14/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/14/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] YutSean commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
YutSean commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r776131590
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,29 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ regions = getRegions(getConfiguration(), hostname)
Review comment:
Fixed this in the latest commit. But I have one more question. It seems that we don't have UTs like java code for rb scripts. How could we test it currently?
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999486740
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 33s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 5 new + 411 unchanged - 5 fixed = 416 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 28s | The patch does not generate ASF License warnings. |
| | | 4m 34s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | ClientAPI=1.41 ServerAPI=1.41 base: https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/9/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 273be5a19429 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/9/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/9/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999363992
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 34s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 5s | The patch generated 3 new + 415 unchanged - 1 fixed = 418 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 29s | The patch does not generate ASF License warnings. |
| | | 4m 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-3972/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 45b39b6bb6ac 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/2/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/2/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999466121
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 39s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 6s | The patch generated 10 new + 413 unchanged - 3 fixed = 423 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 31s | The patch does not generate ASF License warnings. |
| | | 4m 39s | |
| 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-3972/7/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 30fc36144459 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/7/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/7/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] virajjasani commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r776598215
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,30 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(servers, rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ server_name = getServerName(servers, hostname, port)
+ regions = getRegions(getConfiguration(), server_name)
+ if !regions.nil? && regions.empty?
+ exit 0
+ else
+ $LOG.warn('The server ' + hostname + ' belongs to no rsgroup. But still holds regions.')
+ exit 5
+ end
+ rescue java.io.IOException => e
+ $LOG.warn(e)
+ exit 0
Review comment:
Since we want to exit gracefully, we should log it at INFO level
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,30 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(servers, rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ server_name = getServerName(servers, hostname, port)
+ regions = getRegions(getConfiguration(), server_name)
+ if !regions.nil? && regions.empty?
Review comment:
Can regions be nil here by any chance? Might have to evaluate this to confirm:
`ProtobufUtil::getOnlineRegions(connection.getAdmin(ServerName.valueOf(servername)))`
--
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] YutSean commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
YutSean commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r776709653
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,30 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(servers, rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ server_name = getServerName(servers, hostname, port)
+ regions = getRegions(getConfiguration(), server_name)
+ if !regions.nil? && regions.empty?
+ exit 0
+ else
+ $LOG.warn('The server ' + hostname + ' belongs to no rsgroup. But still holds regions.')
+ exit 5
+ end
+ rescue java.io.IOException => e
+ $LOG.warn(e)
+ exit 0
Review comment:
Fixed in the latest commit.
--
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] virajjasani commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r777078839
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,30 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(servers, rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ server_name = getServerName(servers, hostname, port)
+ regions = getRegions(getConfiguration(), server_name)
+ if !regions.nil? && regions.empty?
Review comment:
Sounds good. Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] d-c-manning commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r776173184
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,29 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ regions = getRegions(getConfiguration(), hostname)
Review comment:
I too was wondering whether there was any test code.
--
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 #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999372796
:broken_heart: **-1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 36s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 1s | No case conflicting files found. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ branch-1 Compile Tests _ |
||| _ Patch Compile Tests _ |
| -1 :x: | rubocop | 0m 4s | The patch generated 1 new + 0 unchanged - 416 fixed = 1 total (was 416) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 1m 29s | The patch does not generate ASF License warnings. |
| | | 4m 38s | |
| 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-3972/3/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/3972 |
| Optional Tests | dupname asflicense rubocop |
| uname | Linux 1ffea5300570 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 | /home/jenkins/jenkins-home/workspace/Base-PreCommit-GitHub-PR_PR-3972/out/precommit/personality/provided.sh |
| git revision | branch-1 / 1d1b52cb55 |
| rubocop | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/3/artifact/out/diff-patch-rubocop.txt |
| Max. process+thread count | 41 (vs. ulimit of 10000) |
| modules | C: . U: . |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3972/3/console |
| versions | git=2.17.1 maven=3.6.0 rubocop=0.81.0 |
| 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] d-c-manning commented on a change in pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
d-c-manning commented on a change in pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#discussion_r776033981
##########
File path: bin/region_mover.rb
##########
@@ -493,11 +493,29 @@ def getFilename(options, targetServer, port)
return filename
end
+def getServersRSGroup(rsgroup_admin, hostname, port)
+ rsgroup = rsgroup_admin.getRSGroupOfServer(Address.fromParts(hostname,
+ java.lang.Integer.parseInt(port)))
+ return rsgroup unless rsgroup.nil?
+ begin
+ regions = getRegions(getConfiguration(), hostname)
Review comment:
Does this work? I believe the call to `getRegions` requires a `ServerName` object, and here we just have a string of the `hostname` (with no `port` or `startcode`.) Compare to https://github.com/apache/hbase/blob/1813237699a72f666217c352525c6736781fe3d9/bin/region_mover.rb#L319-L330 where we call `stripServer` to get the host's `ServerName` object from the call to `getServers` which returns a collection of `ServerName`.
--
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] virajjasani commented on pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-999700054
> @virajjasani It seems that some of the rules of rubocop are strange.
No worries, you can ignore them for now.
--
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] YutSean commented on pull request #3972: HBASE-26596 region_mover should gracefully ignore null response from RSGroupAdmin#getRSGroupOfServer
Posted by GitBox <gi...@apache.org>.
YutSean commented on pull request #3972:
URL: https://github.com/apache/hbase/pull/3972#issuecomment-1000748586
Did the change as the above discussion.
--
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