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/02/02 19:12:55 UTC
[GitHub] [hbase] bharathv opened a new pull request #2922: [WIP] HBASE-25547: Thread pools should release unused resources
bharathv opened a new pull request #2922:
URL: https://github.com/apache/hbase/pull/2922
Still marked as a WIP because the defaults have not been tuned yet.
All the code is in place, I still need to identify the low priority
thread pools and tune them to release unused threads quickly.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779671714
So far, we are starting off with terminating core worker threads if no tasks arrive in keepAlive time only for EventTypes:
`RS_LOG_REPLAY_OPS`, `RS_PARALLEL_SEEK`, `MASTER_SNAPSHOT_OPERATIONS`, `MASTER_MERGE_OPERATIONS`. Is that correct? In HMaster#startServiceThreads, only `MASTER_MERGE_OPERATIONS` has this comment. Let's also add same comment for other 3 EventTypes?
Also, I might have missed it somewhere, but do we not want to keep `allowCoreThreadTimeout` as `true` by default? If the core threads are terminated, and if new task arrives, they are re-initiated right?
```
* Sets the policy governing whether core threads may time out and
* terminate if no tasks arrive within the keep-alive time, being
* replaced if needed when new tasks arrive.
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r576650821
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -207,6 +207,11 @@ public boolean allowCoreThreadTimeout() {
return allowCoreThreadTimeout;
}
+ /**
+ * Allows timing out of core threads. Good to set this for non-critical thread pools for
+ * release of unused resources. Refer to {@link ThreadPoolExecutor#allowCoreThreadTimeOut}
+ * for additional details.
+ */
Review comment:
Perfect!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779628681
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 18s | master passed |
| +1 :green_heart: | compile | 3m 21s | master passed |
| +1 :green_heart: | checkstyle | 1m 15s | master passed |
| +1 :green_heart: | spotbugs | 2m 12s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 50s | the patch passed |
| +1 :green_heart: | compile | 3m 18s | the patch passed |
| +1 :green_heart: | javac | 3m 18s | the patch passed |
| -0 :warning: | checkstyle | 1m 12s | hbase-server: The patch generated 2 new + 163 unchanged - 0 fixed = 165 total (was 163) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 3s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 19s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 50m 4s | |
| 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-2922/5/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 6d04eba9160e 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b6649a8784 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/5/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/5/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779722778
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 2m 53s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 48s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed |
| +1 :green_heart: | shadedjars | 7m 25s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 40s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 41s | the patch passed |
| +1 :green_heart: | compile | 1m 12s | the patch passed |
| +1 :green_heart: | javac | 1m 12s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 29s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 201m 30s | hbase-server in the patch passed. |
| | | 234m 28s | |
| 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-2922/5/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 714bf2386b05 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b6649a8784 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/5/testReport/ |
| Max. process+thread count | 3437 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/5/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779862038
@bharathv When you commit, can you please mention 4 EventTypes for which this commit is going to consider releasing unused core threads in thread pools? Maybe you can mention them in Release notes as well.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-777194224
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 29s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 6m 34s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 36s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 40s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 35s | hbase-server generated 5 new + 16 unchanged - 0 fixed = 21 total (was 16) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 142m 54s | hbase-server in the patch passed. |
| | | 168m 57s | |
| 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-2922/4/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f4d6ac3a2a66 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3e743df6d0 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/4/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/4/testReport/ |
| Max. process+thread count | 4638 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-777209816
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 13s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 30s | master passed |
| +1 :green_heart: | compile | 1m 12s | master passed |
| +1 :green_heart: | shadedjars | 7m 29s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 39s | the patch passed |
| +1 :green_heart: | compile | 1m 12s | the patch passed |
| +1 :green_heart: | javac | 1m 12s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 18s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 199m 36s | hbase-server in the patch passed. |
| | | 230m 25s | |
| 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-2922/4/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 8ba84d171741 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3e743df6d0 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/4/testReport/ |
| Max. process+thread count | 3622 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/4/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: [WIP] HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-771937042
:confetti_ball: **+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. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 56s | master passed |
| +1 :green_heart: | compile | 3m 8s | master passed |
| +1 :green_heart: | checkstyle | 1m 8s | master passed |
| +1 :green_heart: | spotbugs | 2m 3s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 29s | the patch passed |
| +1 :green_heart: | compile | 3m 5s | the patch passed |
| +1 :green_heart: | javac | 3m 5s | the patch passed |
| -0 :warning: | checkstyle | 1m 6s | hbase-server: The patch generated 2 new + 163 unchanged - 0 fixed = 165 total (was 163) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 17m 24s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 11s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 16s | The patch does not generate ASF License warnings. |
| | | 45m 47s | |
| 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-2922/1/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux f79ecc1b485b 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a04ea7ea44 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/1/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/1/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779850080
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 15s | 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 _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 34s | master passed |
| +1 :green_heart: | compile | 1m 12s | master passed |
| +1 :green_heart: | shadedjars | 7m 30s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 37s | the patch passed |
| +1 :green_heart: | compile | 1m 12s | the patch passed |
| +1 :green_heart: | javac | 1m 12s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 25s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 200m 22s | hbase-server in the patch passed. |
| | | 231m 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-2922/6/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 0710a3a11ae1 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b6649a8784 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/6/testReport/ |
| Max. process+thread count | 3537 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/6/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779685200
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 54s | master passed |
| +1 :green_heart: | compile | 0m 59s | master passed |
| +1 :green_heart: | shadedjars | 6m 31s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 35s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 39s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 38s | hbase-server generated 5 new + 16 unchanged - 0 fixed = 21 total (was 16) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 144m 3s | hbase-server in the patch passed. |
| | | 170m 43s | |
| 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-2922/5/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 704a2643703c 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b6649a8784 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/5/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/5/testReport/ |
| Max. process+thread count | 5019 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/5/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r570169987
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
Review comment:
Should we check this upfront, in the related setter (or maybe at a non default constructor)?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
+ return maxPoolSize;
+ }
+
+ public ExecutorConfig setMaxPoolSize(int maxPoolSize) {
Review comment:
What if the maxPoolSize is set to less than the corePoolSize? Would it be worth to check and automatically set to same as corePoolSize?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
Review comment:
Could we define minimal valid defaults for this, since we are enforcing checks later? Or maybe require setting these at construction time.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv merged pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
bharathv merged pull request #2922:
URL: https://github.com/apache/hbase/pull/2922
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-777166133
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 10s | master passed |
| +1 :green_heart: | compile | 3m 21s | master passed |
| +1 :green_heart: | checkstyle | 1m 16s | master passed |
| +1 :green_heart: | spotbugs | 2m 9s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 50s | the patch passed |
| +1 :green_heart: | compile | 3m 16s | the patch passed |
| +1 :green_heart: | javac | 3m 16s | the patch passed |
| -0 :warning: | checkstyle | 1m 15s | hbase-server: The patch generated 2 new + 163 unchanged - 0 fixed = 165 total (was 163) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 18m 58s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 49m 45s | |
| 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-2922/4/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 8a53446e2f61 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3e743df6d0 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/4/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 85 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/4/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r570838893
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
+ return maxPoolSize;
+ }
+
+ public ExecutorConfig setMaxPoolSize(int maxPoolSize) {
Review comment:
> Thanks @wchevreuil for your comments, got a couple of questions before I push a new rev.
>
> > Should we also make the type of threads queue configurable?
>
> Just to be sure, you mean ExecutorType enum? Ya thats a good idea.
>
> > My understanding is that with unbounded queues, maxPoolSize has no effect.
>
> Not sure I follow. maxPoolSize is the max number of threads not the size of backing workQueue ([javadoc](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ThreadPoolExecutor.html)). Mind rephrasing the question?
My understanding is that maxPoolSize works as a cap mostly when queueing wasn't possible, for example, because the queue was full. Revisited ThreadPoolExecutor docs:
"
...
- If fewer than corePoolSize threads are running, the Executor always prefers adding a new thread rather than queuing.
- If corePoolSize or more threads are running, the Executor always prefers queuing a request rather than adding a new thread.
- If a request cannot be queued, a new thread is created unless this would exceed maximumPoolSize, in which case, the task will be rejected.
...
There are three general strategies for queuing:
...
Unbounded queues. Using an unbounded queue (for example a LinkedBlockingQueue without a predefined capacity) will cause new tasks to wait in the queue when all corePoolSize threads are busy. Thus, no more than corePoolSize threads will ever be created. (And the value of the maximumPoolSize therefore doesn't have any effect.)
...
"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: [WIP] HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-771937042
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
bharathv commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779682472
> Let's also add same comment for other 3 EventTypes?
Instead moved it to the actual method. Copy-pasting it to multiple places seems redundant.
> If the core threads are terminated, and if new task arrives, they are re-initiated right?
JVM thread creation is expensive. It involves initializing the stack memory, creating a corresponding native thread etc.. We may not want to do it for all thread pools for performance sake. We can add other thread pools over time as we see fit but turning it on by default might have some unintended and hard to track down performance issues.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: [WIP] HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-772077291
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 52s | master passed |
| +1 :green_heart: | compile | 1m 13s | master passed |
| +1 :green_heart: | shadedjars | 8m 36s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 52s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 43s | the patch passed |
| +1 :green_heart: | compile | 1m 13s | the patch passed |
| +1 :green_heart: | javac | 1m 13s | the patch passed |
| +1 :green_heart: | shadedjars | 8m 2s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 51s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 200m 17s | hbase-server in the patch passed. |
| | | 234m 10s | |
| 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-2922/1/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 82ce466d1d7d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a04ea7ea44 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/1/testReport/ |
| Max. process+thread count | 2621 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-777078974
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 4s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 13s | master passed |
| +1 :green_heart: | compile | 3m 20s | master passed |
| +1 :green_heart: | checkstyle | 1m 17s | master passed |
| +1 :green_heart: | spotbugs | 2m 9s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 51s | the patch passed |
| +1 :green_heart: | compile | 3m 17s | the patch passed |
| +1 :green_heart: | javac | 3m 17s | the patch passed |
| -0 :warning: | checkstyle | 1m 14s | hbase-server: The patch generated 2 new + 163 unchanged - 0 fixed = 165 total (was 163) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 3s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 21s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 15s | The patch does not generate ASF License warnings. |
| | | 50m 3s | |
| 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-2922/3/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux efe24478ff06 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3e743df6d0 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/3/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/3/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
bharathv commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-780065150
I just figured that I missed the commit that rolls the op-code into the ExecutorConfig, let me create an addendum.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r570650398
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
Review comment:
I think we can remove all the checks and delegate them to ThreadPoolExecutor, it checks all these anyway.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
+ return maxPoolSize;
+ }
+
+ public ExecutorConfig setMaxPoolSize(int maxPoolSize) {
Review comment:
ThreadPoolConstructor throws in that case.
https://github.com/openjdk/jdk/blob/9a9add8825a040565051a09010b29b099c2e7d49/jdk/src/share/classes/java/util/concurrent/ThreadPoolExecutor.java#L1305
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
Review comment:
will do in next iter.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
virajjasani commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r572088450
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
Review comment:
> delegate them to ThreadPoolExecutor, it checks all these anyway
That's better. And perhaps we can just comment here that ThreadPoolExecutor can throw IllegalArgumentException if one of the following holds:
```
* {@code corePoolSize < 0}<br>
* {@code keepAliveTime < 0}<br>
* {@code maximumPoolSize <= 0}<br>
* {@code maximumPoolSize < corePoolSize}
```
or maybe just point to Javadoc of ThreadPoolExecutor constructor, anything that you feel comfortable.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -81,15 +82,16 @@ public ExecutorService(final String servername) {
/**
* Start an executor service with a given name. If there was a service already
* started with the same name, this throws a RuntimeException.
- * @param name Name of the service to start.
+ * @param config Configuration to use for the executor.
*/
- public void startExecutorService(String name, int maxThreads) {
+ public void startExecutorService(final ExecutorConfig config) {
Review comment:
Javadoc of this class seems to be referring to deprecated usage:
```
* <p>In order to create a new service, create an instance of this class and
* then do: <code>instance.startExecutorService("myService");</code>. When done
* call {@link #shutdown()}.
```
Good to change that too with this PR?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
+ return maxPoolSize;
+ }
+
+ public ExecutorConfig setMaxPoolSize(int maxPoolSize) {
Review comment:
Agree and we use unbounded queue:
```
// work queue to use - unbounded queue
final BlockingQueue<Runnable> q = new LinkedBlockingQueue<>();
```
Looks like our implementation never needed to worry about maxPoolSize so far, interesting!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r570650398
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
Review comment:
I think we can remove all the checks and delegate them to ThreadPoolExecutor, it checks all these anyway.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
+ return maxPoolSize;
+ }
+
+ public ExecutorConfig setMaxPoolSize(int maxPoolSize) {
Review comment:
ThreadPoolConstructor throws in that case.
https://github.com/openjdk/jdk/blob/9a9add8825a040565051a09010b29b099c2e7d49/jdk/src/share/classes/java/util/concurrent/ThreadPoolExecutor.java#L1305
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
Review comment:
will do in next iter.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-772916861
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | shadedjars | 6m 33s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 38s | the patch passed |
| +1 :green_heart: | compile | 1m 0s | the patch passed |
| +1 :green_heart: | javac | 1m 0s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 36s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 37s | hbase-server generated 5 new + 16 unchanged - 0 fixed = 21 total (was 16) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 140m 58s | hbase-server in the patch passed. |
| | | 167m 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-2922/2/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 2e96d2d5de8d 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d6d67d1e9f |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/2/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/2/testReport/ |
| Max. process+thread count | 4101 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] virajjasani commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
virajjasani commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779686322
Oh Great, you are right. Thread creation should be considered only if necessary for tasks like merge regions (recent change), where we are certain that it might happen for less than 1% of total cluster's available times. We are good with default value false.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] bharathv commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
bharathv commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r574098579
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -81,15 +82,16 @@ public ExecutorService(final String servername) {
/**
* Start an executor service with a given name. If there was a service already
* started with the same name, this throws a RuntimeException.
- * @param name Name of the service to start.
+ * @param config Configuration to use for the executor.
*/
- public void startExecutorService(String name, int maxThreads) {
+ public void startExecutorService(final ExecutorConfig config) {
Review comment:
Updated.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,60 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ // Argument validation and bound checks delegated to the underlying ThreadPoolExecutor
+ // implementation.
Review comment:
@wchevreuil @virajjasani I lost the old comments after a force push. I think I updated the code to get rid of maxPoolSize which is not honored for unbounded taskQueue (thanks Wellington)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-777132417
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 27s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 35s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | shadedjars | 6m 33s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 39s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 36s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 36s | hbase-server generated 5 new + 16 unchanged - 0 fixed = 21 total (was 16) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 142m 48s | hbase-server in the patch passed. |
| | | 168m 56s | |
| 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-2922/3/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f8f2135be814 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3e743df6d0 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/3/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/3/testReport/ |
| Max. process+thread count | 4715 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] wchevreuil commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
wchevreuil commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r570169987
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
Review comment:
Should we check this upfront, in the related setter (or maybe at a non default constructor)?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
+ return maxPoolSize;
+ }
+
+ public ExecutorConfig setMaxPoolSize(int maxPoolSize) {
Review comment:
What if the maxPoolSize is set to less than the corePoolSize? Would it be worth to check and automatically set to same as corePoolSize?
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
Review comment:
Could we define minimal valid defaults for this, since we are enforcing checks later? Or maybe require setting these at construction time.
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -182,12 +182,75 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
return ret;
}
+ /**
+ * Configuration wrapper for {@link Executor}.
+ */
+ public static class ExecutorConfig {
+ // Refer to ThreadPoolExecutor javadoc for details of these configuration.
+ public static final long KEEP_ALIVE_TIME_MILLIS_DEFAULT = 1000;
+ private int corePoolSize = -1;
+ private int maxPoolSize = -1;
+ private boolean allowCoreThreadTimeout = false;
+ private long keepAliveTimeMillis = KEEP_ALIVE_TIME_MILLIS_DEFAULT;
+ private String name;
+
+ public int getMaxPoolSize() {
+ Preconditions.checkState(maxPoolSize >= 1, "Invalid max pool size: " + maxPoolSize);
+ return maxPoolSize;
+ }
+
+ public ExecutorConfig setMaxPoolSize(int maxPoolSize) {
Review comment:
> Thanks @wchevreuil for your comments, got a couple of questions before I push a new rev.
>
> > Should we also make the type of threads queue configurable?
>
> Just to be sure, you mean ExecutorType enum? Ya thats a good idea.
>
> > My understanding is that with unbounded queues, maxPoolSize has no effect.
>
> Not sure I follow. maxPoolSize is the max number of threads not the size of backing workQueue ([javadoc](https://docs.oracle.com/javase/8/docs/api/java/util/concurrent/ThreadPoolExecutor.html)). Mind rephrasing the question?
My understanding is that maxPoolSize works as a cap mostly when queueing wasn't possible, for example, because the queue was full. Revisited ThreadPoolExecutor docs:
"
...
- If fewer than corePoolSize threads are running, the Executor always prefers adding a new thread rather than queuing.
- If corePoolSize or more threads are running, the Executor always prefers queuing a request rather than adding a new thread.
- If a request cannot be queued, a new thread is created unless this would exceed maximumPoolSize, in which case, the task will be rejected.
...
There are three general strategies for queuing:
...
Unbounded queues. Using an unbounded queue (for example a LinkedBlockingQueue without a predefined capacity) will cause new tasks to wait in the queue when all corePoolSize threads are busy. Thus, no more than corePoolSize threads will ever be created. (And the value of the maximumPoolSize therefore doesn't have any effect.)
...
"
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779755977
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 3s | master passed |
| +1 :green_heart: | compile | 3m 17s | master passed |
| +1 :green_heart: | checkstyle | 1m 12s | master passed |
| +1 :green_heart: | spotbugs | 2m 7s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 48s | the patch passed |
| +1 :green_heart: | compile | 3m 19s | the patch passed |
| +1 :green_heart: | javac | 3m 19s | the patch passed |
| -0 :warning: | checkstyle | 1m 12s | hbase-server: The patch generated 2 new + 163 unchanged - 0 fixed = 165 total (was 163) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 19m 4s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 18s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 12s | The patch does not generate ASF License warnings. |
| | | 49m 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-2922/6/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux 83db8f3c1252 4.15.0-126-generic #129-Ubuntu SMP Mon Nov 23 18:53:38 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b6649a8784 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/6/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 86 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/6/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] saintstack commented on a change in pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
saintstack commented on a change in pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#discussion_r572381154
##########
File path: hbase-server/src/main/java/org/apache/hadoop/hbase/executor/ExecutorService.java
##########
@@ -196,13 +259,14 @@ public void delayedSubmit(EventHandler eh, long delay, TimeUnit unit) {
private static final AtomicLong seqids = new AtomicLong(0);
private final long id;
- protected Executor(String name, int maxThreads) {
+ protected Executor(ExecutorConfig config) {
this.id = seqids.incrementAndGet();
- this.name = name;
+ this.name = config.getName();
// create the thread pool executor
this.threadPoolExecutor = new TrackingThreadPoolExecutor(
- maxThreads, maxThreads,
- keepAliveTimeInMillis, TimeUnit.MILLISECONDS, q);
+ config.getCorePoolSize(), config.getMaxPoolSize(),
+ config.getKeepAliveTimeMillis(), TimeUnit.MILLISECONDS, q);
+ this.threadPoolExecutor.allowCoreThreadTimeOut(config.allowCoreThreadTimeout());
Review comment:
Great
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-777151546
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 6s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 51s | master passed |
| +1 :green_heart: | compile | 1m 12s | master passed |
| +1 :green_heart: | shadedjars | 7m 28s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 42s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 30s | the patch passed |
| +1 :green_heart: | compile | 1m 11s | the patch passed |
| +1 :green_heart: | javac | 1m 11s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 30s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 199m 33s | hbase-server in the patch passed. |
| | | 230m 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-2922/3/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 1427a4898c6c 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / 3e743df6d0 |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/3/testReport/ |
| Max. process+thread count | 3474 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/3/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: [WIP] HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-772030733
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 28s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 52s | master passed |
| +1 :green_heart: | compile | 0m 57s | master passed |
| +1 :green_heart: | shadedjars | 6m 33s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 32s | the patch passed |
| +1 :green_heart: | compile | 0m 57s | the patch passed |
| +1 :green_heart: | javac | 0m 57s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 35s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 38s | hbase-server generated 5 new + 16 unchanged - 0 fixed = 21 total (was 16) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 141m 57s | hbase-server in the patch passed. |
| | | 168m 15s | |
| 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-2922/1/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux f55bca1c32f8 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / a04ea7ea44 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/1/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/1/testReport/ |
| Max. process+thread count | 3683 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/1/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-772859689
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 29s | Docker mode activated. |
||| _ Prechecks _ |
| +1 :green_heart: | dupname | 0m 0s | No case conflicting files found. |
| +1 :green_heart: | hbaseanti | 0m 0s | Patch does not have any anti-patterns. |
| +1 :green_heart: | @author | 0m 0s | The patch does not contain any @author tags. |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 37s | master passed |
| +1 :green_heart: | compile | 3m 7s | master passed |
| +1 :green_heart: | checkstyle | 1m 7s | master passed |
| +1 :green_heart: | spotbugs | 1m 59s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 26s | the patch passed |
| +1 :green_heart: | compile | 3m 8s | the patch passed |
| +1 :green_heart: | javac | 3m 8s | the patch passed |
| -0 :warning: | checkstyle | 1m 5s | hbase-server: The patch generated 2 new + 163 unchanged - 0 fixed = 165 total (was 163) |
| +1 :green_heart: | whitespace | 0m 0s | The patch has no whitespace issues. |
| +1 :green_heart: | hadoopcheck | 17m 20s | Patch does not cause any errors with Hadoop 3.1.2 3.2.1 3.3.0. |
| +1 :green_heart: | spotbugs | 2m 8s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | asflicense | 0m 13s | The patch does not generate ASF License warnings. |
| | | 45m 7s | |
| 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-2922/2/artifact/yetus-general-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | dupname asflicense javac spotbugs hadoopcheck hbaseanti checkstyle compile |
| uname | Linux e19783f96501 4.15.0-60-generic #67-Ubuntu SMP Thu Aug 22 16:55:30 UTC 2019 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d6d67d1e9f |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| checkstyle | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/2/artifact/yetus-general-check/output/diff-checkstyle-hbase-server.txt |
| Max. process+thread count | 96 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/2/console |
| versions | git=2.17.1 maven=3.6.3 spotbugs=3.1.12 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-779816536
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 0m 31s | Docker mode activated. |
| -0 :warning: | yetus | 0m 4s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 28s | master passed |
| +1 :green_heart: | compile | 0m 58s | master passed |
| +1 :green_heart: | shadedjars | 6m 36s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 38s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 3m 30s | the patch passed |
| +1 :green_heart: | compile | 0m 58s | the patch passed |
| +1 :green_heart: | javac | 0m 58s | the patch passed |
| +1 :green_heart: | shadedjars | 6m 33s | patch has no errors when building our shaded downstream artifacts. |
| -0 :warning: | javadoc | 0m 38s | hbase-server generated 5 new + 16 unchanged - 0 fixed = 21 total (was 16) |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 144m 0s | hbase-server in the patch passed. |
| | | 170m 9s | |
| 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-2922/6/artifact/yetus-jdk8-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux 9300f05f3340 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / b6649a8784 |
| Default Java | AdoptOpenJDK-1.8.0_232-b09 |
| javadoc | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/6/artifact/yetus-jdk8-hadoop3-check/output/diff-javadoc-javadoc-hbase-server.txt |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/6/testReport/ |
| Max. process+thread count | 4352 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/6/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [hbase] Apache-HBase commented on pull request #2922: HBASE-25547: Thread pools should release unused resources
Posted by GitBox <gi...@apache.org>.
Apache-HBase commented on pull request #2922:
URL: https://github.com/apache/hbase/pull/2922#issuecomment-772946914
:confetti_ball: **+1 overall**
| Vote | Subsystem | Runtime | Comment |
|:----:|----------:|--------:|:--------|
| +0 :ok: | reexec | 1m 8s | Docker mode activated. |
| -0 :warning: | yetus | 0m 3s | Unprocessed flag(s): --brief-report-file --spotbugs-strict-precheck --whitespace-eol-ignore-list --whitespace-tabs-ignore-list --quick-hadoopcheck |
||| _ Prechecks _ |
||| _ master Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 38s | master passed |
| +1 :green_heart: | compile | 1m 11s | master passed |
| +1 :green_heart: | shadedjars | 7m 21s | branch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 43s | master passed |
||| _ Patch Compile Tests _ |
| +1 :green_heart: | mvninstall | 4m 33s | the patch passed |
| +1 :green_heart: | compile | 1m 11s | the patch passed |
| +1 :green_heart: | javac | 1m 11s | the patch passed |
| +1 :green_heart: | shadedjars | 7m 22s | patch has no errors when building our shaded downstream artifacts. |
| +1 :green_heart: | javadoc | 0m 41s | the patch passed |
||| _ Other Tests _ |
| +1 :green_heart: | unit | 195m 27s | hbase-server in the patch passed. |
| | | 226m 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-2922/2/artifact/yetus-jdk11-hadoop3-check/output/Dockerfile |
| GITHUB PR | https://github.com/apache/hbase/pull/2922 |
| JIRA Issue | HBASE-25547 |
| Optional Tests | javac javadoc unit shadedjars compile |
| uname | Linux ae0297d0bdff 4.15.0-112-generic #113-Ubuntu SMP Thu Jul 9 23:41:39 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux |
| Build tool | maven |
| Personality | dev-support/hbase-personality.sh |
| git revision | master / d6d67d1e9f |
| Default Java | AdoptOpenJDK-11.0.6+10 |
| Test Results | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/2/testReport/ |
| Max. process+thread count | 2614 (vs. ulimit of 30000) |
| modules | C: hbase-server U: hbase-server |
| Console output | https://ci-hadoop.apache.org/job/HBase/job/HBase-PreCommit-GitHub-PR/job/PR-2922/2/console |
| versions | git=2.17.1 maven=3.6.3 |
| Powered by | Apache Yetus 0.12.0 https://yetus.apache.org |
This message was automatically generated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org