diff options
author | Øyvind Grønnesby <oyving@verizonmedia.com> | 2019-08-15 14:21:08 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2019-08-15 14:21:08 +0200 |
commit | f6712d97f2b06cc8717911a93b2ff66c6c2953a8 (patch) | |
tree | 2aae68e5f68170336956a95ba191fabe164bf3ac | |
parent | 118e44351bcdf6d6517e4a959efdec9cd8b22728 (diff) | |
parent | 83a792b4bc0c8f2991b1a87ff5f824c5ae03e024 (diff) |
Merge pull request #10290 from vespa-engine/ogronnesby/update-deployment-metrics-aggregation
Improve aggregation over .sum()
3 files changed, 52 insertions, 16 deletions
diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/ClusterMetrics.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/ClusterMetrics.java index 1377a333335..28c94960daf 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/ClusterMetrics.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/application/v4/model/ClusterMetrics.java @@ -1,15 +1,23 @@ // Copyright 2019 Oath Inc. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.vespa.hosted.controller.api.application.v4.model; -import java.util.Collections; import java.util.HashMap; import java.util.Map; +import java.util.Optional; +import java.util.OptionalDouble; /** * @author olaa */ public class ClusterMetrics { + // These field names originate from the MetricsResponse class + private static final String QUERIES_PER_SECOND = "queriesPerSecond"; + private static final String FEED_PER_SECOND = "feedPerSecond"; + private static final String DOCUMENT_COUNT = "documentCount"; + private static final String FEED_LATENCY = "feedLatency"; + private static final String QUERY_LATENCY = "queryLatency"; + private final String clusterId; private final ClusterType clusterType; private final Map<String, Double> metrics; @@ -28,8 +36,24 @@ public class ClusterMetrics { return clusterType; } - public Map<String, Double> getMetrics() { - return Collections.unmodifiableMap(metrics); + public Optional<Double> queriesPerSecond() { + return Optional.ofNullable(metrics.get(QUERIES_PER_SECOND)); + } + + public Optional<Double> feedPerSecond() { + return Optional.ofNullable(metrics.get(FEED_PER_SECOND)); + } + + public Optional<Double> documentCount() { + return Optional.ofNullable(metrics.get(DOCUMENT_COUNT)); + } + + public Optional<Double> feedLatency() { + return Optional.ofNullable(metrics.get(FEED_LATENCY)); + } + + public Optional<Double> queryLatency() { + return Optional.ofNullable(metrics.get(QUERY_LATENCY)); } public void addMetric(String name, double value) { diff --git a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java index a47e2165291..2240afee804 100644 --- a/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java +++ b/controller-api/src/main/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsService.java @@ -10,7 +10,8 @@ import com.yahoo.vespa.hosted.controller.api.integration.routing.RotationStatus; import java.util.List; import java.util.Map; -import java.util.stream.Stream; +import java.util.Optional; +import java.util.function.Function; /** * Retrieves metrics from the configuration server. @@ -35,17 +36,13 @@ public class ConfigServerMetricsService implements MetricsService { var deploymentId = new DeploymentId(application, zone); var metrics = configServerClient.getMetrics(deploymentId); - // TODO(ogronnesby): We probably want something more intelligent than just using .sum(), but it's better to - // TODO(ogronnesby): get some values populated and then fix the formula later. - // The field names here come from the MetricsResponse class. - return new DeploymentMetrics( - doubleStream(metrics, "queriesPerSecond").mapToDouble(Double::doubleValue).sum(), - doubleStream(metrics, "feedPerSecond").mapToDouble(Double::doubleValue).sum(), - doubleStream(metrics, "documentCount").mapToLong(Double::longValue).sum(), - doubleStream(metrics, "queryLatency").mapToDouble(Double::doubleValue).sum(), - doubleStream(metrics, "feedLatency").mapToDouble(Double::doubleValue).sum() + metrics.stream().flatMap(m -> m.queriesPerSecond().stream()).mapToDouble(Double::doubleValue).sum(), + metrics.stream().flatMap(m -> m.feedPerSecond().stream()).mapToDouble(Double::doubleValue).sum(), + metrics.stream().flatMap(m -> m.documentCount().stream()).mapToLong(Double::longValue).sum(), + weightedAverageLatency(metrics, ClusterMetrics::queriesPerSecond, ClusterMetrics::queryLatency), + weightedAverageLatency(metrics, ClusterMetrics::feedPerSecond, ClusterMetrics::feedLatency) ); } @@ -62,7 +59,22 @@ public class ConfigServerMetricsService implements MetricsService { return Map.of(); } - private Stream<Double> doubleStream(List<ClusterMetrics> metrics, String name) { - return metrics.stream().map(m -> m.getMetrics().getOrDefault(name, 0.0)); + private double weightedAverageLatency(List<ClusterMetrics> metrics, + Function<ClusterMetrics, Optional<Double>> rateExtractor, + Function<ClusterMetrics, Optional<Double>> latencyExtractor) + { + var rateSum = metrics.stream().flatMap(m -> rateExtractor.apply(m).stream()).mapToDouble(Double::longValue).sum(); + if (rateSum == 0) { + return 0.0; + } + + var weightedLatency = metrics.stream() + .flatMap(m -> { + return latencyExtractor.apply(m).flatMap(l -> rateExtractor.apply(m).map(r -> l * r)).stream(); + }) + .mapToDouble(Double::doubleValue) + .sum(); + + return weightedLatency / rateSum; } } diff --git a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsServiceTest.java b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsServiceTest.java index 9c14bc641c7..01998d634ca 100644 --- a/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsServiceTest.java +++ b/controller-api/src/test/java/com/yahoo/vespa/hosted/controller/api/integration/metrics/ConfigServerMetricsServiceTest.java @@ -60,7 +60,7 @@ public class ConfigServerMetricsServiceTest { var deploymentMetrics = service.getDeploymentMetrics(applicationId, zoneId); assertEquals(23.0 + 11.0, deploymentMetrics.queriesPerSecond(), 0.001); - assertEquals(1337.0 + 12.0, deploymentMetrics.queryLatencyMillis(), 0.001); // TODO: again, this definition of combined latency makes no sense + assertEquals(908.323, deploymentMetrics.queryLatencyMillis(), 0.001); assertEquals(0, deploymentMetrics.documentCount()); assertEquals(0.0, deploymentMetrics.writeLatencyMillis(), 0.001); assertEquals(0.0, deploymentMetrics.writesPerSecond(), 0.001); |