Revert "Use separate non-public config definition for ContainerThreadPool"

4 files changed, 19 insertions, 60 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 dc594903f21..425387039ff 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,7 +5,6 @@ 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;
 
@@ -24,27 +23,12 @@ public class ThreadPoolProvider extends AbstractComponent implements Provider<Ex
     private final ContainerThreadPool threadpool;
 
     @Inject
-    public ThreadPoolProvider(ThreadpoolConfig config, Metric metric) {
-        this.threadpool = new ContainerThreadPool(translateConfig(config), metric);
+    public ThreadPoolProvider(ThreadpoolConfig threadpoolConfig, Metric metric) {
+        this.threadpool = new ContainerThreadPool(threadpoolConfig, metric);
     }
 
-    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()));
+    public ThreadPoolProvider(ThreadpoolConfig threadpoolConfig, Metric metric, ProcessTerminator processTerminator) {
+        this.threadpool = new ContainerThreadPool(threadpoolConfig, metric, processTerminator);
     }
 
     /**
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 e2d38427de1..74bcb3a2e04 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,6 +4,7 @@ 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;
 
@@ -26,26 +27,26 @@ public class ContainerThreadPool extends AbstractComponent implements AutoClosea
     private final ExecutorServiceWrapper threadpool;
 
     @Inject
-    public ContainerThreadPool(ContainerThreadpoolConfig config, Metric metric) {
+    public ContainerThreadPool(ThreadpoolConfig config, Metric metric) {
         this(config, metric, new ProcessTerminator());
     }
 
-    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);
+    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);
         WorkerCompletionTimingThreadPoolExecutor executor =
                 new WorkerCompletionTimingThreadPoolExecutor(coreNumThreads, maxNumThreads,
-                        (int)config.keepAliveTime() * 1000, TimeUnit.MILLISECONDS,
-                        createQ(config.queueSize(), maxNumThreads),
-                        ThreadFactoryFactory.getThreadFactory(config.name()),
+                        (int)threadpoolConfig.keepAliveTime() * 1000, TimeUnit.MILLISECONDS,
+                        createQ(threadpoolConfig.queueSize(), maxNumThreads),
+                        ThreadFactoryFactory.getThreadFactory(threadpoolConfig.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,
-                config.maxThreadExecutionTimeSeconds() * 1000L);
+                threadpoolConfig.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
deleted file mode 100644
index 9248bf2e2bf..00000000000
--- a/container-core/src/main/resources/configdefinitions/container-threadpool.def
+++ /dev/null
@@ -1,25 +0,0 @@
-# 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 02e791099ed..f6a3aebd7ff 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,6 +3,7 @@ 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;
@@ -23,7 +24,7 @@ import static org.junit.Assert.fail;
 public class ContainerThreadPoolTest {
     @Test
     public final void testThreadPool() throws InterruptedException {
-        ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder().maxThreads(1));
+        ThreadpoolConfig config = new ThreadpoolConfig(new ThreadpoolConfig.Builder().maxthreads(1));
         ContainerThreadPool threadPool = new ContainerThreadPool(config, Mockito.mock(Metric.class));
         Executor exec = threadPool.executor();
         Tuple2<Receiver.MessageState, Boolean> reply;
@@ -55,7 +56,7 @@ public class ContainerThreadPoolTest {
     }
 
     private ThreadPoolExecutor createPool(int maxThreads, int queueSize) {
-        ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder().maxThreads(maxThreads).queueSize(queueSize));
+        ThreadpoolConfig config = new ThreadpoolConfig(new ThreadpoolConfig.Builder().maxthreads(maxThreads).queueSize(queueSize));
         ContainerThreadPool threadPool = new ContainerThreadPool(config, Mockito.mock(Metric.class));
         ExecutorServiceWrapper wrapper = (ExecutorServiceWrapper) threadPool.executor();
         WorkerCompletionTimingThreadPoolExecutor executor = (WorkerCompletionTimingThreadPoolExecutor)wrapper.delegate();
@@ -99,10 +100,8 @@ public class ContainerThreadPoolTest {
     @Test
     @Ignore // Ignored because it depends on the system time and so is unstable on factory
     public void testThreadPoolTerminationOnBreakdown() throws InterruptedException {
-        ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(
-                new ContainerThreadpoolConfig.Builder()
-                        .maxThreads(2)
-                        .maxThreadExecutionTimeSeconds(1));
+        ThreadpoolConfig config = new ThreadpoolConfig(new ThreadpoolConfig.Builder().maxthreads(2)
+                .maxThreadExecutionTimeSeconds(1));
         MockProcessTerminator terminator = new MockProcessTerminator();
         ContainerThreadPool threadPool = new ContainerThreadPool(config, Mockito.mock(Metric.class), terminator);