Test nodemetricsdb

5 files changed, 44 insertions, 19 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
index 803a6fbeb7f..ad0951f16f5 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/Autoscaler.java
@@ -14,6 +14,7 @@ import com.yahoo.vespa.hosted.provision.provisioning.NodeResourceLimits;
 import java.time.Duration;
 import java.util.List;
 import java.util.Optional;
+import java.util.stream.Collectors;
 
 /**
  * The autoscaler makes decisions about the flavor and node count that should be allocated to a cluster
@@ -25,13 +26,14 @@ public class Autoscaler {
 
     /*
      TODO:
-     - X Test gc
-     - X Test AutoscalingMaintainer
      - X Implement node metrics fetch
      - X Avoid making decisions for the same app at multiple config servers
+     - Scale group size
      - Have a better idea about whether we have sufficient information to make decisions
      - Consider taking spikes/variance into account
      - Measure observed regulation lag (startup+redistribution) into account when deciding regulation observation window
+     - Test AutoscalingMaintainer
+     - Include performance not just load+cost
      */
 
     private static final int minimumMeasurements = 500; // TODO: Per node instead? Also say something about interval?
@@ -102,14 +104,7 @@ public class Autoscaler {
         }
         return bestAllocation;
     }
-/*
-    private boolean isSimilar(ClusterResources a1, ClusterResources a2) {
-        if (a1.nodes() != a2.nodes()) return false; // A full node is always a significant difference
-        return isSimilar(a1.nodeResources().vcpu(), a2.nodeResources().vcpu()) &&
-               isSimilar(a1.nodeResources().memoryGb(), a2.nodeResources().memoryGb()) &&
-               isSimilar(a1.nodeResources().diskGb(), a2.nodeResources().diskGb());
-    }
-*/
+
     private boolean similarCost(double cost1, double cost2) {
         return similar(cost1, cost2, costDifferenceRatioWorthReallocation);
     }
@@ -162,7 +157,7 @@ public class Autoscaler {
     private Optional<Double> averageLoad(Resource resource, ClusterSpec cluster, List<Node> clusterNodes) {
         NodeMetricsDb.Window window = metricsDb.getWindow(nodeRepository.clock().instant().minus(scalingWindow(cluster.type())),
                                                           resource,
-                                                          clusterNodes);
+                                                          clusterNodes.stream().map(Node::hostname).collect(Collectors.toList()));
 
         if (window.measurementCount() < minimumMeasurements) return Optional.empty();
         if (window.hostnames() != clusterNodes.size()) return Optional.empty(); // Regulate only when all nodes are measured
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 1a394648a32..d101bd2ce91 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
@@ -31,9 +31,9 @@ public class NodeMetricsDb {
     private final Object lock = new Object();
 
     /** Add a measurement to this */
-    public void add(Node node, Resource resource, Instant timestamp, float value) {
+    public void add(String hostname, Resource resource, Instant timestamp, float value) {
         synchronized (lock) {
-            List<Measurement> measurements = db.computeIfAbsent(new MeasurementKey(node.hostname(), resource), (__) -> new ArrayList<>());
+            List<Measurement> measurements = db.computeIfAbsent(new MeasurementKey(hostname, resource), (__) -> new ArrayList<>());
             measurements.add(new Measurement(timestamp.toEpochMilli(), value));
         }
     }
@@ -60,8 +60,8 @@ public class NodeMetricsDb {
     }
 
     /** Returns a window within which we can ask for specific information from this db */
-    public Window getWindow(Instant startTime, Resource resource, List<Node> nodes) {
-        return new Window(startTime, resource, nodes);
+    public Window getWindow(Instant startTime, Resource resource, List<String> hostnames) {
+        return new Window(startTime, resource, hostnames);
     }
 
     public class Window {
@@ -69,9 +69,9 @@ public class NodeMetricsDb {
         private final long startTime;
         private List<MeasurementKey> keys;
 
-        public Window(Instant startTime, Resource resource, List<Node> nodes) {
+        private Window(Instant startTime, Resource resource, List<String> hostnames) {
             this.startTime = startTime.toEpochMilli();
-            keys = nodes.stream().map(node -> new MeasurementKey(node.hostname(), resource)).collect(Collectors.toList());
+            keys = hostnames.stream().map(hostname -> new MeasurementKey(hostname, resource)).collect(Collectors.toList());
         }
 
         public int measurementCount() {
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java
index 4f320dd8b03..86b4347dea7 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/maintenance/NodeMetricsDbMaintainer.java
@@ -40,7 +40,7 @@ public class NodeMetricsDbMaintainer extends Maintainer {
             try {
                 Collection<NodeMetrics.Metric> metrics = nodeMetrics.fetchMetrics(node.hostname());
                 Instant timestamp = nodeRepository().clock().instant();
-                metrics.forEach(metric -> nodeMetricsDb.add(node, Resource.fromMetric(metric.name()), timestamp, metric.value()));
+                metrics.forEach(metric -> nodeMetricsDb.add(node.hostname(), Resource.fromMetric(metric.name()), timestamp, metric.value()));
             }
             catch (Exception e) {
                 if (warnings++ < maxWarningsPerInvocation)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
index 738e47cc0a1..95d64d167df 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTester.java
@@ -132,7 +132,7 @@ class AutoscalingTester {
             clock().advance(Duration.ofMinutes(1));
             for (Node node : nodes) {
                 for (Resource r : Resource.values())
-                    db.add(node, r, clock().instant(),
+                    db.add(node.hostname(), r, clock().instant(),
                            (r == resource ? value : (float)r.idealAverageLoad() * otherResourcesLoad) * oneExtraNodeFactor);
             }
         }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java
new file mode 100644
index 00000000000..348e1b295f6
--- /dev/null
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/NodeMetricsDbTest.java
@@ -0,0 +1,30 @@
+// 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.provision.autoscale;
+
+import com.yahoo.test.ManualClock;
+import org.junit.Test;
+
+import java.time.Duration;
+import java.util.List;
+
+import static org.junit.Assert.assertEquals;
+
+public class NodeMetricsDbTest {
+
+    @Test
+    public void testNodeMetricsDb() {
+        ManualClock clock = new ManualClock();
+        NodeMetricsDb db = new NodeMetricsDb();
+        for (int i = 0; i < 40; i++) {
+            db.add("host0", Resource.cpu, clock.instant(), 0.9f);
+            clock.advance(Duration.ofHours(1));
+        }
+
+        assertEquals(32, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.cpu,    List.of("host0")).measurementCount());
+        assertEquals( 0, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.memory, List.of("host0")).measurementCount());
+        db.gc(clock);
+        assertEquals(26, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.cpu,    List.of("host0")).measurementCount());
+        assertEquals( 0, db.getWindow(clock.instant().minus(Duration.ofHours(30)), Resource.memory, List.of("host0")).measurementCount());
+    }
+
+}