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/07/19 20:48:32 UTC

[GitHub] [hbase] shahrs87 opened a new pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

shahrs87 opened a new pull request #3506:
URL: https://github.com/apache/hbase/pull/3506


   


-- 
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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-883343557






-- 
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] shahrs87 commented on a change in pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
shahrs87 commented on a change in pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#discussion_r673047325



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Connection.java
##########
@@ -109,16 +109,15 @@ default Table getTable(TableName tableName, ExecutorService pool) throws IOExcep
   /**
    * <p>
    * Retrieve a {@link BufferedMutator} for performing client-side buffering of writes. The
-   * {@link BufferedMutator} returned by this method is thread-safe. This BufferedMutator will
-   * use the Connection's ExecutorService. This object can be used for long lived operations.
+   * {@link BufferedMutator} returned by this method is thread-safe.
+   * This accessor will create a new ThreadPoolExecutor and will be shutdown once we close the
+   *  BufferedMutator. This object can be used for long lived operations.

Review comment:
       @anoopsjohn  Done ! Please review again !




-- 
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] anoopsjohn commented on a change in pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#discussion_r672768084



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Connection.java
##########
@@ -109,16 +109,15 @@ default Table getTable(TableName tableName, ExecutorService pool) throws IOExcep
   /**
    * <p>
    * Retrieve a {@link BufferedMutator} for performing client-side buffering of writes. The
-   * {@link BufferedMutator} returned by this method is thread-safe. This BufferedMutator will
-   * use the Connection's ExecutorService. This object can be used for long lived operations.
+   * {@link BufferedMutator} returned by this method is thread-safe.
+   * This accessor will create a new ThreadPoolExecutor and will be shutdown once we close the
+   *  BufferedMutator. This object can be used for long lived operations.

Review comment:
       Nit: May be for API getBufferedMutator(BufferedMutatorParams params) we can say clearly that if user passes a ThreadPool in params, we will use that or else same as this method (new one and handled by us on close)




-- 
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] shahrs87 commented on pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
shahrs87 commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882851077


   @saintstack @anoopsjohn @apurtell This PR is just for branch-2. The behavior is different in master branch. In master brach, there is only AsyncConnection and I don't see it creating a new ThreadPoolExecutor but still studying master code base. Please 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.

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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882891511


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   2m  5s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 47s |  branch-2 passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  branch-2 passed  |
   | +1 :green_heart: |  shadedjars  |   7m 18s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  branch-2 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 26s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 48s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 56s |  hbase-client in the patch passed.  |
   |  |   |  31m 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-3506/2/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3506 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 88ca43cee7ff 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2 / cabf0dc51d |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/2/testReport/ |
   | Max. process+thread count | 236 (vs. ulimit of 12500) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Reidddddd commented on pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Reidddddd commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-883016273


   > Just a nit.
   > This issue is not there in branch-1 anyways right?
   > So for all branch-2 branches, we are sticking with the existing behaviour to have the compatibility and correcting the Javadoc. Pls mark this issue accordingly and we can address master branch (3.x also?) in another task. There we can even change to use the current javadoc way of using Connection's pool (If bug is there still)
   
   branch-1 seems no such issue, because there's no this flag `cleanupPoolOnClose`, so the pool no matter created by default or provided by applications will be shutdown


-- 
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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882890166


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 11s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  5s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 59s |  branch-2 passed  |
   | +1 :green_heart: |  compile  |   0m 33s |  branch-2 passed  |
   | +1 :green_heart: |  shadedjars  |   6m 41s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 28s |  branch-2 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 32s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 52s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 50s |  hbase-client in the patch passed.  |
   |  |   |  29m  0s |   |
   
   
   | 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-3506/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3506 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux f46173bf6ac8 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2 / cabf0dc51d |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/2/testReport/ |
   | Max. process+thread count | 298 (vs. ulimit of 12500) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/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.

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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882864532






-- 
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] Reidddddd commented on pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Reidddddd commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-883016273


   > Just a nit.
   > This issue is not there in branch-1 anyways right?
   > So for all branch-2 branches, we are sticking with the existing behaviour to have the compatibility and correcting the Javadoc. Pls mark this issue accordingly and we can address master branch (3.x also?) in another task. There we can even change to use the current javadoc way of using Connection's pool (If bug is there still)
   
   branch-1 seems no such issue, because there's no this flag `cleanupPoolOnClose`, so the pool no matter created by default or provided by applications will be shutdown


-- 
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] anoopsjohn commented on a change in pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#discussion_r672768084



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Connection.java
##########
@@ -109,16 +109,15 @@ default Table getTable(TableName tableName, ExecutorService pool) throws IOExcep
   /**
    * <p>
    * Retrieve a {@link BufferedMutator} for performing client-side buffering of writes. The
-   * {@link BufferedMutator} returned by this method is thread-safe. This BufferedMutator will
-   * use the Connection's ExecutorService. This object can be used for long lived operations.
+   * {@link BufferedMutator} returned by this method is thread-safe.
+   * This accessor will create a new ThreadPoolExecutor and will be shutdown once we close the
+   *  BufferedMutator. This object can be used for long lived operations.

Review comment:
       Nit: May be for API getBufferedMutator(BufferedMutatorParams params) we can say clearly that if user passes a ThreadPool in params, we will use that or else same as this method (new one and handled by us on close)




-- 
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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882875698


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 46s |  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.  |
   ||| _ branch-2 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 41s |  branch-2 passed  |
   | +1 :green_heart: |  compile  |   1m 22s |  branch-2 passed  |
   | +1 :green_heart: |  checkstyle  |   0m 49s |  branch-2 passed  |
   | +1 :green_heart: |  spotbugs  |   1m 41s |  branch-2 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 29s |  the patch passed  |
   | +1 :green_heart: |  compile  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  javac  |   1m 19s |  the patch passed  |
   | +1 :green_heart: |  checkstyle  |   0m 41s |  the patch passed  |
   | +1 :green_heart: |  whitespace  |   0m  0s |  The patch has no whitespace issues.  |
   | +1 :green_heart: |  hadoopcheck  |  14m 36s |  Patch does not cause any errors with Hadoop 3.1.2 3.2.1.  |
   | +1 :green_heart: |  spotbugs  |   1m 38s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  asflicense  |   0m 14s |  The patch does not generate ASF License warnings.  |
   |  |   |  42m 52s |   |
   
   
   | 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-3506/1/artifact/yetus-general-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3506 |
   | Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
   | uname | Linux b3d2c31b3cad 4.15.0-142-generic #146-Ubuntu SMP Tue Apr 13 01:11:19 UTC 2021 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2 / cabf0dc51d |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   | Max. process+thread count | 86 (vs. ulimit of 12500) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/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.

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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882864532






-- 
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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882864532


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   0m 37s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  7s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 43s |  branch-2 passed  |
   | +1 :green_heart: |  compile  |   0m 26s |  branch-2 passed  |
   | +1 :green_heart: |  shadedjars  |   6m  2s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 27s |  branch-2 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   3m 21s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 27s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   5m 58s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 25s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 22s |  hbase-client in the patch passed.  |
   |  |   |  25m 12s |   |
   
   
   | 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-3506/1/artifact/yetus-jdk8-hadoop2-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3506 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux 44df0013242f 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 | branch-2 / cabf0dc51d |
   | Default Java | AdoptOpenJDK-1.8.0_282-b08 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/1/testReport/ |
   | Max. process+thread count | 346 (vs. ulimit of 12500) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] Reidddddd commented on pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Reidddddd commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-883016273


   > Just a nit.
   > This issue is not there in branch-1 anyways right?
   > So for all branch-2 branches, we are sticking with the existing behaviour to have the compatibility and correcting the Javadoc. Pls mark this issue accordingly and we can address master branch (3.x also?) in another task. There we can even change to use the current javadoc way of using Connection's pool (If bug is there still)
   
   branch-1 seems no such issue, because there's no this flag `cleanupPoolOnClose`, so the pool no matter created by default or provided by applications will be shutdown


-- 
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] anoopsjohn commented on a change in pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
anoopsjohn commented on a change in pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#discussion_r672768084



##########
File path: hbase-client/src/main/java/org/apache/hadoop/hbase/client/Connection.java
##########
@@ -109,16 +109,15 @@ default Table getTable(TableName tableName, ExecutorService pool) throws IOExcep
   /**
    * <p>
    * Retrieve a {@link BufferedMutator} for performing client-side buffering of writes. The
-   * {@link BufferedMutator} returned by this method is thread-safe. This BufferedMutator will
-   * use the Connection's ExecutorService. This object can be used for long lived operations.
+   * {@link BufferedMutator} returned by this method is thread-safe.
+   * This accessor will create a new ThreadPoolExecutor and will be shutdown once we close the
+   *  BufferedMutator. This object can be used for long lived operations.

Review comment:
       Nit: May be for API getBufferedMutator(BufferedMutatorParams params) we can say clearly that if user passes a ThreadPool in params, we will use that or else same as this method (new one and handled by us on close)




-- 
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] anoopsjohn merged pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
anoopsjohn merged pull request #3506:
URL: https://github.com/apache/hbase/pull/3506


   


-- 
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] shahrs87 commented on pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
shahrs87 commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882851077


   @saintstack @anoopsjohn @apurtell This PR is just for branch-2. The behavior is different in master branch. In master brach, there is only AsyncConnection and I don't see it creating a new ThreadPoolExecutor but still studying master code base. Please 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.

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 #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882867063


   :confetti_ball: **+1 overall**
   
   
   
   
   
   
   | Vote | Subsystem | Runtime | Comment |
   |:----:|----------:|--------:|:--------|
   | +0 :ok: |  reexec  |   1m 10s |  Docker mode activated.  |
   | -0 :warning: |  yetus  |   0m  8s |  Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck  |
   ||| _ Prechecks _ |
   ||| _ branch-2 Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m 19s |  branch-2 passed  |
   | +1 :green_heart: |  compile  |   0m 31s |  branch-2 passed  |
   | +1 :green_heart: |  shadedjars  |   6m 47s |  branch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 30s |  branch-2 passed  |
   ||| _ Patch Compile Tests _ |
   | +1 :green_heart: |  mvninstall  |   4m  0s |  the patch passed  |
   | +1 :green_heart: |  compile  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  javac  |   0m 30s |  the patch passed  |
   | +1 :green_heart: |  shadedjars  |   6m 45s |  patch has no errors when building our shaded downstream artifacts.  |
   | +1 :green_heart: |  javadoc  |   0m 29s |  the patch passed  |
   ||| _ Other Tests _ |
   | +1 :green_heart: |  unit  |   2m 42s |  hbase-client in the patch passed.  |
   |  |   |  29m 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-3506/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
   | GITHUB PR | https://github.com/apache/hbase/pull/3506 |
   | Optional Tests | javac javadoc unit shadedjars compile |
   | uname | Linux d9a81630bbf8 4.15.0-65-generic #74-Ubuntu SMP Tue Sep 17 17:06:04 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
   | Build tool | maven |
   | Personality | dev-support/hbase-personality.sh |
   | git revision | branch-2 / cabf0dc51d |
   | Default Java | AdoptOpenJDK-11.0.10+9 |
   |  Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/1/testReport/ |
   | Max. process+thread count | 292 (vs. ulimit of 12500) |
   | modules | C: hbase-client U: hbase-client |
   | Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-3506/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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [hbase] shahrs87 commented on pull request #3506: HBASE-26088 Fix thread leaks in conn#getBufferedMutator(tableName) method call

Posted by GitBox <gi...@apache.org>.
shahrs87 commented on pull request #3506:
URL: https://github.com/apache/hbase/pull/3506#issuecomment-882851077


   @saintstack @anoopsjohn @apurtell This PR is just for branch-2. The behavior is different in master branch. In master brach, there is only AsyncConnection and I don't see it creating a new ThreadPoolExecutor but still studying master code base. Please 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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org