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 2020/01/06 22:59:03 UTC
[GitHub] [hbase] bharathv opened a new pull request #994: HBASE-23648:
Re-use underlying connection registry in RawAsyncHBaseAdmin
bharathv opened a new pull request #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994
No need to create and close a new registry on demand. Audited other
usages of getRegistry() and the code looks fine.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
bharathv commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-572740376
@saintstack No problemo! Appreciate the reviews.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
bharathv commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-572249254
@saintstack is this good to go? thanks for the review.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv edited a comment on issue #994: HBASE-23648:
Re-use underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
bharathv edited a comment on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-571453526
> LGTM. Where does it get committed? On branch?
Yes please, feature branch. It builds on the "ConnectionRegistry" change that exists only in the branch. I'm hoping the branch gets merged into the master soon.
>For the future, try to minimize your changes. Most of the below is just refactoring. Takes a while to verify no accidental breakage. For the future.
Ack. I see your point. Generally I'm trying to fix all the check style issues around the places I'm touching, there are tons of violations. But I guess it is better to bulk fix them in separate changes.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] saintstack commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
saintstack commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-572735020
Sorry @bharathv I left it too long? There are conflicts? Mind fixing please?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
bharathv commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-571453526
> LGTM. Where does it get committed? On branch?
Yes please, feature branch. It builds on the "ConnectionRegistry" change that exists only in the branch. I'm hoping the branch gets merged into the master soon.
>For the future, try to minimize your changes. Most of the below is just refactoring. Takes a while to verify no accidental breakage. For the future.
Ack. I see your point. Generally I'm trying to fix all the check style issues around the places I'm touching, there are tons of violations. But I guess it is better to bulk fix them in separate changes.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] saintstack merged pull request #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
saintstack merged pull request #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-571367571
: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: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ HBASE-18095/client-locate-meta-no-zookeeper Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 33s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +1 :green_heart: | compile | 0m 26s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +1 :green_heart: | checkstyle | 0m 34s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +1 :green_heart: | shadedjars | 4m 37s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +0 :ok: | spotbugs | 1m 9s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 8s | HBASE-18095/client-locate-meta-no-zookeeper passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 7s | the patch passed |
| +1 :green_heart: | compile | 0m 26s | the patch passed |
| +1 :green_heart: | javac | 0m 26s | the patch passed |
| -1 :x: | checkstyle | 0m 31s | hbase-client: The patch generated 1 new + 13 unchanged - 0 fixed = 14 total (was 13) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 41s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 15m 50s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 23s | the patch passed |
| +1 :green_heart: | findbugs | 1m 14s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 59s | hbase-client in the patch passed. |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 50m 13s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-994/1/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/994 |
| JIRA Issue | HBASE-23648 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 2561f2b295d6 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-994/out/precommit/personality/provided.sh |
| git revision | HBASE-18095/client-locate-meta-no-zookeeper / d016cd9962 |
| Default Java | 1.8.0_181 |
| checkstyle | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-994/1/artifact/out/diff-checkstyle-hbase-client.txt |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-994/1/testReport/ |
| Max. process+thread count | 288 (vs. ulimit of 10000) |
| modules | C: hbase-client U: hbase-client |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-994/1/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
bharathv commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-571373603
Existing test coverage should be good enough, hence no new tests are added.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] bharathv commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
bharathv commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-571354007
@saintstack / @Apache9 A small fix that fixes ad-hoc creation of registries. I audited the code and other invocations look fine.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [hbase] Apache-HBase commented on issue #994: HBASE-23648: Re-use
underlying connection registry in RawAsyncHBaseAdmin
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on issue #994: HBASE-23648: Re-use underlying connection registry in RawAsyncHBaseAdmin
URL: https://github.com/apache/hbase/pull/994#issuecomment-571388033
:confetti_ball: **+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: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
| -0 :warning: | test4tests | 0m 0s | The patch doesn't appear to include any new or modified tests. Please justify why no new tests are needed for this patch. Also please list what manual steps were performed to verify this patch. |
||| _ HBASE-18095/client-locate-meta-no-zookeeper Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 31s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +1 :green_heart: | compile | 0m 25s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +1 :green_heart: | checkstyle | 0m 32s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +1 :green_heart: | shadedjars | 4m 42s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 25s | HBASE-18095/client-locate-meta-no-zookeeper passed |
| +0 :ok: | spotbugs | 1m 8s | Used deprecated FindBugs config; considering switching to SpotBugs. |
| +1 :green_heart: | findbugs | 1m 5s | HBASE-18095/client-locate-meta-no-zookeeper passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 5m 0s | the patch passed |
| +1 :green_heart: | compile | 0m 25s | the patch passed |
| +1 :green_heart: | javac | 0m 25s | the patch passed |
| +1 :green_heart: | checkstyle | 0m 31s | hbase-client: The patch generated 0 new + 0 unchanged - 13 fixed = 0 total (was 13) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | shadedjars | 4m 36s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | hadoopcheck | 15m 47s | Patch does not cause any errors with Hadoop 2.8.5 2.9.2 or 3.1.2. |
| +1 :green_heart: | javadoc | 0m 24s | the patch passed |
| +1 :green_heart: | findbugs | 1m 12s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 1m 58s | hbase-client in the patch passed. |
| +1 :green_heart: | asflicense | 0m 14s | The patch does not generate ASF License warnings. |
| | | 49m 36s | |
| Subsystem | Report/Notes |
|----------:|:-------------|
| Docker | Client=19.03.5 Server=19.03.5 base: https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-994/2/artifact/out/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/994 |
| JIRA Issue | HBASE-23648 |
| Optional Tests | dupname asflicense javac javadoc unit spotbugs findbugs shadedjars hadoopcheck hbaseanti checkstyle compile |
| uname | Linux b8d4fae3599c 4.15.0-58-generic #64-Ubuntu SMP Tue Aug 6 11:12:41 UTC 2019 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | /home/jenkins/jenkins-slave/workspace/HBase-PreCommit-GitHub-PR_PR-994/out/precommit/personality/provided.sh |
| git revision | HBASE-18095/client-locate-meta-no-zookeeper / d016cd9962 |
| Default Java | 1.8.0_181 |
| Test Results | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-994/2/testReport/ |
| Max. process+thread count | 289 (vs. ulimit of 10000) |
| modules | C: hbase-client U: hbase-client |
| Console output | https://builds.apache.org/job/HBase-PreCommit-GitHub-PR/job/PR-994/2/console |
| versions | git=2.11.0 maven=2018-06-17T18:33:14Z) findbugs=3.1.11 |
| Powered by | Apache Yetus 0.11.1 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
With regards,
Apache Git Services