Revert "Merge pull request #10489 from vespa-engine/revert-10485-olaa/parallelize-metric-gathering"

This reverts commit 6b1d725aee53839385a44d4542955dd6b9c5221e, reversing
changes made to 7e26976e385c864da3b8bebaf63d4648bee694b2.

5 files changed, 109 insertions, 64 deletions

diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
index c8b99e2c3a0..c165705dc8c 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/ApplicationRepository.java
@@ -44,9 +44,7 @@ import com.yahoo.vespa.config.server.http.LogRetriever;
 import com.yahoo.vespa.config.server.http.SimpleHttpFetcher;
 import com.yahoo.vespa.config.server.http.v2.MetricsResponse;
 import com.yahoo.vespa.config.server.http.v2.PrepareResult;
-import com.yahoo.vespa.config.server.metrics.ClusterInfo;
-import com.yahoo.vespa.config.server.metrics.MetricsAggregator;
-import com.yahoo.vespa.config.server.metrics.MetricsRetriever;
+import com.yahoo.vespa.config.server.metrics.ClusterMetricsRetriever;
 import com.yahoo.vespa.config.server.provision.HostProvisionerProvider;
 import com.yahoo.vespa.config.server.session.LocalSession;
 import com.yahoo.vespa.config.server.session.LocalSessionRepo;
@@ -73,11 +71,8 @@ import java.time.Duration;
 import java.time.Instant;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.HashSet;
-import java.util.LinkedHashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
 import java.util.logging.Level;
@@ -87,7 +82,6 @@ import java.util.stream.Collectors;
 import static com.yahoo.config.model.api.container.ContainerServiceType.CLUSTERCONTROLLER_CONTAINER;
 import static com.yahoo.config.model.api.container.ContainerServiceType.CONTAINER;
 import static com.yahoo.config.model.api.container.ContainerServiceType.LOGSERVER_CONTAINER;
-import static com.yahoo.config.model.api.container.ContainerServiceType.METRICS_PROXY_CONTAINER;
 import static com.yahoo.vespa.config.server.tenant.TenantRepository.HOSTED_VESPA_TENANT;
 import static java.nio.file.Files.readAttributes;
 
@@ -649,16 +643,8 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
     // ---------------- Metrics ------------------------------------------------------------------------
 
     public MetricsResponse getMetrics(ApplicationId applicationId) {
-        var metricsRetriever = new MetricsRetriever();
-        var clusters = getClustersOfApplication(applicationId);
-        var clusterMetrics = new LinkedHashMap<ClusterInfo, MetricsAggregator>();
-
-        clusters.forEach(cluster -> {
-            var metrics = metricsRetriever.requestMetricsForCluster(cluster);
-            clusterMetrics.put(cluster, metrics);
-        });
-
-        return new MetricsResponse(200, applicationId, clusterMetrics);
+        Application application = getApplication(applicationId);
+        return ClusterMetricsRetriever.getMetrics(application);
     }
 
     // ---------------- Misc operations ----------------------------------------------------------------
@@ -792,35 +778,6 @@ public class ApplicationRepository implements com.yahoo.config.provision.Deploye
         return port;
     }
 
-    /** Finds the hosts of an application, grouped by cluster name */
-    private Collection<ClusterInfo> getClustersOfApplication(ApplicationId applicationId) {
-        Application application = getApplication(applicationId);
-        Map<String, ClusterInfo> clusters = new HashMap<>();
-        application.getModel().getHosts().stream()
-                .filter(host -> host.getServices().stream().noneMatch(serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver")))
-                .forEach(hostInfo -> {
-                            ServiceInfo metricsService = getServiceInfoByType(hostInfo, METRICS_PROXY_CONTAINER.serviceName);
-                            ServiceInfo clusterServiceInfo =  getServiceInfoByType(hostInfo, "container", "searchnode");
-                            ClusterInfo clusterInfo = createClusterInfo(clusterServiceInfo);
-                            URI host = URI.create("http://" + hostInfo.getHostname() + ":" + servicePort(metricsService) + "/metrics/v1/values?consumer=Vespa");
-                            clusters.computeIfAbsent(clusterInfo.getClusterId(), c -> clusterInfo).addHost(host);
-                        }
-                );
-        return clusters.values();
-
-    }
-
-    private ServiceInfo getServiceInfoByType(HostInfo hostInfo, String... types) {
-        List<String> type = List.of(types);
-        return hostInfo.getServices().stream().filter(serviceInfo -> type.contains(serviceInfo.getServiceType())).findFirst().orElseThrow();
-    }
-
-    private ClusterInfo createClusterInfo(ServiceInfo serviceInfo) {
-        String clusterName = serviceInfo.getServiceName();
-        ClusterInfo.ClusterType clusterType = serviceInfo.getServiceType().equals("searchnode") ? ClusterInfo.ClusterType.content : ClusterInfo.ClusterType.container;
-        return new ClusterInfo(clusterName, clusterType);
-    }
-
     /** Returns version to use when deploying application in given environment */
     static Version decideVersion(ApplicationId application, Environment environment, Version sessionVersion, boolean bootstrap) {
         if (     environment.isManuallyDeployed()
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java
index 7507b5c4c2c..bd453dddd74 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterInfo.java
@@ -1,15 +1,22 @@
 // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.config.server.metrics;
 
+import com.yahoo.config.model.api.ServiceInfo;
+
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
+import java.util.Set;
 
 /**
  * @author olaa
  */
 public class ClusterInfo {
 
+    private static final Set<String> CONTENT_SERVICES = Set.of("storagenode", "searchnode", "distributor", "container-clustercontroller");
+    private static final Set<String> CONTAINER_SERVICES = Set.of("qrserver", "container");
+
     private final String clusterId;
     private final ClusterType clusterType;
     private final List<URI> hostnames;
@@ -40,17 +47,18 @@ public class ClusterInfo {
         hostnames.add(host);
     }
 
+    // Try to determine whether host is content or container based on service
+    public static Optional<ClusterInfo> fromServiceInfo(ServiceInfo serviceInfo) {
+        String serviceType = serviceInfo.getServiceType();
+        ClusterType clusterType;
+        if (CONTENT_SERVICES.contains(serviceType)) clusterType = ClusterType.content;
+        else if (CONTAINER_SERVICES.contains(serviceType)) clusterType = ClusterType.container;
+        else return Optional.empty();
+        return Optional.of(new ClusterInfo(serviceInfo.getServiceName(), clusterType));
+    }
+
     public enum ClusterType {
         content,
-        container;
-
-        public static boolean isValidType(String enumString) {
-            try {
-                valueOf(enumString);
-                return true;
-            } catch (IllegalArgumentException e) {
-                return false;
-            }
-        }
-    };
+        container
+    }
 }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java
new file mode 100644
index 00000000000..50d5dd41d9c
--- /dev/null
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/ClusterMetricsRetriever.java
@@ -0,0 +1,80 @@
+// Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+package com.yahoo.vespa.config.server.metrics;
+
+import com.yahoo.config.model.api.HostInfo;
+import com.yahoo.config.model.api.ServiceInfo;
+import com.yahoo.vespa.config.server.application.Application;
+import com.yahoo.vespa.config.server.http.v2.MetricsResponse;
+import com.yahoo.vespa.model.admin.metricsproxy.MetricsProxyContainer;
+
+import java.net.URI;
+import java.util.Collection;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Optional;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ForkJoinPool;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Predicate;
+
+/**
+ * @author olaa
+ *
+ * Retrieves metrics for given application, grouped by cluster
+ */
+public class ClusterMetricsRetriever {
+
+    public static MetricsResponse getMetrics(Application application) {
+        var clusters = getClustersOfApplication(application);
+        var clusterMetrics = new ConcurrentHashMap<ClusterInfo, MetricsAggregator>();
+
+        Runnable retrieveMetricsJob = () ->
+            clusters.parallelStream().forEach(cluster -> {
+                MetricsAggregator metrics = MetricsRetriever.requestMetricsForCluster(cluster);
+                clusterMetrics.put(cluster, metrics);
+            });
+
+        ForkJoinPool threadPool = new ForkJoinPool(5);
+        threadPool.submit(retrieveMetricsJob);
+        threadPool.shutdown();
+
+        try {
+            threadPool.awaitTermination(1, TimeUnit.MINUTES);
+        } catch (InterruptedException e) {
+            throw new RuntimeException(e);
+        }
+
+        return new MetricsResponse(200, application.getId(), clusterMetrics);
+    }
+
+    /** Finds the hosts of an application, grouped by cluster name */
+    private static Collection<ClusterInfo> getClustersOfApplication(Application application) {
+        Map<String, ClusterInfo> clusters = new HashMap<>();
+
+        application.getModel().getHosts().stream()
+                .filter(host -> host.getServices().stream().noneMatch(isLogserver()))
+                .forEach(hostInfo -> {
+                            ClusterInfo clusterInfo = createClusterInfo(hostInfo);
+                            URI metricsProxyURI = createMetricsProxyURI(hostInfo.getHostname());
+                            clusters.computeIfAbsent(clusterInfo.getClusterId(), c -> clusterInfo).addHost(metricsProxyURI);
+                        }
+                );
+        return clusters.values();
+
+    }
+
+    private static Predicate<ServiceInfo> isLogserver() {
+        return serviceInfo -> serviceInfo.getServiceType().equalsIgnoreCase("logserver");
+    }
+
+    private static URI createMetricsProxyURI(String hostname) {
+        return URI.create("http://" + hostname + ":" + MetricsProxyContainer.BASEPORT + "/metrics/v1/values?consumer=Vespa");
+    }
+
+    private static ClusterInfo createClusterInfo(HostInfo hostInfo) {
+        return hostInfo.getServices().stream()
+                .map(ClusterInfo::fromServiceInfo)
+                .filter(Optional::isPresent)
+                .findFirst().get().orElseThrow();
+    }
+}
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java
index 4fbeae11758..8a18843db5f 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/metrics/MetricsRetriever.java
@@ -27,19 +27,18 @@ import java.util.logging.Logger;
  */
 public class MetricsRetriever {
     private static final Logger log = Logger.getLogger(MetricsRetriever.class.getName());
-    private final HttpClient httpClient = HttpClientBuilder.create().build();
 
     /**
      * Call the metrics API on each host in the cluster and aggregate the metrics
      * into a single value.
      */
-    public MetricsAggregator requestMetricsForCluster(ClusterInfo clusterInfo) {
+    public static MetricsAggregator requestMetricsForCluster(ClusterInfo clusterInfo) {
         var aggregator = new MetricsAggregator();
         clusterInfo.getHostnames().forEach(host -> getHostMetrics(host, aggregator));
         return aggregator;
     }
 
-    private void getHostMetrics(URI hostURI, MetricsAggregator metrics) {
+    private static void getHostMetrics(URI hostURI, MetricsAggregator metrics) {
             Slime responseBody = doMetricsRequest(hostURI);
             var parseError = responseBody.get().field("error_message");
 
@@ -53,9 +52,10 @@ public class MetricsRetriever {
             });
     }
 
-    private Slime doMetricsRequest(URI hostURI) {
+    private static Slime doMetricsRequest(URI hostURI) {
         HttpGet get = new HttpGet(hostURI);
         try {
+            HttpClient httpClient = HttpClientBuilder.create().build();
             HttpResponse response = httpClient.execute(get);
             InputStream is = response.getEntity().getContent();
             Slime slime = SlimeUtils.jsonToSlime(is.readAllBytes());
@@ -66,7 +66,7 @@ public class MetricsRetriever {
         }
     }
 
-    private void parseService(Inspector service, MetricsAggregator metrics) {
+    private static void parseService(Inspector service, MetricsAggregator metrics) {
         String serviceName = service.field("name").asString();
         Instant timestamp = Instant.ofEpochSecond(service.field("timestamp").asLong());
         metrics.setTimestamp(timestamp);
diff --git a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java
index 1b878a432c9..67e91b90803 100644
--- a/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java
+++ b/configserver/src/test/java/com/yahoo/vespa/config/server/metrics/MetricsRetrieverTest.java
@@ -55,7 +55,7 @@ public class MetricsRetrieverTest {
 
         compareAggregators(
                 new MetricsAggregator().addDocumentCount(6000.0),
-                metricsRetriever.requestMetricsForCluster(clusters.get(0))
+                MetricsRetriever.requestMetricsForCluster(clusters.get(0))
         );
 
         compareAggregators(
@@ -64,7 +64,7 @@ public class MetricsRetrieverTest {
                         .addContainerLatency(2000, 0)
                         .addQrLatency(3000, 43)
                         .addFeedLatency(3000, 43),
-                metricsRetriever.requestMetricsForCluster(clusters.get(1))
+                MetricsRetriever.requestMetricsForCluster(clusters.get(1))
         );
 
         wireMock.stop();