Store and read all metrics together

2 files changed, 54 insertions, 59 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricSnapshot.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricSnapshot.java
index 43636c6573a..53a1c1047e3 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricSnapshot.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/MetricSnapshot.java
@@ -2,6 +2,7 @@
 package com.yahoo.vespa.hosted.provision.autoscale;
 
 import com.yahoo.config.provision.ClusterSpec;
+import com.yahoo.metrics.simple.Measurement;
 import com.yahoo.vespa.hosted.provision.Node;
 import com.yahoo.vespa.hosted.provision.NodeRepository;
 import com.yahoo.vespa.hosted.provision.applications.Cluster;
@@ -55,7 +56,7 @@ public class MetricSnapshot {
                 if (nodeGenerationMeasurements.isPresent()) {
                     var firstMeasurementOfCorrectGeneration =
                             nodeGenerationMeasurements.get().asList().stream()
-                                                      .filter(m -> m.value() >= deployment.generation())
+                                                      .filter(m -> m.generation() >= deployment.generation())
                                                       .findFirst();
                     if (firstMeasurementOfCorrectGeneration.isPresent()) {
                         startTimePerHost.put(node.hostname(), firstMeasurementOfCorrectGeneration.get().at());
@@ -85,10 +86,20 @@ public class MetricSnapshot {
         if (measurementCount / clusterNodes.size() < Autoscaler.minimumMeasurementsPerNode(clusterType)) return Optional.empty();
         if (measurements.size() != clusterNodes.size()) return Optional.empty();
 
-        double measurementSum = measurements.stream().flatMap(m -> m.asList().stream()).mapToDouble(m -> m.value()).sum();
+
+        double measurementSum = measurements.stream().flatMap(m -> m.asList().stream()).mapToDouble(m -> value(resource, m)).sum();
         return Optional.of(measurementSum / measurementCount);
     }
 
+    private double value(Resource resource, NodeMetricsDb.Measurement measurement) {
+        switch (resource) {
+            case cpu: return measurement.cpu();
+            case memory: return measurement.memopry();
+            case disk: return measurement.disk();
+            default: throw new IllegalArgumentException("Got an unknown resource " + resource);
+        }
+    }
+
     private List<NodeMetricsDb.NodeMeasurements> filterStale(List<NodeMetricsDb.NodeMeasurements> measurements,
                                                              Map<String, Instant> startTimePerHost) {
         if (startTimePerHost.isEmpty()) return measurements; // Map is either empty or complete
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java
index ba7d65eef1c..28ccfed0789 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDb.java
@@ -14,7 +14,6 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
 
@@ -28,8 +27,8 @@ public class NodeMetricsDb {
 
     private final NodeRepository nodeRepository;
 
-    /** Measurements by key. Each list of measurements is sorted by increasing timestamp */
-    private final Map<NodeMeasurementsKey, NodeMeasurements> db = new HashMap<>();
+    /** Measurements by host. Each list of measurements is sorted by increasing timestamp */
+    private final Map<String, NodeMeasurements> db = new HashMap<>();
 
     /** Lock all access for now since we modify lists inside a map */
     private final Object lock = new Object();
@@ -42,29 +41,23 @@ public class NodeMetricsDb {
     public void add(Collection<MetricsFetcher.NodeMetrics> nodeMetrics) {
         synchronized (lock) {
             for (var value : nodeMetrics) {
-                add(value.hostname(), value.timestampSecond(), value.cpuUtil(), Metric.cpu);
-                add(value.hostname(), value.timestampSecond(), value.totalMemUtil(), Metric.memory);
-                add(value.hostname(), value.timestampSecond(), value.diskUtil(), Metric.disk);
-                add(value.hostname(), value.timestampSecond(), value.applicationGeneration(), Metric.generation);
+                add(value.hostname(), new Measurement(value));
             }
         }
     }
 
-    private void add(String hostname, long timestampSeconds, double value, Metric metric) {
-        NodeMeasurementsKey key = new NodeMeasurementsKey(hostname, metric);
-        NodeMeasurements measurements = db.get(key);
+    private void add(String hostname, Measurement measurement) {
+        NodeMeasurements measurements = db.get(hostname);
         if (measurements == null) { // new node
             Optional<Node> node = nodeRepository.getNode(hostname);
             if (node.isEmpty()) return;
             if (node.get().allocation().isEmpty()) return;
             measurements = new NodeMeasurements(hostname,
-                                                metric,
                                                 node.get().allocation().get().membership().cluster().type(),
                                                 new ArrayList<>());
-            db.put(key, measurements);
+            db.put(hostname, measurements);
         }
-        measurements.add(new Measurement(timestampSeconds * 1000,
-                                         metric.valueFromMetric(value)));
+        measurements.add(measurement);
     }
 
     /** Must be called intermittently (as long as any add methods are called) to gc old data */
@@ -78,8 +71,6 @@ public class NodeMetricsDb {
                 if (measurements.isEmpty())
                     i.remove();
             }
-
-            // TODO: gc events
         }
     }
 
@@ -91,7 +82,7 @@ public class NodeMetricsDb {
         synchronized (lock) {
             List<NodeMeasurements> measurementsList = new ArrayList<>(hostnames.size());
             for (String hostname : hostnames) {
-                NodeMeasurements measurements = db.get(new NodeMeasurementsKey(hostname, metric));
+                NodeMeasurements measurements = db.get(hostname);
                 if (measurements == null) continue;
                 measurements = measurements.copyAfter(startTime);
                 if (measurements.isEmpty()) continue;
@@ -101,47 +92,15 @@ public class NodeMetricsDb {
         }
     }
 
-    private static class NodeMeasurementsKey {
-
-        private final String hostname;
-        private final Metric metric;
-
-        public NodeMeasurementsKey(String hostname, Metric metric) {
-            this.hostname = hostname;
-            this.metric = metric;
-        }
-
-        @Override
-        public int hashCode() {
-            return Objects.hash(hostname, metric);
-        }
-
-        @Override
-        public boolean equals(Object o) {
-            if (this == o) return true;
-            if ( ! (o instanceof NodeMeasurementsKey)) return false;
-            NodeMeasurementsKey other = (NodeMeasurementsKey)o;
-            if ( ! this.hostname.equals(other.hostname)) return false;
-            if ( ! this.metric.equals(other.metric)) return false;
-            return true;
-        }
-
-        @Override
-        public String toString() { return "key to measurements of " + metric + " for " + hostname; }
-
-    }
-
     public static class NodeMeasurements {
 
         private final String hostname;
-        private final Metric metric;
         private final ClusterSpec.Type type;
         private final List<Measurement> measurements;
 
         // Note: This transfers ownership of the measurement list to this
-        private NodeMeasurements(String hostname, Metric metric, ClusterSpec.Type type, List<Measurement> measurements) {
+        private NodeMeasurements(String hostname, ClusterSpec.Type type, List<Measurement> measurements) {
             this.hostname = hostname;
-            this.metric = metric;
             this.type = type;
             this.measurements = measurements;
         }
@@ -160,7 +119,7 @@ public class NodeMetricsDb {
 
         public NodeMeasurements copyAfter(Instant oldestTime) {
             long oldestTimestamp = oldestTime.toEpochMilli();
-            return new NodeMeasurements(hostname, metric, type,
+            return new NodeMeasurements(hostname, type,
                                         measurements.stream()
                                                     .filter(measurement -> measurement.timestamp >= oldestTimestamp)
                                                     .collect(Collectors.toList()));
@@ -183,19 +142,44 @@ public class NodeMetricsDb {
         /** The time of this measurement in epoch millis */
         private final long timestamp;
 
-        /** The measured value */
-        private final double value;
+        private final double cpu;
+        private final double memory;
+        private final double disk;
+        private final long generation;
+
+        public Measurement(MetricsFetcher.NodeMetrics metrics) {
+            this.timestamp = metrics.timestampSecond() * 1000;
+            this.cpu = Metric.cpu.valueFromMetric(metrics.cpuUtil());
+            this.memory = Metric.memory.valueFromMetric(metrics.totalMemUtil());
+            this.disk = Metric.disk.valueFromMetric(metrics.diskUtil());
+            this.generation = (long)Metric.generation.valueFromMetric(metrics.applicationGeneration());
+
+        }
 
-        public Measurement(long timestamp, float value) {
+        public Measurement(long timestamp,
+                           float cpu,
+                           float memory,
+                           float disk,
+                           float generation) {
             this.timestamp = timestamp;
-            this.value = value;
+            this.cpu = cpu;
+            this.memory = memory;
+            this.disk = disk;
+            this.generation = (long) generation;
         }
 
-        public double value() { return value; }
+        public double cpu() { return cpu; }
+        public double memopry() { return memory; }
+        public double disk() { return disk; }
+        public long generation() { return generation; }
         public Instant at() { return Instant.ofEpochMilli(timestamp); }
 
         @Override
-        public String toString() { return "measurement at " + timestamp + ": " + value; }
+        public String toString() { return "measurement at " + timestamp + ": " +
+                                          "cpu: " + cpu +
+                                          "memory: " + memory +
+                                          "disk: " + disk +
+                                          "generation: " + generation; }
 
     }