- Effective executor Q size is max pool size + max Q size.

- Also use timed wait/notify over sleep to reduce shutdown time.
- Join metric thread to ensure no latent issues.

2 files changed, 27 insertions, 14 deletions

diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java
index 8d07e7c3757..638336e51d8 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadpool.java
@@ -60,7 +60,7 @@ public class DefaultContainerThreadpool extends AbstractComponent implements Aut
         executor.prestartAllCoreThreads();
         threadpool = new ExecutorServiceWrapper(
                 executor, threadPoolMetric, processTerminator, config.maxThreadExecutionTimeSeconds() * 1000L,
-                name, queueSize);
+                name);
     }
 
     @Override public Executor executor() { return threadpool; }
diff --git a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java
index 52936f7bbef..5ba5985db37 100644
--- a/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java
+++ b/container-core/src/main/java/com/yahoo/container/handler/threadpool/ExecutorServiceWrapper.java
@@ -32,38 +32,51 @@ class ExecutorServiceWrapper extends ForwardingExecutorService {
                            ThreadPoolMetric metric,
                            ProcessTerminator processTerminator,
                            long maxThreadExecutionTimeMillis,
-                           String name,
-                           int queueCapacity) {
+                           String name) {
         this.wrapped = wrapped;
         this.metric = metric;
         this.processTerminator = processTerminator;
         this.maxThreadExecutionTimeMillis = maxThreadExecutionTimeMillis;
-        this.queueCapacity = queueCapacity;
+        this.queueCapacity = wrapped.getMaximumPoolSize() + wrapped.getQueue().remainingCapacity() + wrapped.getQueue().size();
 
         metric.reportThreadPoolSize(wrapped.getPoolSize());
         metric.reportActiveThreads(wrapped.getActiveCount());
         metricReporter = new Thread(this::reportMetrics);
         metricReporter.setName(name + "-threadpool-metric-reporter");
-        metricReporter.setDaemon(true);
         metricReporter.start();
     }
 
     private void reportMetrics() {
-        try {
-            while (!closed.get()) {
-                metric.reportThreadPoolSize(wrapped.getPoolSize());
-                metric.reportActiveThreads(wrapped.getActiveCount());
-                metric.reportWorkQueueSize(wrapped.getQueue().size());
-                metric.reportWorkQueueCapacity(queueCapacity);
-                Thread.sleep(100);
+        while (timeToReportMetricsAgain(100)) {
+            metric.reportThreadPoolSize(wrapped.getPoolSize());
+            metric.reportActiveThreads(wrapped.getActiveCount());
+            metric.reportWorkQueueSize(wrapped.getQueue().size());
+            metric.reportWorkQueueCapacity(queueCapacity);
+        }
+    }
+    private boolean timeToReportMetricsAgain(int timeoutMS) {
+        synchronized (closed) {
+            if (!closed.get()) {
+                try {
+                    closed.wait(timeoutMS);
+                } catch (InterruptedException e) {
+                    return false;
+                }
             }
-        } catch (InterruptedException e) { }
+        }
+        return !closed.get();
     }
 
     @Override
     public void shutdown() {
         super.shutdown();
-        closed.set(true);
+        synchronized (closed) {
+            closed.set(true);
+            closed.notify();
+        }
+        try {
+            metricReporter.join();
+        } catch (InterruptedException e) {}
     }
 
     /**