Metrics Q size and capacity must into account what kind of Q is used.

5 files changed, 97 insertions, 35 deletions

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 5ba5985db37..99f49f10526 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
@@ -26,6 +26,7 @@ class ExecutorServiceWrapper extends ForwardingExecutorService {
     private final long maxThreadExecutionTimeMillis;
     private final int queueCapacity;
     private final Thread metricReporter;
+    private final boolean threadPoolIsOnlyQ;
     private final AtomicBoolean closed = new AtomicBoolean(false);
 
     ExecutorServiceWrapper(WorkerCompletionTimingThreadPoolExecutor wrapped,
@@ -37,21 +38,32 @@ class ExecutorServiceWrapper extends ForwardingExecutorService {
         this.metric = metric;
         this.processTerminator = processTerminator;
         this.maxThreadExecutionTimeMillis = maxThreadExecutionTimeMillis;
-        this.queueCapacity = wrapped.getMaximumPoolSize() + wrapped.getQueue().remainingCapacity() + wrapped.getQueue().size();
+        int maxQueueCapacity = wrapped.getQueue().remainingCapacity() + wrapped.getQueue().size();
+        this.threadPoolIsOnlyQ = (maxQueueCapacity == 0);
+        this.queueCapacity = threadPoolIsOnlyQ
+                ? wrapped.getMaximumPoolSize()
+                : maxQueueCapacity;
 
         metric.reportThreadPoolSize(wrapped.getPoolSize());
         metric.reportActiveThreads(wrapped.getActiveCount());
-        metricReporter = new Thread(this::reportMetrics);
+        reportMetrics();
+        metricReporter = new Thread(this::reportMetricsRegularly);
         metricReporter.setName(name + "-threadpool-metric-reporter");
         metricReporter.start();
     }
 
     private void reportMetrics() {
+        int activeThreads = wrapped.getActiveCount();
+        metric.reportThreadPoolSize(wrapped.getPoolSize());
+        metric.reportActiveThreads(activeThreads);
+        int queueSize = threadPoolIsOnlyQ ? activeThreads : wrapped.getQueue().size();
+        metric.reportWorkQueueSize(queueSize);
+        metric.reportWorkQueueCapacity(queueCapacity);
+    }
+
+    private void reportMetricsRegularly() {
         while (timeToReportMetricsAgain(100)) {
-            metric.reportThreadPoolSize(wrapped.getPoolSize());
-            metric.reportActiveThreads(wrapped.getActiveCount());
-            metric.reportWorkQueueSize(wrapped.getQueue().size());
-            metric.reportWorkQueueCapacity(queueCapacity);
+            reportMetrics();
         }
     }
     private boolean timeToReportMetricsAgain(int timeoutMS) {
diff --git a/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java b/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java
index 2233ea44b2e..b56d89cafb3 100644
--- a/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java
+++ b/container-core/src/test/java/com/yahoo/container/handler/threadpool/DefaultContainerThreadPoolTest.java
@@ -4,10 +4,10 @@ package com.yahoo.container.handler.threadpool;
 import com.yahoo.collections.Tuple2;
 import com.yahoo.concurrent.Receiver;
 import com.yahoo.container.protect.ProcessTerminator;
+import com.yahoo.container.test.MetricMock;
 import com.yahoo.jdisc.Metric;
 import org.junit.Ignore;
 import org.junit.Test;
-import org.mockito.Mockito;
 
 import java.util.concurrent.Executor;
 import java.util.concurrent.RejectedExecutionException;
@@ -26,8 +26,9 @@ public class DefaultContainerThreadPoolTest {
 
     @Test
     public final void testThreadPool() throws InterruptedException {
+        Metric metrics = new MetricMock();
         ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder().maxThreads(1));
-        ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, Mockito.mock(Metric.class));
+        ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, metrics);
         Executor exec = threadPool.executor();
         Tuple2<Receiver.MessageState, Boolean> reply;
         FlipIt command = new FlipIt();
@@ -58,12 +59,15 @@ public class DefaultContainerThreadPoolTest {
     }
 
     private ThreadPoolExecutor createPool(int maxThreads, int queueSize) {
+        return createPool(new MetricMock(), maxThreads, queueSize);
+    }
+    private ThreadPoolExecutor createPool(Metric metric, int maxThreads, int queueSize) {
         ContainerThreadpoolConfig config = new ContainerThreadpoolConfig(new ContainerThreadpoolConfig.Builder()
                 .maxThreads(maxThreads)
                 .minThreads(maxThreads)
                 .queueSize(queueSize));
         ContainerThreadPool threadPool = new DefaultContainerThreadpool(
-                config, Mockito.mock(Metric.class), new MockProcessTerminator(), CPUS);
+                config, metric, new MockProcessTerminator(), CPUS);
         ExecutorServiceWrapper wrapper = (ExecutorServiceWrapper) threadPool.executor();
         WorkerCompletionTimingThreadPoolExecutor executor = (WorkerCompletionTimingThreadPoolExecutor)wrapper.delegate();
         return executor;
@@ -71,15 +75,27 @@ public class DefaultContainerThreadPoolTest {
 
     @Test
     public void testThatThreadPoolSizeFollowsConfig() {
-        ThreadPoolExecutor executor = createPool(3, 1200);
+        MetricMock metrics = new MetricMock();
+        ThreadPoolExecutor executor = createPool(metrics, 3, 1200);
         assertEquals(3, executor.getMaximumPoolSize());
         assertEquals(1200, executor.getQueue().remainingCapacity());
+        assertEquals(4, metrics.innvocations().size());
+        assertEquals(3L, metrics.innvocations().get("serverThreadPoolSize").val);
+        assertEquals(0L, metrics.innvocations().get("serverActiveThreads").val);
+        assertEquals(1200L, metrics.innvocations().get("jdisc.thread_pool.work_queue.capacity").val);
+        assertEquals(0L, metrics.innvocations().get("jdisc.thread_pool.work_queue.size").val);
     }
     @Test
     public void testThatThreadPoolSizeAutoDetected() {
-        ThreadPoolExecutor executor = createPool(0, 0);
+        MetricMock metrics = new MetricMock();
+        ThreadPoolExecutor executor = createPool(metrics, 0, 0);
         assertEquals(CPUS*4, executor.getMaximumPoolSize());
         assertEquals(0, executor.getQueue().remainingCapacity());
+        assertEquals(4, metrics.innvocations().size());
+        assertEquals(64L, metrics.innvocations().get("serverThreadPoolSize").val);
+        assertEquals(0L, metrics.innvocations().get("serverActiveThreads").val);
+        assertEquals(64L, metrics.innvocations().get("jdisc.thread_pool.work_queue.capacity").val);
+        assertEquals(0L, metrics.innvocations().get("jdisc.thread_pool.work_queue.size").val);
     }
     @Test
     public void testThatQueueSizeAutoDetected() {
@@ -111,7 +127,8 @@ public class DefaultContainerThreadPoolTest {
                         .maxThreads(2)
                         .maxThreadExecutionTimeSeconds(1));
         MockProcessTerminator terminator = new MockProcessTerminator();
-        ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, Mockito.mock(Metric.class), terminator);
+        Metric metrics = new MetricMock();
+        ContainerThreadPool threadPool = new DefaultContainerThreadpool(config, metrics, terminator);
 
         // No dying when threads hang shorter than max thread execution time
         threadPool.executor().execute(new Hang(500));
diff --git a/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java b/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java
index 9207fb34935..2592b587539 100644
--- a/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java
+++ b/container-core/src/test/java/com/yahoo/container/jdisc/ThreadedHttpRequestHandlerTest.java
@@ -1,11 +1,10 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.container.jdisc;
 
+import com.yahoo.container.test.MetricMock;
 import com.yahoo.jdisc.Metric;
 import org.junit.Test;
 
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.Executors;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -23,28 +22,12 @@ public class ThreadedHttpRequestHandlerTest {
 
         driver.sendRequest("http://localhost/myhandler").readAll();
         String expectedMetricName = "jdisc.http.handler.unhandled_exceptions";
-        assertThat(metricMock.addInvocations)
+        assertThat(metricMock.innvocations())
                 .containsKey(expectedMetricName);
-        assertThat(metricMock.addInvocations.get(expectedMetricName).dimensions)
+        assertThat(((MetricMock.SimpleMetricContext)metricMock.innvocations().get(expectedMetricName).ctx).dimensions)
                 .containsEntry("exception", "DummyException");
     }
 
-    private static class MetricMock implements Metric {
-        final ConcurrentHashMap<String, SimpleMetricContext> addInvocations = new ConcurrentHashMap<>();
-
-        @Override public void add(String key, Number val, Context ctx) {
-            addInvocations.put(key, (SimpleMetricContext)ctx);
-        }
-        @Override public void set(String key, Number val, Context ctx) {}
-        @Override public Context createContext(Map<String, ?> properties) { return new SimpleMetricContext(properties); }
-    }
-
-    private static class SimpleMetricContext implements Metric.Context {
-        final Map<String, String> dimensions;
-
-        @SuppressWarnings("unchecked")
-        SimpleMetricContext(Map<String, ?> dimensions) { this.dimensions = (Map<String, String>)dimensions; }
-    }
 
     private static class ThreadedHttpRequestHandlerThrowingException extends ThreadedHttpRequestHandler {
         ThreadedHttpRequestHandlerThrowingException(Metric metric) {
diff --git a/container-core/src/test/java/com/yahoo/container/test/MetricMock.java b/container-core/src/test/java/com/yahoo/container/test/MetricMock.java
new file mode 100644
index 00000000000..8dd023b5605
--- /dev/null
+++ b/container-core/src/test/java/com/yahoo/container/test/MetricMock.java
@@ -0,0 +1,51 @@
+// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.container.test;
+
+import com.yahoo.jdisc.Metric;
+
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+
+/**
+ * Simple mock for use whne testing metrics.
+ *
+ * @author bjorncs
+ */
+public class MetricMock implements Metric {
+    public static class SimpleMetricContext implements Context {
+        public final Map<String, String> dimensions;
+
+        @SuppressWarnings("unchecked")
+        SimpleMetricContext(Map<String, ?> dimensions) {
+            this.dimensions = (Map<String, String>) dimensions;
+        }
+    }
+
+    public static class Invocation {
+        public final Number val;
+        public final Context ctx;
+        public Invocation(Number val, Context ctx) {
+            this.val = val;
+            this.ctx = ctx;
+        }
+    }
+
+    private final Map<String, Invocation> addInvocations = new ConcurrentHashMap<>();
+
+    public Map<String, Invocation> innvocations() {
+        return addInvocations;
+    }
+
+    @Override
+    public void add(String key, Number val, Context ctx) {
+        addInvocations.put(key, new Invocation(val, ctx));
+    }
+
+    @Override
+    public void set(String key, Number val, Context ctx) { addInvocations.put(key, new Invocation(val, ctx)); }
+
+    @Override
+    public Context createContext(Map<String, ?> properties) {
+        return new SimpleMetricContext(properties);
+    }
+}
diff --git a/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java b/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java
index 83173782118..c8a5f17fdfb 100644
--- a/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java
+++ b/container-core/src/test/java/com/yahoo/metrics/simple/MetricsTest.java
@@ -1,18 +1,17 @@
 // Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.metrics.simple;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 import java.util.Collection;
 import java.util.Map.Entry;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
-import com.yahoo.metrics.ManagerConfig;
 import com.yahoo.metrics.simple.jdisc.JdiscMetricsFactory;
 import com.yahoo.metrics.simple.jdisc.SimpleMetricConsumer;