[impala] 03/03: Reapply IMPALA-11865: Set thread names for Java thread pools

[jo...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit fd4db07e7592446fdc23b601da0f5cfdc0d9f9ec
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Mon Jan 30 09:43:34 2023 -0800

    Reapply 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: Iaa3028666ff9e51bc0722d3702060bc404bb3da3
    Reviewed-on: http://gerrit.cloudera.org:8080/19642
    Reviewed-by: Michael Smith <mi...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 .../java/org/apache/impala/catalog/CatalogServiceCatalog.java |  4 +++-
 .../main/java/org/apache/impala/catalog/TableLoadingMgr.java  | 11 ++++++++---
 .../impala/catalog/metastore/CatalogMetastoreServer.java      |  4 +++-
 .../java/org/apache/impala/hooks/QueryEventHookManager.java   |  5 ++++-
 fe/src/main/java/org/apache/impala/service/Frontend.java      | 11 ++++++-----
 5 files changed, 24 insertions(+), 11 deletions(-)

diff --git a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
index 53f423f4c..96465f69a 100644
--- a/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
+++ b/fe/src/main/java/org/apache/impala/catalog/CatalogServiceCatalog.java
@@ -138,6 +138,7 @@ import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
 import com.google.common.base.Stopwatch;
 import com.google.common.collect.Maps;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
 /**
  * Specialized Catalog that implements the CatalogService specific Catalog
@@ -265,7 +266,8 @@ public class CatalogServiceCatalog extends Catalog {
 
   // Periodically polls HDFS to get the latest set of known cache pools.
   private final ScheduledExecutorService cachePoolReader_ =
-      Executors.newScheduledThreadPool(1);
+    Executors.newScheduledThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat("HDFSCachePoolReader").build());
 
   // Log of deleted catalog objects.
   private final CatalogDeltaLog deleteLog_;
diff --git a/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java b/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
index 37685c6ff..48720d0bb 100644
--- a/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
+++ b/fe/src/main/java/org/apache/impala/catalog/TableLoadingMgr.java
@@ -36,6 +36,7 @@ import org.apache.impala.util.HdfsCachingUtil;
 import org.apache.log4j.Logger;
 
 import com.google.common.base.Preconditions;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
 /**
 * Class that manages scheduling the loading of table metadata from the Hive Metastore and
@@ -148,7 +149,8 @@ public class TableLoadingMgr {
   // (no work will be rejected, but memory consumption is unbounded). If this thread
   // dies it will be automatically restarted.
   // The tables to process are read from the resfreshThreadWork_ queue.
-  ExecutorService asyncRefreshThread_ = Executors.newSingleThreadExecutor();
+  ExecutorService asyncRefreshThread_ = Executors.newSingleThreadExecutor(
+      new ThreadFactoryBuilder().setNameFormat("TableAsyncRefreshThread").build());
 
   // Tables for the async refresh thread to process. Synchronization must be handled
   // externally.
@@ -162,7 +164,8 @@ public class TableLoadingMgr {
     catalog_ = catalog;
     tblLoader_ = new TableLoader(catalog_);
     numLoadingThreads_ = numLoadingThreads;
-    tblLoadingPool_ = Executors.newFixedThreadPool(numLoadingThreads_);
+    tblLoadingPool_ = Executors.newFixedThreadPool(numLoadingThreads_,
+        new ThreadFactoryBuilder().setNameFormat("TableLoadingThread-%d").build());
 
     // Start the background table loading submitter threads.
     startTableLoadingSubmitterThreads();
@@ -265,7 +268,9 @@ public class TableLoadingMgr {
    */
   private void startTableLoadingSubmitterThreads() {
     ExecutorService submitterLoadingPool =
-        Executors.newFixedThreadPool(numLoadingThreads_);
+      Executors.newFixedThreadPool(numLoadingThreads_,
+          new ThreadFactoryBuilder()
+              .setNameFormat("TableLoadingSubmitterThread-%d").build());
     try {
       for (int i = 0; i < numLoadingThreads_; ++i) {
         submitterLoadingPool.execute(new Runnable() {
diff --git a/fe/src/main/java/org/apache/impala/catalog/metastore/CatalogMetastoreServer.java b/fe/src/main/java/org/apache/impala/catalog/metastore/CatalogMetastoreServer.java
index 14a0f6076..ea7ab2a2f 100644
--- a/fe/src/main/java/org/apache/impala/catalog/metastore/CatalogMetastoreServer.java
+++ b/fe/src/main/java/org/apache/impala/catalog/metastore/CatalogMetastoreServer.java
@@ -28,6 +28,7 @@ import java.util.Set;
 import com.codahale.metrics.Timer;
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 
 import java.lang.reflect.InvocationHandler;
 import java.lang.reflect.InvocationTargetException;
@@ -109,7 +110,8 @@ public class CatalogMetastoreServer extends ThriftHiveMetastore implements
 
   // Logs Catalogd HMS cache metrics at a fixed frequency.
   private final ScheduledExecutorService metricsLoggerService_ =
-      Executors.newScheduledThreadPool(1);
+    Executors.newScheduledThreadPool(1,
+        new ThreadFactoryBuilder().setNameFormat("MetricsLoggerService").build());
 
   // the server is started in a daemon thread so that instantiating this is not
   // a blocking call.
diff --git a/fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java b/fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java
index eef24673b..b1e4d702d 100644
--- a/fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java
+++ b/fe/src/main/java/org/apache/impala/hooks/QueryEventHookManager.java
@@ -31,6 +31,8 @@ import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 import java.util.stream.Collectors;
 
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
+
 /**
  * {@link QueryEventHookManager} manages the registration and execution of
  * {@link QueryEventHook}s. Each manager instance may manage its own hooks,
@@ -117,7 +119,8 @@ public class QueryEventHookManager {
   private QueryEventHookManager(int nHookExecutorThreads, String[] hookClasses)
       throws InternalException {
 
-    this.hookExecutor_ = Executors.newFixedThreadPool(nHookExecutorThreads);
+    this.hookExecutor_ = Executors.newFixedThreadPool(nHookExecutorThreads,
+        new ThreadFactoryBuilder().setNameFormat("QueryEventHookExecutor-%d").build());
     Runtime.getRuntime().addShutdownHook(new Thread(() -> this.cleanUp()));
 
     final List<QueryEventHook> hooks = new ArrayList<>(hookClasses.length);
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index c9da2bdea..cde002df3 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -28,6 +28,7 @@ import com.google.common.collect.Sets;
 import com.google.common.math.IntMath;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.Uninterruptibles;
+import com.google.common.util.concurrent.ThreadFactoryBuilder;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -400,13 +401,11 @@ public class Frontend {
   // Privileges in which the user should have any of them to see a database or table,
   private final EnumSet<Privilege> minPrivilegeSetForShowStmts_;
   /**
-   * Authorization checker. Initialized and periodically loaded by a task
-   * running on the {@link #policyReader_} thread.
+   * Authorization checker. Initialized on creation, then it is kept up to date
+   * via calls to updateCatalogCache().
    */
   private final AtomicReference<AuthorizationChecker> authzChecker_ =
       new AtomicReference<>();
-  private final ScheduledExecutorService policyReader_ =
-      Executors.newScheduledThreadPool(1);
 
   private final ImpaladTableUsageTracker impaladTableUsageTracker_;
 
@@ -455,7 +454,9 @@ public class Frontend {
         checkAuthorizationPool_ = MoreExecutors.newDirectExecutorService();
       } else {
         LOG.info("Using a thread pool of size {} for authorization", numThreads);
-        checkAuthorizationPool_ = Executors.newFixedThreadPool(numThreads);
+        checkAuthorizationPool_ = Executors.newFixedThreadPool(numThreads,
+            new ThreadFactoryBuilder()
+                .setNameFormat("AuthorizationCheckerThread-%d").build());
       }
     } else {
       authzChecker_.set(authzFactory.newAuthorizationChecker());