[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19458 ) Change subject: IMPALA-11865: Set thread names for Java thread pools .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/19458/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/19458/2/fe/src/main/java/org/apache/impala/service/Frontend.java@447 PS2, Line 447: new ThreadFactoryBuilder().setDaemon(true) > Oh, I missed that. Let me revert and we can do it right. I'm going off https://docs.oracle.com/javase/7/docs/api/java/util/concurrent/Executors.html#defaultThreadFactory(), which notes "Each new thread is created as a non-daemon thread". -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Tue, 31 Jan 2023 00:11:47 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19458 ) Change subject: IMPALA-11865: Set thread names for Java thread pools .. Patch Set 2: (1 comment) http://gerrit.cloudera.org:8080/#/c/19458/2/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/19458/2/fe/src/main/java/org/apache/impala/service/Frontend.java@447 PS2, Line 447: new ThreadFactoryBuilder().setDaemon(true) > Marking as daemon threads seems to make sense for this. It does look like i Oh, I missed that. Let me revert and we can do it right. -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Tue, 31 Jan 2023 00:10:25 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Michael Smith has posted comments on this change. (
http://gerrit.cloudera.org:8080/19458 )
Change subject: IMPALA-11865: Set thread names for Java thread pools
..
Patch Set 2:
(2 comments)
Noticed a few things after merge.
http://gerrit.cloudera.org:8080/#/c/19458/2/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/19458/2/fe/src/main/java/org/apache/impala/service/Frontend.java@394
PS2, Line 394:* running on the {@link #policyReader_} thread.
This comment is out-of-date. I'm curious how it works now.
http://gerrit.cloudera.org:8080/#/c/19458/2/fe/src/main/java/org/apache/impala/service/Frontend.java@447
PS2, Line 447: new ThreadFactoryBuilder().setDaemon(true)
Marking as daemon threads seems to make sense for this. It does look like it's
a change in behavior we'll want to be aware of.
--
To view, visit http://gerrit.cloudera.org:8080/19458
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc
Gerrit-Change-Number: 19458
Gerrit-PatchSet: 2
Gerrit-Owner: Joe McDonnell
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Joe McDonnell
Gerrit-Reviewer: Michael Smith
Gerrit-Comment-Date: Tue, 31 Jan 2023 00:05:10 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Joe McDonnell has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19458 ) Change subject: IMPALA-11865: Set thread names for Java thread pools .. IMPALA-11865: Set thread names for Java thread pools Currently, some Java thread pools use the default thread names, which take the form "pool-N-thread-M". This sets the thread names to more human-friendly names by adding a ThreadFactory to Executor thread pools in our Java code. This should make the jstack output easier to understand. For example, "pool-8-thread-1" becomes "TableLoadingSubmitterThread-0". Testing: - Ran jstack locally and verified Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Reviewed-on: http://gerrit.cloudera.org:8080/19458 Reviewed-by: Michael Smith Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/metastore/CatalogMetastoreServer.java M fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java 5 files changed, 27 insertions(+), 9 deletions(-) Approvals: Michael Smith: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 2 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Joe McDonnell Gerrit-Reviewer: Michael Smith
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19458 ) Change subject: IMPALA-11865: Set thread names for Java thread pools .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 30 Jan 2023 23:58:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19458 ) Change subject: IMPALA-11865: Set thread names for Java thread pools .. Patch Set 1: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9000/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 30 Jan 2023 18:44:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Michael Smith has posted comments on this change. ( http://gerrit.cloudera.org:8080/19458 ) Change subject: IMPALA-11865: Set thread names for Java thread pools .. Patch Set 1: Code-Review+2 That'll be nice for debugging. -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Michael Smith Gerrit-Comment-Date: Mon, 30 Jan 2023 18:23:24 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19458 ) Change subject: IMPALA-11865: Set thread names for Java thread pools .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/12265/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Mon, 30 Jan 2023 18:11:26 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11865: Set thread names for Java thread pools
Joe McDonnell has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19458 Change subject: IMPALA-11865: Set thread names for Java thread pools .. IMPALA-11865: Set thread names for Java thread pools Currently, some Java thread pools use the default thread names, which take the form "pool-N-thread-M". This sets the thread names to more human-friendly names by adding a ThreadFactory to Executor thread pools in our Java code. This should make the jstack output easier to understand. For example, "pool-8-thread-1" becomes "TableLoadingSubmitterThread-0". Testing: - Ran jstack locally and verified Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc --- M fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java M fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java M fe/src/main/java/org/apache/impala/catalog/metastore/CatalogMetastoreServer.java M fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java 5 files changed, 27 insertions(+), 9 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/58/19458/1 -- To view, visit http://gerrit.cloudera.org:8080/19458 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I583093eeb2e92a208ee5ea5a023686e36436c4dc Gerrit-Change-Number: 19458 Gerrit-PatchSet: 1 Gerrit-Owner: Joe McDonnell
