Use separate non-public config definition for ContainerThreadPool

4 files changed, 60 insertions, 19 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java b/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java
index 425387039ff..dc594903f21 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/ThreadPoolProvider.java
@@ -5,6 +5,7 @@ import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.container.di.componentgraph.Provider;
 import com.yahoo.container.handler.threadpool.ContainerThreadPool;
+import com.yahoo.container.handler.threadpool.ContainerThreadpoolConfig;
 import com.yahoo.container.protect.ProcessTerminator;
 import com.yahoo.jdisc.Metric;
 
@@ -23,12 +24,27 @@ public class ThreadPoolProvider extends AbstractComponent implements Provider<Ex
     private final ContainerThreadPool threadpool;
 
     @Inject
-    public ThreadPoolProvider(ThreadpoolConfig threadpoolConfig, Metric metric) {
-        this.threadpool = new ContainerThreadPool(threadpoolConfig, metric);
+    public ThreadPoolProvider(ThreadpoolConfig config, Metric metric) {
+        this.threadpool = new ContainerThreadPool(translateConfig(config), metric);
     }
 
-    public ThreadPoolProvider(ThreadpoolConfig threadpoolConfig, Metric metric, ProcessTerminator processTerminator) {
-        this.threadpool = new ContainerThreadPool(threadpoolConfig, metric, processTerminator);
+    public ThreadPoolProvider(ThreadpoolConfig config, Metric metric, ProcessTerminator processTerminator) {
+        this.threadpool = new ContainerThreadPool(translateConfig(config), metric, processTerminator);
+    }
+
+    /**
+     * The underlying {@link ContainerThreadPool} uses a different config definition ({@link ContainerThreadpoolConfig})
+     * as {@link ThreadpoolConfig} is currently public api.
+     */
+    private static ContainerThreadpoolConfig translateConfig(ThreadpoolConfig config) {
+        return new ContainerThreadpoolConfig(
+                new ContainerThreadpoolConfig.Builder()
+                        .maxThreads(config.maxthreads())
+                        .minThreads(config.corePoolSize())
+                        .name(config.name())
+                        .queueSize(config.queueSize())
+                        .keepAliveTime(config.keepAliveTime())
+                        .maxThreadExecutionTimeSeconds(config.maxThreadExecutionTimeSeconds()));
     }
 
     /**
diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadPool.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadPool.java
index 74bcb3a2e04..e2d38427de1 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadPool.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ContainerThreadPool.java
@@ -4,7 +4,6 @@ package com.yahoo.container.handler.threadpool;
 import com.google.inject.Inject;
 import com.yahoo.component.AbstractComponent;
 import com.yahoo.concurrent.ThreadFactoryFactory;
-import com.yahoo.container.handler.ThreadpoolConfig;
 import com.yahoo.container.protect.ProcessTerminator;
 import com.yahoo.jdisc.Metric;
 
@@ -27,26 +26,26 @@ public class ContainerThreadPool extends AbstractComponent implements AutoClosea
     private final ExecutorServiceWrapper threadpool;
 
     @Inject
-    public ContainerThreadPool(ThreadpoolConfig config, Metric metric) {
+    public ContainerThreadPool(ContainerThreadpoolConfig config, Metric metric) {
         this(config, metric, new ProcessTerminator());
     }
 
-    public ContainerThreadPool(ThreadpoolConfig threadpoolConfig, Metric metric, ProcessTerminator processTerminator) {
-        ThreadPoolMetric threadPoolMetric = new ThreadPoolMetric(metric, threadpoolConfig.name());
-        int maxNumThreads = computeMaximumThreadPoolSize(threadpoolConfig.maxthreads());
-        int coreNumThreads = computeCoreThreadPoolSize(threadpoolConfig.corePoolSize(), maxNumThreads);
+    public ContainerThreadPool(ContainerThreadpoolConfig config, Metric metric, ProcessTerminator processTerminator) {
+        ThreadPoolMetric threadPoolMetric = new ThreadPoolMetric(metric, config.name());
+        int maxNumThreads = computeMaximumThreadPoolSize(config.maxThreads());
+        int coreNumThreads = computeCoreThreadPoolSize(config.minThreads(), maxNumThreads);
         WorkerCompletionTimingThreadPoolExecutor executor =
                 new WorkerCompletionTimingThreadPoolExecutor(coreNumThreads, maxNumThreads,
-                        (int)threadpoolConfig.keepAliveTime() * 1000, TimeUnit.MILLISECONDS,
-                        createQ(threadpoolConfig.queueSize(), maxNumThreads),
-                        ThreadFactoryFactory.getThreadFactory(threadpoolConfig.name()),
+                        (int)config.keepAliveTime() * 1000, TimeUnit.MILLISECONDS,
+                        createQ(config.queueSize(), maxNumThreads),
+                        ThreadFactoryFactory.getThreadFactory(config.name()),
                         threadPoolMetric);
         // Prestart needed, if not all threads will be created by the fist N tasks and hence they might also
         // get the dreaded thread locals initialized even if they will never run.
         // That counters what we we want to achieve with the Q that will prefer thread locality.
         executor.prestartAllCoreThreads();
         threadpool = new ExecutorServiceWrapper(executor, threadPoolMetric, processTerminator,
-                threadpoolConfig.maxThreadExecutionTimeSeconds() * 1000L);
+                config.maxThreadExecutionTimeSeconds() * 1000L);
     }
 
     public Executor executor() { return threadpool; }
diff --git a/container-core/src/main/resources/configdefinitions/container-threadpool.def b/container-core/src/main/resources/configdefinitions/container-threadpool.def
new file mode 100644
index 00000000000..9248bf2e2bf
--- /dev/null
+++ b/container-core/src/main/resources/configdefinitions/container-threadpool.def
@@ -0,0 +1,25 @@
+# Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+namespace=container.handler.threadpool
+
+## Maximum number of thread in the thread pool
+## 0 is translated to vcpu*4
+maxThreads int default=0
+
+## Minimum number of thread in the thread pool
+## 0 is translated to vcpu*2
+minThreads int default=0
+
+## The number of seconds that excess idle threads will wait for new tasks before terminating
+keepAliveTime double default=5.0
+
+## Max queue size
+queueSize int default=0
+
+## The max time the container tolerates having no threads available before it shuts down to
+## get out of a bad state. This should be set a bit higher than the expected max execution
+## time of each request when in a state of overload, i.e about "worst case execution time*2"
+maxThreadExecutionTimeSeconds int default=190
+
+# Prefix for the name of the threads
+name string default="default-pool"
diff --git a/container-core/src/test/java/com/yahoo/container/handler/threadpool/ContainerThreadPoolTest.java b/container-core/src/test/java/com/yahoo/container/handler/threadpool/ContainerThreadPoolTest.java
index f6a3aebd7ff..02e791099ed 100644
--- a/container-core/src/test/java/com/yahoo/container/handler/threadpool/ContainerThreadPoolTest.java
+++ b/container-core/src/test/java/com/yahoo/container/handler/threadpool/ContainerThreadPoolTest.java
@@ -3,7 +3,6 @@ package com.yahoo.container.handler.threadpool;
 
 import com.yahoo.collections.Tuple2;
 import com.yahoo.concurrent.Receiver;
-import com.yahoo.container.handler.ThreadpoolConfig;
 import com.yahoo.container.protect.ProcessTerminator;
 import com.yahoo.jdisc.Metric;
 import org.junit.Ignore;
@@ -24,7 +23,7 @@ import static org.junit.Assert.fail;
 public class ContainerThreadPoolTest {
     @Test
     public final void testThreadPool() throws InterruptedException {
-        ThreadpoolConfig config = new ThreadpoolConfig(new ThreadpoolConfig.Builder().maxthreads(1));
+        ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder().maxThreads(1));
         ContainerThreadPool threadPool = new ContainerThreadPool(config, Mockito.mock(Metric.class));
         Executor exec = threadPool.executor();
         Tuple2<Receiver.MessageState, Boolean> reply;
@@ -56,7 +55,7 @@ public class ContainerThreadPoolTest {
     }
 
     private ThreadPoolExecutor createPool(int maxThreads, int queueSize) {
-        ThreadpoolConfig config = new ThreadpoolConfig(new ThreadpoolConfig.Builder().maxthreads(maxThreads).queueSize(queueSize));
+        ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder().maxThreads(maxThreads).queueSize(queueSize));
         ContainerThreadPool threadPool = new ContainerThreadPool(config, Mockito.mock(Metric.class));
         ExecutorServiceWrapper wrapper = (ExecutorServiceWrapper) threadPool.executor();
         WorkerCompletionTimingThreadPoolExecutor executor = (WorkerCompletionTimingThreadPoolExecutor)wrapper.delegate();
@@ -100,8 +99,10 @@ public class ContainerThreadPoolTest {
     @Test
     @Ignore // Ignored because it depends on the system time and so is unstable on factory
     public void testThreadPoolTerminationOnBreakdown() throws InterruptedException {
-        ThreadpoolConfig config = new ThreadpoolConfig(new ThreadpoolConfig.Builder().maxthreads(2)
-                .maxThreadExecutionTimeSeconds(1));
+        ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(
+                new ContainerThreadpoolConfig.Builder()
+                        .maxThreads(2)
+                        .maxThreadExecutionTimeSeconds(1));
         MockProcessTerminator terminator = new MockProcessTerminator();
         ContainerThreadPool threadPool = new ContainerThreadPool(config, Mockito.mock(Metric.class), terminator);