Merge pull request #14275 from vespa-engine/bjorncs/feed-threadpool-feature-flag

Bjorncs/feed threadpool feature flag

9 files changed, 64 insertions, 40 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index 413a7dbc62e..8ccd1e67ba1 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -120,7 +120,7 @@ public interface ModelContext {
         default Duration jdiscHealthCheckProxyClientTimeout() { return Duration.ofMillis(100); }
 
         // TODO(bjorncs): Temporary feature flag
-        default double feedCoreThreadPoolSizeFactor() { return 1.0; }
+        default double feedCoreThreadPoolSizeFactor() { return 4.0; }
 
         default Quota quota() {
             return Quota.empty();
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
index a4737c9f54c..9012cc2aba0 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/clients/ContainerDocumentApi.java
@@ -64,10 +64,9 @@ public class ContainerDocumentApi {
     }
 
     private static ThreadPoolExecutorComponent newExecutorComponent(String name, ContainerCluster<?> cluster, Options options) {
-        int maxPoolSize = maxPoolSize(cluster);
         return new ThreadPoolExecutorComponent.Builder(name)
-                .maxPoolSize(maxPoolSize)
-                .corePoolSize(corePoolSize(maxPoolSize, options))
+                .maxPoolSize(maxPoolSize(cluster, options))
+                .corePoolSize(corePoolSize(cluster, options))
                 .queueSize(500)
                 .build();
     }
@@ -93,29 +92,36 @@ public class ContainerDocumentApi {
         return handler;
     }
 
-    private static int maxPoolSize(ContainerCluster<?> cluster) {
+    private static int maxPoolSize(ContainerCluster<?> cluster, Options options) {
+        double vcpu = vcpu(cluster);
+        if (vcpu == 0) return FALLBACK_MAX_POOL_SIZE;
+        return Math.max(2, (int)Math.ceil(vcpu * options.feedThreadPoolSizeFactor));
+    }
+
+    private static int corePoolSize(ContainerCluster<?> cluster, Options options) {
+        double vcpu = vcpu(cluster);
+        if (vcpu == 0) return FALLBACK_CORE_POOL_SIZE;
+        return Math.max(1, (int)Math.ceil(vcpu * options.feedThreadPoolSizeFactor * 0.5));
+    }
+
+    private static double vcpu(ContainerCluster<?> cluster) {
         List<Double> vcpus = cluster.getContainers().stream()
                 .filter(c -> c.getHostResource() != null && c.getHostResource().realResources() != null)
                 .map(c -> c.getHostResource().realResources().vcpu())
                 .distinct()
                 .collect(Collectors.toList());
         // We can only use host resource for calculation if all container nodes in the cluster are homogeneous (in terms of vcpu)
-        if (vcpus.size() != 1 || vcpus.get(0) == 0) return FALLBACK_MAX_POOL_SIZE;
-        return Math.max(2, (int)Math.ceil(vcpus.get(0)));
-    }
-
-    private static int corePoolSize(int maxPoolSize, Options options) {
-        if (maxPoolSize == FALLBACK_MAX_POOL_SIZE) return FALLBACK_CORE_POOL_SIZE;
-        return Math.max(1, (int)Math.ceil(options.feedCoreThreadPoolSizeFactor * maxPoolSize));
+        if (vcpus.size() != 1 || vcpus.get(0) == 0) return 0;
+        return vcpus.get(0);
     }
 
     public static final class Options {
         private final Collection<String> bindings;
-        private final double feedCoreThreadPoolSizeFactor;
+        private final double feedThreadPoolSizeFactor;
 
-        public Options(Collection<String> bindings, double feedCoreThreadPoolSizeFactor) {
+        public Options(Collection<String> bindings, double feedThreadPoolSizeFactor) {
             this.bindings = Collections.unmodifiableCollection(bindings);
-            this.feedCoreThreadPoolSizeFactor = feedCoreThreadPoolSizeFactor;
+            this.feedThreadPoolSizeFactor = feedThreadPoolSizeFactor;
         }
     }
 
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java
index 37e5dc21346..ab73309bb7d 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/ContainerDocumentApiBuilderTest.java
@@ -116,8 +116,8 @@ public class ContainerDocumentApiBuilderTest extends ContainerModelBuilderTestBa
         assertThat(injectedComponentIds, hasItem("threadpool@feedapi-handler"));
 
         ThreadpoolConfig config = root.getConfig(ThreadpoolConfig.class, "cluster1/component/com.yahoo.vespa.http.server.FeedHandler/threadpool@feedapi-handler");
-        assertEquals(4, config.maxthreads());
-        assertEquals(4, config.corePoolSize());
+        assertEquals(16, config.maxthreads());
+        assertEquals(8, config.corePoolSize());
     }
 
     private static class HostProvisionerWithCustomRealResource implements HostProvisioner {
diff --git a/container-di/pom.xml b/container-di/pom.xml
index 8e89910e247..f143d5afd19 100644
--- a/container-di/pom.xml
+++ b/container-di/pom.xml
@@ -119,6 +119,10 @@
           <groupId>com.yahoo.vespa</groupId>
           <artifactId>abi-check-plugin</artifactId>
         </plugin>
+        <plugin>
+          <groupId>org.apache.maven.plugins</groupId>
+          <artifactId>maven-compiler-plugin</artifactId>
+        </plugin>
       </plugins>
   </build>
 </project>
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
index f33ca549cf7..b7c7fc5ff15 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/Flags.java
@@ -346,8 +346,8 @@ public class Flags {
     );
 
     public static final UnboundDoubleFlag FEED_CORE_THREAD_POOL_SIZE_FACTOR = defineDoubleFlag(
-            "feed-core-thread-pool-size-factor", 1.0,
-            "Number of core threads in threadpool for feeding APIs as factor of max pool size",
+            "feed-core-thread-pool-size-factor", 4.0,
+            "Max threads in threadpool for feeding APIs as a factor of vcpu",
             "Takes effect on next internal redeployment",
             APPLICATION_ID);
 
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java
index cf33b6033cd..6000026580a 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/document/restapi/resource/RestApi.java
@@ -3,11 +3,11 @@ package com.yahoo.document.restapi.resource;
 
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
-import com.google.inject.Inject;
-
 import com.fasterxml.jackson.databind.node.ObjectNode;
+import com.google.inject.Inject;
 import com.yahoo.cloud.config.ClusterListConfig;
 import com.yahoo.container.handler.ThreadpoolConfig;
+import com.yahoo.container.handler.threadpool.ContainerThreadPool;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
@@ -15,7 +15,6 @@ import com.yahoo.container.logging.AccessLog;
 import com.yahoo.document.DocumentTypeManager;
 import com.yahoo.document.TestAndSetCondition;
 import com.yahoo.document.config.DocumentmanagerConfig;
-
 import com.yahoo.document.json.SingleDocumentParser;
 import com.yahoo.document.restapi.OperationHandler;
 import com.yahoo.document.restapi.OperationHandlerImpl;
@@ -27,11 +26,11 @@ import com.yahoo.document.select.parser.ParseException;
 import com.yahoo.documentapi.messagebus.MessageBusDocumentAccess;
 import com.yahoo.documentapi.messagebus.MessageBusParams;
 import com.yahoo.documentapi.messagebus.loadtypes.LoadTypeSet;
-import java.util.logging.Level;
+import com.yahoo.jdisc.Metric;
 import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.text.Text;
-import com.yahoo.vespa.config.content.LoadTypeConfig;
 import com.yahoo.vespa.config.content.AllClustersBucketSpacesConfig;
+import com.yahoo.vespa.config.content.LoadTypeConfig;
 import com.yahoo.vespaclient.ClusterDef;
 import com.yahoo.vespaclient.ClusterList;
 import com.yahoo.vespaxmlparser.DocumentFeedOperation;
@@ -46,6 +45,7 @@ import java.util.List;
 import java.util.Optional;
 import java.util.concurrent.Executor;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Level;
 
 import static com.yahoo.jdisc.Response.Status.BAD_REQUEST;
 
@@ -77,11 +77,16 @@ public class RestApi extends LoggingRequestHandler {
     private final AtomicInteger threadsAvailableForApi;
 
     @Inject
-    public RestApi(LoggingRequestHandler.Context parentCtx, DocumentmanagerConfig documentManagerConfig,
-                   LoadTypeConfig loadTypeConfig, ThreadpoolConfig threadpoolConfig,
+    public RestApi(ContainerThreadPool threadpool,
+                   AccessLog accessLog,
+                   Metric metric,
+                   DocumentmanagerConfig documentManagerConfig,
+                   LoadTypeConfig loadTypeConfig,
+                   ThreadpoolConfig threadpoolConfig,
                    AllClustersBucketSpacesConfig bucketSpacesConfig,
-                   ClusterListConfig clusterListConfig, MetricReceiver metricReceiver) {
-        super(parentCtx);
+                   ClusterListConfig clusterListConfig,
+                   MetricReceiver metricReceiver) {
+        super(threadpool.executor(), accessLog, metric);
         MessageBusParams params = new MessageBusParams(new LoadTypeSet(loadTypeConfig));
         params.setDocumentmanagerConfig(documentManagerConfig);
         this.operationHandler = new OperationHandlerImpl(new MessageBusDocumentAccess(params),
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java
index 1df0ce3594b..7776c8fa34c 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandler.java
@@ -3,12 +3,15 @@ package com.yahoo.vespa.http.server;
 
 import com.yahoo.collections.Tuple2;
 import com.yahoo.container.handler.ThreadpoolConfig;
+import com.yahoo.container.handler.threadpool.ContainerThreadPool;
 import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
 import com.yahoo.container.jdisc.messagebus.SessionCache;
+import com.yahoo.container.logging.AccessLog;
 import com.yahoo.document.config.DocumentmanagerConfig;
 import com.yahoo.documentapi.metrics.DocumentApiMetrics;
+import com.yahoo.jdisc.Metric;
 import com.yahoo.messagebus.ReplyHandler;
 import com.yahoo.metrics.simple.MetricReceiver;
 import com.yahoo.vespa.http.client.core.Headers;
@@ -39,15 +42,17 @@ public class FeedHandler extends LoggingRequestHandler {
     private final DocumentApiMetrics metricsHelper;
 
     @Inject
-    public FeedHandler(LoggingRequestHandler.Context parentCtx,
+    public FeedHandler(ContainerThreadPool threadpool,
+                       Metric metric,
+                       AccessLog accessLog,
                        DocumentmanagerConfig documentManagerConfig,
                        SessionCache sessionCache,
                        ThreadpoolConfig threadpoolConfig,
                        MetricReceiver metricReceiver) throws Exception {
-        super(parentCtx);
+        super(threadpool.executor(), accessLog, metric);
         metricsHelper = new DocumentApiMetrics(metricReceiver, "vespa.http.server");
-        feedHandlerV3 = new FeedHandlerV3(parentCtx, documentManagerConfig, sessionCache, threadpoolConfig, metricsHelper);
-        feedReplyHandler = new FeedReplyReader(parentCtx.getMetric(), metricsHelper);
+        feedHandlerV3 = new FeedHandlerV3(threadpool.executor(), metric, accessLog, documentManagerConfig, sessionCache, threadpoolConfig, metricsHelper);
+        feedReplyHandler = new FeedReplyReader(metric, metricsHelper);
     }
 
     private Tuple2<HttpResponse, Integer> checkProtocolVersion(HttpRequest request) {
diff --git a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java
index a932ca935e0..9bd48b707f8 100644
--- a/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java
+++ b/vespaclient-container-plugin/src/main/java/com/yahoo/vespa/http/server/FeedHandlerV3.java
@@ -7,26 +7,26 @@ import com.yahoo.container.jdisc.HttpRequest;
 import com.yahoo.container.jdisc.HttpResponse;
 import com.yahoo.container.jdisc.LoggingRequestHandler;
 import com.yahoo.container.jdisc.messagebus.SessionCache;
+import com.yahoo.container.logging.AccessLog;
 import com.yahoo.document.DocumentTypeManager;
 import com.yahoo.document.config.DocumentmanagerConfig;
 import com.yahoo.documentapi.metrics.DocumentApiMetrics;
 import com.yahoo.jdisc.Metric;
 import com.yahoo.jdisc.ReferencedResource;
-import java.util.logging.Level;
 import com.yahoo.messagebus.ReplyHandler;
 import com.yahoo.messagebus.SourceSessionParams;
 import com.yahoo.messagebus.shared.SharedSourceSession;
 import com.yahoo.vespa.http.client.core.Headers;
 import com.yahoo.yolean.Exceptions;
 
-import java.time.Duration;
-import java.time.Instant;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
+import java.util.concurrent.Executor;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicInteger;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 /**
@@ -48,18 +48,20 @@ public class FeedHandlerV3 extends LoggingRequestHandler {
     private final AtomicInteger threadsAvailableForFeeding;
     private static final Logger log = Logger.getLogger(FeedHandlerV3.class.getName());
 
-    public FeedHandlerV3(LoggingRequestHandler.Context parentCtx,
+    public FeedHandlerV3(Executor executor,
+                         Metric metric,
+                         AccessLog accessLog,
                          DocumentmanagerConfig documentManagerConfig,
                          SessionCache sessionCache,
                          ThreadpoolConfig threadpoolConfig,
                          DocumentApiMetrics metricsHelper) {
-        super(parentCtx);
+        super(executor, accessLog, metric);
         docTypeManager = new DocumentTypeManager(documentManagerConfig);
         this.sessionCache = sessionCache;
-        feedReplyHandler = new FeedReplyReader(parentCtx.getMetric(), metricsHelper);
+        feedReplyHandler = new FeedReplyReader(metric, metricsHelper);
         cron = new ScheduledThreadPoolExecutor(1, ThreadFactoryFactory.getThreadFactory("feedhandlerv3.cron"));
         cron.scheduleWithFixedDelay(this::removeOldClients, 16, 11, TimeUnit.MINUTES);
-        this.metric = parentCtx.getMetric();
+        this.metric = metric;
         // 40% of the threads can be blocking on feeding before we deny requests.
         if (threadpoolConfig != null) {
             threadsAvailableForFeeding = new AtomicInteger(Math.max((int) (0.4 * threadpoolConfig.maxthreads()), 1));
diff --git a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java
index 1efa8129cdb..a6cc52e8aea 100644
--- a/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java
+++ b/vespaclient-container-plugin/src/test/java/com/yahoo/feedhandler/v3/FeedTesterV3.java
@@ -120,7 +120,9 @@ public class FeedTesterV3 {
         Executor threadPool = Executors.newCachedThreadPool();
         DocumentmanagerConfig docMan = new DocumentmanagerConfig(new DocumentmanagerConfig.Builder().enablecompression(true));
         FeedHandlerV3 feedHandlerV3 = new FeedHandlerV3(
-                new FeedHandlerV3.Context(threadPool, AccessLog.voidAccessLog(), metric),
+                threadPool,
+                metric,
+                AccessLog.voidAccessLog(),
                 docMan,
                 null /* session cache */,
                 threadPoolConfig /* thread pool config */,