Cooldown from completion rather than start of scaling

6 files changed, 42 insertions, 15 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java
index 281d9efe51a..2f9ad28a072 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/ClusterModel.java
@@ -243,8 +243,10 @@ public class ClusterModel {
     }
 
     private boolean hasScaledIn(Duration period) {
-        return cluster.lastScalingEvent().map(event -> event.at()).orElse(Instant.MIN)
-                      .isAfter(clock.instant().minus(period));
+        if (cluster.lastScalingEvent().isEmpty()) return false;
+        var lastCompletion = cluster.lastScalingEvent().get().completion();
+        if (lastCompletion.isEmpty()) return true; // Ongoing
+        return lastCompletion.get().isAfter(clock.instant().minus(period));
     }
 
     private ClusterNodesTimeseries nodeTimeseries() { return nodeTimeseries; }
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
index cbee7437d5d..f1cf33d0477 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/AutoscalingTest.java
@@ -77,6 +77,11 @@ public class AutoscalingTest {
 
         fixture.tester().clock().advance(Duration.ofDays(2));
         fixture.loader().applyCpuLoad(0.1f, 10);
+        assertTrue("Last scaling not completed", fixture.autoscale().resources().isEmpty());
+
+        fixture.completeLastScaling();
+        fixture.tester().clock().advance(Duration.ofDays(7));
+        fixture.loader().applyCpuLoad(0.1f, 10);
         fixture.tester().assertResources("Scaling cpu down since usage has gone down significantly",
                                          8, 1, 1.0, 7.3, 22.1,
                                          fixture.autoscale());
@@ -243,14 +248,15 @@ public class AutoscalingTest {
         var fixture = DynamicProvisioningTester.fixture().awsProdSetup(true).clusterType(ClusterSpec.Type.container).build();
 
         fixture.loader().applyCpuLoad(0.25f, 120);
-        ClusterResources scaledResources = fixture.tester().assertResources("Scaling cpu up",
-                                                                  4, 1, 4,  16.0, 40.8,
-                                                                  fixture.autoscale());
+        var scaledResources = fixture.tester().assertResources("Scaling cpu up",
+                                                               3, 1, 4,  16.0, 40.8,
+                                                               fixture.autoscale());
         fixture.deploy(Capacity.from(scaledResources));
         fixture.deactivateRetired(Capacity.from(scaledResources));
+        fixture.completeLastScaling();
         fixture.loader().applyCpuLoad(0.1f, 120);
         fixture.tester().assertResources("Scaling down since cpu usage has gone down",
-                                         3, 1, 4, 16, 30.6,
+                                         3, 1, 2, 16, 27.2,
                                          fixture.autoscale());
     }
 
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java
index 48db3fade95..5d1fd58489b 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/autoscale/Fixture.java
@@ -55,6 +55,7 @@ public class Fixture {
         var deployCapacity = initialResources.isPresent() ? Capacity.from(initialResources.get()) : capacity;
         tester.deploy(builder.application, builder.cluster, deployCapacity);
         this.loader = new Loader(this);
+        store(cluster().with(cluster().lastScalingEvent().get().withCompletion(tester.clock().instant())));
     }
 
     public DynamicProvisioningTester tester() { return tester; }
@@ -141,6 +142,16 @@ public class Fixture {
         tester.nodeRepository().applications().put(application, tester.nodeRepository().applications().lock(applicationId));
     }
 
+    public void store(Cluster cluster) {
+        var application = application();
+        application = application.with(cluster);
+        tester.nodeRepository().applications().put(application, tester.nodeRepository().applications().lock(applicationId));
+    }
+
+    public void completeLastScaling() {
+        store(cluster().with(cluster().lastScalingEvent().get().withCompletion(tester().clock().instant())));
+    }
+
     public static class Builder {
 
         ApplicationId application = DynamicProvisioningTester.applicationId("application1");
@@ -162,7 +173,7 @@ public class Fixture {
         }
 
         /** Set to true to behave as if hosts are provisioned dynamically. */
-        public Fixture. Builder dynamicProvisioning(boolean dynamicProvisioning) {
+        public Fixture.Builder dynamicProvisioning(boolean dynamicProvisioning) {
             this.zone = new Zone(Cloud.builder()
                                       .dynamicProvisioning(dynamicProvisioning)
                                       .allowHostSharing(zone.cloud().allowHostSharing())
@@ -174,7 +185,7 @@ public class Fixture {
         }
 
         /** Set to true to allow multiple nodes be provisioned on the same host. */
-        public Fixture. Builder allowHostSharing(boolean allowHostSharing) {
+        public Fixture.Builder allowHostSharing(boolean allowHostSharing) {
             this.zone = new Zone(Cloud.builder()
                                       .dynamicProvisioning(zone.cloud().dynamicProvisioning())
                                       .allowHostSharing(allowHostSharing)
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java
index 2786da4b69e..e6c183d02ce 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/maintenance/ScalingSuggestionsMaintainerTest.java
@@ -58,10 +58,11 @@ public class ScalingSuggestionsMaintainerTest {
         tester.deploy(app1, cluster1, Capacity.from(new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)),
                                                     new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)),
                                                     IntRange.empty(), false, true, Optional.empty(), ClusterInfo.empty()));
+        storeCompletion(app1, cluster1.id(), tester.nodeRepository());
         tester.deploy(app2, cluster2, Capacity.from(new ClusterResources(5, 1, new NodeResources(4, 4, 10, 0.1)),
                                                     new ClusterResources(10, 1, new NodeResources(6.5, 5, 15, 0.1)),
                                                     IntRange.empty(), false, true, Optional.empty(), ClusterInfo.empty()));
-
+        storeCompletion(app2, cluster2.id(), tester.nodeRepository());
         tester.clock().advance(Duration.ofHours(13));
         Duration timeAdded = addMeasurements(0.90f, 0.90f, 0.90f, 0, 500, app1, tester.nodeRepository());
         tester.clock().advance(timeAdded.negated());
@@ -109,6 +110,16 @@ public class ScalingSuggestionsMaintainerTest {
         assertFalse("Suggestion is not made as it matches what we have", shouldSuggest(app1, cluster1, tester));
     }
 
+    private void storeCompletion(ApplicationId appId, ClusterSpec.Id clusterId, NodeRepository nodeRepository) {
+        try (var lock = nodeRepository.applications().lock(appId)) {
+            var app = nodeRepository.applications().require(appId);
+            var cluster = app.cluster(clusterId).get();
+            cluster = cluster.with(cluster.lastScalingEvent().get().withCompletion(nodeRepository.clock().instant()));
+            app = app.with(cluster);
+            nodeRepository.applications().put(app, lock);
+        }
+    }
+
     private Autoscaling suggestionOf(ApplicationId app, ClusterSpec cluster, ProvisioningTester tester) {
         return tester.nodeRepository().applications().get(app).get().cluster(cluster.id()).get().suggested();
     }
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java
index a3fc6aae9ac..92aa65eac99 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTForestOptimizer.java
@@ -48,9 +48,8 @@ public class GBDTForestOptimizer extends Optimizer {
      */
     private ExpressionNode findAndOptimize(ExpressionNode node) {
         ExpressionNode newNode = optimize(node);
-        if ( ! (newNode instanceof CompositeNode)) return newNode; //
+        if ( ! (newNode instanceof CompositeNode newComposite)) return newNode;
 
-        CompositeNode newComposite = (CompositeNode)newNode;
         List<ExpressionNode> newChildren = new ArrayList<>();
         for (ExpressionNode child : newComposite.children()) {
             newChildren.add(findAndOptimize(child));
@@ -84,10 +83,9 @@ public class GBDTForestOptimizer extends Optimizer {
             currentTreesOptimized++;
             return true;
         }
-        if (!(node instanceof OperationNode)) {
+        if (!(node instanceof OperationNode aNode)) {
             return false;
         }
-        OperationNode aNode = (OperationNode)node;
         for (Operator op : aNode.operators()) {
             if (op != Operator.plus) {
                 return false;
diff --git a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java
index 7ba671e62eb..eb79c8ba2c1 100644
--- a/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java
+++ b/searchlib/src/main/java/com/yahoo/searchlib/rankingexpression/evaluation/gbdtoptimization/GBDTOptimizer.java
@@ -95,8 +95,7 @@ public class GBDTOptimizer extends Optimizer {
      */
     private int consumeNode(ExpressionNode node, List<Double> values, ContextIndex context) {
         int beforeIndex = values.size();
-        if ( node instanceof IfNode) {
-            IfNode ifNode = (IfNode)node;
+        if (node instanceof IfNode ifNode) {
             int jumpValueIndex = consumeIfCondition(ifNode.getCondition(), values, context);
             values.add(0d); // jumpValue goes here after the next line
             int jumpValue = consumeNode(ifNode.getTrueExpression(), values, context) + 1;