Sort snapshots explicitly

3 files changed, 32 insertions, 30 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java
index 769174a188e..80a5fe98350 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterTimeseries.java
@@ -27,8 +27,8 @@ public class ClusterTimeseries {
     final int measurementCountWithoutStaleOutOfService;
     final int measurementCountWithoutStaleOutOfServiceUnstable;
 
-    /** The measurements for all hosts in this snapshot */
-    private final List<NodeTimeseries> nodeTimeseries;
+    /** The measurements for all nodes in this snapshot */
+    private final List<NodeTimeseries> allNodeTimeseries;
 
     public ClusterTimeseries(Cluster cluster, List<Node> clusterNodes, MetricsDb db, NodeRepository nodeRepository) {
         this.clusterNodes = clusterNodes;
@@ -48,7 +48,7 @@ public class ClusterTimeseries {
         timeseries = filter(timeseries, snapshot -> snapshot.stable());
         measurementCountWithoutStaleOutOfServiceUnstable = timeseries.stream().mapToInt(m -> m.size()).sum();
 
-        this.nodeTimeseries = timeseries;
+        this.allNodeTimeseries = timeseries;
     }
 
     /**
@@ -57,23 +57,22 @@ public class ClusterTimeseries {
      */
     private Map<String, Instant> metricStartTimes(Cluster cluster,
                                                   List<Node> clusterNodes,
-                                                  List<NodeTimeseries> nodeTimeseries,
+                                                  List<NodeTimeseries> allNodeTimeseries,
                                                   NodeRepository nodeRepository) {
+        if (cluster.lastScalingEvent().isEmpty()) return Map.of();
+
+        var deployment = cluster.lastScalingEvent().get();
         Map<String, Instant> startTimePerHost = new HashMap<>();
-        if (cluster.lastScalingEvent().isPresent()) {
-            var deployment = cluster.lastScalingEvent().get();
-            for (Node node : clusterNodes) {
-                startTimePerHost.put(node.hostname(), nodeRepository.clock().instant()); // Discard all unless we can prove otherwise
-                var nodeGenerationMeasurements =
-                        nodeTimeseries.stream().filter(m -> m.hostname().equals(node.hostname())).findAny();
-                if (nodeGenerationMeasurements.isPresent()) {
-                    var firstMeasurementOfCorrectGeneration =
-                            nodeGenerationMeasurements.get().asList().stream()
-                                                      .filter(m -> m.generation() >= deployment.generation())
-                                                      .findFirst();
-                    if (firstMeasurementOfCorrectGeneration.isPresent()) {
-                        startTimePerHost.put(node.hostname(), firstMeasurementOfCorrectGeneration.get().at());
-                    }
+        for (Node node : clusterNodes) {
+            startTimePerHost.put(node.hostname(), nodeRepository.clock().instant()); // Discard all unless we can prove otherwise
+            var nodeTimeseries = allNodeTimeseries.stream().filter(m -> m.hostname().equals(node.hostname())).findAny();
+            if (nodeTimeseries.isPresent()) {
+                var firstMeasurementOfCorrectGeneration =
+                        nodeTimeseries.get().asList().stream()
+                                                     .filter(m -> m.generation() >= deployment.generation())
+                                                     .findFirst();
+                if (firstMeasurementOfCorrectGeneration.isPresent()) {
+                    startTimePerHost.put(node.hostname(), firstMeasurementOfCorrectGeneration.get().at());
                 }
             }
         }
@@ -82,19 +81,19 @@ public class ClusterTimeseries {
 
     /** Returns the average number of measurements per node */
     public int measurementsPerNode() {
-        int measurementCount = nodeTimeseries.stream().mapToInt(m -> m.size()).sum();
+        int measurementCount = allNodeTimeseries.stream().mapToInt(m -> m.size()).sum();
         return measurementCount / clusterNodes.size();
     }
 
     /** Returns the number of nodes measured in this */
     public int nodesMeasured() {
-        return nodeTimeseries.size();
+        return allNodeTimeseries.size();
     }
 
     /** Returns the average load of this resource in this */
     public double averageLoad(Resource resource) {
-        int measurementCount = nodeTimeseries.stream().mapToInt(m -> m.size()).sum();
-        double measurementSum = nodeTimeseries.stream().flatMap(m -> m.asList().stream()).mapToDouble(m -> value(resource, m)).sum();
+        int measurementCount = allNodeTimeseries.stream().mapToInt(m -> m.size()).sum();
+        double measurementSum = allNodeTimeseries.stream().flatMap(m -> m.asList().stream()).mapToDouble(m -> value(resource, m)).sum();
         return measurementSum / measurementCount;
     }
 
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 7fb2de5d958..aae3e5173d4 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
@@ -8,9 +8,8 @@ import java.time.Instant;
  *
  * @author bratseth
  */
-public class MetricSnapshot {
+public class MetricSnapshot implements Comparable<MetricSnapshot> {
 
-    // TODO: Order by timestamp
     private final Instant at;
 
     private final double cpu;
@@ -40,6 +39,11 @@ public class MetricSnapshot {
     public boolean stable() { return stable; }
 
     @Override
+    public int compareTo(MetricSnapshot other) {
+        return at.compareTo(other.at);
+    }
+
+    @Override
     public String toString() { return "metrics at " + at + ":" +
                                       " cpu: " + cpu +
                                       " memory: " + memory +
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeTimeseries.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeTimeseries.java
index bebe87929ff..24876609f58 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeTimeseries.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/NodeTimeseries.java
@@ -1,8 +1,6 @@
 // Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.provision.autoscale;
 
-import com.yahoo.config.provision.ClusterSpec;
-
 import java.time.Instant;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -11,7 +9,7 @@ import java.util.function.Predicate;
 import java.util.stream.Collectors;
 
 /**
- * A list of metric snapshots from a host
+ * A list of metric snapshots from a node, sorted by increasing time (newest last).
  *
  * @author bratseth
  */
@@ -20,10 +18,11 @@ public class NodeTimeseries {
     private final String hostname;
     private final List<MetricSnapshot> snapshots;
 
-    // Note: This transfers ownership of the snapshot list to this
     NodeTimeseries(String hostname, List<MetricSnapshot> snapshots) {
         this.hostname = hostname;
-        this.snapshots = snapshots;
+        List<MetricSnapshot> sortedSnapshots = new ArrayList<>(snapshots);
+        Collections.sort(sortedSnapshots);
+        this.snapshots = Collections.unmodifiableList(sortedSnapshots);
     }
 
     public boolean isEmpty() { return snapshots.isEmpty(); }
@@ -32,7 +31,7 @@ public class NodeTimeseries {
 
     public MetricSnapshot get(int index) { return snapshots.get(index); }
 
-    public List<MetricSnapshot> asList() { return Collections.unmodifiableList(snapshots); }
+    public List<MetricSnapshot> asList() { return snapshots; }
 
     public String hostname() { return hostname; }