Ensure all thread pools created by Helidon are named (master) (helido…

…n-io#3794) * Ensure thread pools are always named. Deprecated a couple of methods to enforce naming. Improved default naming when only a thread name prefix is provided. Signed-off-by: Santiago Pericasgeertsen <[email protected]> * Display logging message only if thread name prefix is not specified. Signed-off-by: Santiago Pericasgeertsen <[email protected]>
fryp · Jan 12, 2022 · 2f1af59 · 2f1af59
1 parent b20263f
commit 2f1af59
Show file tree

Hide file tree

Showing 9 changed files with 56 additions and 24 deletions.
diff --git a/common/configurable/src/main/java/io/helidon/common/configurable/ThreadPoolSupplier.java b/common/configurable/src/main/java/io/helidon/common/configurable/ThreadPoolSupplier.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -44,7 +44,6 @@ public final class ThreadPoolSupplier implements Supplier<ExecutorService> {
     private static final boolean DEFAULT_IS_DAEMON = true;
     private static final String DEFAULT_THREAD_NAME_PREFIX = "helidon-";
     private static final boolean DEFAULT_PRESTART = true;
-    private static final String DEFAULT_POOL_NAME_PREFIX = "helidon-thread-pool-";
     private static final int DEFAULT_GROWTH_RATE = 0; // Maintain JDK pool behavior when max > core
     private static final int DEFAULT_GROWTH_THRESHOLD = 1000;
 
@@ -70,7 +69,7 @@ private ThreadPoolSupplier(Builder builder) {
         this.isDaemon = builder.isDaemon;
         this.threadNamePrefix = builder.threadNamePrefix;
         this.prestart = builder.prestart;
-        this.name = builder.name == null ? DEFAULT_POOL_NAME_PREFIX + DEFAULT_NAME_COUNTER.incrementAndGet() : builder.name;
+        this.name = builder.name;
         this.growthThreshold = builder.growthThreshold;
         this.growthRate = builder.growthRate;
         this.rejectionHandler = builder.rejectionHandler == null ? DEFAULT_REJECTION_POLICY : builder.rejectionHandler;
@@ -91,21 +90,49 @@ public static Builder builder() {
      *
      * @param config config instance
      * @return a new thread pool supplier configured from config
+     * @deprecated since 2.4.2, please use {@link #create(Config, String)}
      */
+    @Deprecated
     public static ThreadPoolSupplier create(Config config) {
         return builder().config(config)
                 .build();
     }
 
+    /**
+     * Load supplier from configuration.
+     *
+     * @param config config instance
+     * @param name thread pool name
+     * @return a new thread pool supplier configured from config
+     */
+    public static ThreadPoolSupplier create(Config config, String name) {
+        return builder().name(name)
+                .config(config)
+                .build();
+    }
+
     /**
      * Create a new thread pool supplier with default configuration.
      *
      * @return a new thread pool supplier with default configuration
+     * @deprecated since 2.4.2, please use {@link #create(String)}
      */
+    @Deprecated
     public static ThreadPoolSupplier create() {
         return builder().build();
     }
 
+    /**
+     * Create a new thread pool supplier with default configuration and
+     * a given name.
+     *
+     * @param name thread pool name
+     * @return a new thread pool supplier with default configuration
+     */
+    public static ThreadPoolSupplier create(String name) {
+        return builder().name(name).build();
+    }
+
     ExecutorService getThreadPool() {
         if (useVirtualThreads) {
             if (VirtualExecutorUtil.isVirtualSupported()) {
@@ -170,7 +197,10 @@ private Builder() {
         @Override
         public ThreadPoolSupplier build() {
             if (name == null) {
-                name = DEFAULT_POOL_NAME_PREFIX + DEFAULT_NAME_COUNTER.incrementAndGet();
+                if (DEFAULT_THREAD_NAME_PREFIX.equals(threadNamePrefix)) {
+                    LOGGER.warning("Neither a thread name prefix nor a pool name specified");
+                }
+                name = threadNamePrefix + "thread-pool-" + DEFAULT_NAME_COUNTER.incrementAndGet();
             }
             if (rejectionHandler == null) {
                 rejectionHandler = DEFAULT_REJECTION_POLICY;

diff --git a/common/configurable/src/test/java/io/helidon/common/configurable/ThreadPoolSupplierTest.java b/common/configurable/src/test/java/io/helidon/common/configurable/ThreadPoolSupplierTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -51,7 +51,7 @@ class ThreadPoolSupplierTest {
 
     @BeforeAll
     static void initClass() {
-        defaultInstance = ensureOurExecutor(ThreadPoolSupplier.create().getThreadPool());
+        defaultInstance = ensureOurExecutor(ThreadPoolSupplier.create("test-thread-pool").getThreadPool());
 
         builtInstance = ensureOurExecutor(ThreadPoolSupplier.builder()
                                           .threadNamePrefix("thread-pool-unit-test-")
@@ -128,7 +128,7 @@ public void close() throws SecurityException {
         try {
             log.addHandler(handler);
             Config config = Config.create(ConfigSources.create(Map.of(thresholdKey, threshold, rateKey, rate)));
-            ExecutorService executor = ThreadPoolSupplier.create(config).get();
+            ExecutorService executor = ThreadPoolSupplier.create(config, "test-thread-pool").get();
             Optional<ThreadPool> asThreadPool = ThreadPool.asThreadPool(executor);
             ThreadPool pool = asThreadPool.orElseThrow(() -> new RuntimeException("not a thread pool"));
             assertThat(pool.getGrowthThreshold(), is(Integer.parseInt(threshold)));

diff --git a/dbclient/jdbc/src/main/java/io/helidon/dbclient/jdbc/JdbcDbClientProviderBuilder.java b/dbclient/jdbc/src/main/java/io/helidon/dbclient/jdbc/JdbcDbClientProviderBuilder.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2020 Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -85,7 +85,7 @@ public DbClient build() {
             this.mapperManager = MapperManager.create();
         }
         if (null == executorService) {
-            executorService = ThreadPoolSupplier.create();
+            executorService = ThreadPoolSupplier.create("jdbc-dbclient-thread-pool");
         }
         return new JdbcDbClient(this);
     }
@@ -97,7 +97,9 @@ public JdbcDbClientProviderBuilder config(Config config) {
                 .ifExists(cfg -> connectionPool(ConnectionPool.create(cfg)));
 
         config.get("statements").as(DbStatements::create).ifPresent(this::statements);
-        config.get("executor-service").as(ThreadPoolSupplier::create).ifPresent(this::executorService);
+        config.get("executor-service")
+                .as(c -> ThreadPoolSupplier.create(c, "jdbc-dbclient-thread-pool"))
+                .ifPresent(this::executorService);
         return this;
     }
 

diff --git a/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileService.java b/examples/media/multipart/src/main/java/io/helidon/examples/media/multipart/FileService.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -45,7 +45,7 @@ public final class FileService implements Service {
 
     private static final JsonBuilderFactory JSON_FACTORY = Json.createBuilderFactory(Map.of());
     private final FileStorage storage;
-    private final ExecutorService executor = ThreadPoolSupplier.create().get();
+    private final ExecutorService executor = ThreadPoolSupplier.create("multipart-thread-pool").get();
 
 
     /**

diff --git a/fault-tolerance/src/main/java/io/helidon/faulttolerance/FaultTolerance.java b/fault-tolerance/src/main/java/io/helidon/faulttolerance/FaultTolerance.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2020, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -84,7 +84,7 @@ public static void config(Config config) {
         CONFIG.set(config);
 
         SCHEDULED_EXECUTOR.set(LazyValue.create(ScheduledThreadPoolSupplier.create(CONFIG.get().get("scheduled-executor"))));
-        EXECUTOR.set(LazyValue.create(ThreadPoolSupplier.create(CONFIG.get().get("executor"))));
+        EXECUTOR.set(LazyValue.create(ThreadPoolSupplier.create(CONFIG.get().get("executor"), "ft-se-thread-pool")));
     }
 
     /**

diff --git a/...ofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingClientRegistrar.java b/...ofile/tracing/src/main/java/io/helidon/microprofile/tracing/MpTracingClientRegistrar.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019 Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -35,7 +35,7 @@ public class MpTracingClientRegistrar implements ClientTracingRegistrarProvider
 
     static {
         Config config = (Config) ConfigProvider.getConfig();
-        EXECUTOR_SERVICE = ThreadPoolSupplier.create(config.get("tracing.executor-service"));
+        EXECUTOR_SERVICE = ThreadPoolSupplier.create(config.get("tracing.executor-service"), "mp-tracing-thread-pool");
     }
 
     @Override

diff --git a/security/security/src/main/java/io/helidon/security/Security.java b/security/security/src/main/java/io/helidon/security/Security.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2018, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -574,7 +574,7 @@ public static final class Builder implements io.helidon.common.Builder<Builder,
         private Tracer tracer;
         private boolean tracingEnabled = true;
         private SecurityTime serverTime = SecurityTime.builder().build();
-        private Supplier<ExecutorService> executorService = ThreadPoolSupplier.create();
+        private Supplier<ExecutorService> executorService = ThreadPoolSupplier.create("security-thread-pool");
         private boolean enabled = true;
 
         private Builder() {
@@ -1173,7 +1173,7 @@ private void fromConfig(Config config) {
             }
 
             config.get("environment.server-time").as(SecurityTime::create).ifPresent(this::serverTime);
-            executorService(ThreadPoolSupplier.create(config.get("environment.executor-service")));
+            executorService(ThreadPoolSupplier.create(config.get("environment.executor-service"), "security-thread-pool"));
 
             Map<String, SecurityProviderService> configKeyToService = new HashMap<>();
             Map<String, SecurityProviderService> classNameToService = new HashMap<>();

diff --git a/...bookstore/bookstore-se/src/test/java/io/helidon/tests/apps/bookstore/se/Http2SslTest.java b/...bookstore/bookstore-se/src/test/java/io/helidon/tests/apps/bookstore/se/Http2SslTest.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -87,7 +87,7 @@ public void testHelloWorldHttp2SslPostFirst() throws Exception {
 
     @Test
     public void testHelloWorldHttp2SslConcurrent() throws Exception {
-        ExecutorService executor = ThreadPoolSupplier.create().get();
+        ExecutorService executor = ThreadPoolSupplier.create("test-thread-pool").get();
         Request.Builder builder = TestServer.newRequestBuilder(webServer, "/books", true);
         Request getBooks = builder.build();
 

diff --git a/webclient/jaxrs/src/main/java/io/helidon/webclient/jaxrs/JaxRsClient.java b/webclient/jaxrs/src/main/java/io/helidon/webclient/jaxrs/JaxRsClient.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2019, 2021 Oracle and/or its affiliates.
+ * Copyright (c) 2019, 2022 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -49,7 +49,7 @@ private JaxRsClient() {
      * </tr>
      * <tr>
      *     <td>executor</td>
-     *     <td>{@link io.helidon.common.configurable.ThreadPoolSupplier#create(io.helidon.config.Config)}</td>
+     *     <td>{@link io.helidon.common.configurable.ThreadPoolSupplier#create(io.helidon.config.Config, String)}</td>
      *     <td>Default executor service to use for asynchronous operations. For configuration options
      *      of {@code executor}, please refer to
      *      {@link io.helidon.common.configurable.ThreadPoolSupplier.Builder#config(io.helidon.config.Config)}</td>
@@ -59,7 +59,7 @@ private JaxRsClient() {
      * @param config configuration to use to configure JAX-RS clients defaults
      */
     public static void configureDefaults(Config config) {
-        EXECUTOR_SUPPLIER.set(ThreadPoolSupplier.create(config));
+        EXECUTOR_SUPPLIER.set(ThreadPoolSupplier.create(config, "jaxrs-client-thread-pool"));
     }
 
     /**