Scale *down* only if it saves cost meaningfully

3 files changed, 17 insertions, 10 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java
index 344b2402e26..5478999e4fe 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/applications/Cluster.java
@@ -82,7 +82,7 @@ public class Cluster {
     public boolean shouldSuggestResources(ClusterResources currentResources) {
         if (suggested.isEmpty()) return false;
         if (suggested.get().resources().isWithin(min, max)) return false;
-        if (Autoscaler.similar(suggested.get().resources(), currentResources)) return false;
+        if ( ! Autoscaler.worthRescaling(currentResources, suggested.get().resources())) return false;
         return true;
     }
 
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 d56d352a765..dcfb8fb7246 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
@@ -93,7 +93,7 @@ public class Autoscaler {
         if (bestAllocation.isEmpty())
             return Advice.dontScale(Status.insufficient, "No allocations are possible within configured limits");
 
-        if (similar(bestAllocation.get().realResources(), currentAllocation.realResources())) {
+        if (! worthRescaling(currentAllocation.realResources(), bestAllocation.get().realResources())) {
             if (bestAllocation.get().fulfilment() < 1)
                 return Advice.dontScale(Status.insufficient, "Configured limits prevents better scaling of this cluster");
             else
@@ -122,12 +122,19 @@ public class Autoscaler {
         return true;
     }
 
-    /** Returns true if both total real resources and total cost are similar */
-    public static boolean similar(ClusterResources a, ClusterResources b) {
-        return similar(a.cost(), b.cost(), costDifferenceWorthReallocation) &&
-               similar(a.totalResources().vcpu(), b.totalResources().vcpu(), resourceDifferenceWorthReallocation) &&
-               similar(a.totalResources().memoryGb(), b.totalResources().memoryGb(), resourceDifferenceWorthReallocation) &&
-               similar(a.totalResources().diskGb(), b.totalResources().diskGb(), resourceDifferenceWorthReallocation);
+    /** Returns true if it is worthwhile to make the given resource change, false if it is too insignificant */
+    public static boolean worthRescaling(ClusterResources from, ClusterResources to) {
+        // *Increase* if needed with no regard for cost difference to prevent running out of a resource
+        if (meaningfulIncrease(from.totalResources().vcpu(), to.totalResources().vcpu())) return true;
+        if (meaningfulIncrease(from.totalResources().memoryGb(), to.totalResources().memoryGb())) return true;
+        if (meaningfulIncrease(from.totalResources().diskGb(), to.totalResources().diskGb())) return true;
+
+        // Otherwise, only *decrease* if it reduces cost meaningfully
+        return ! similar(from.cost(), to.cost(), costDifferenceWorthReallocation);
+    }
+
+    private static boolean meaningfulIncrease(double from, double to) {
+        return from < to && ! similar(from, to, resourceDifferenceWorthReallocation);
     }
 
     private static boolean similar(double r1, double r2, double threshold) {
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 fc477c8187c..a3c7b7d2d2b 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
@@ -690,11 +690,11 @@ public class AutoscalingTest {
                                5, 1, 9.5,  100, 100,
                                tester.autoscale(application1, cluster1.id(), min, max).target());
 
-        tester.addCpuMeasurements(0.4f, 1f, 100, application1);
+        tester.addCpuMeasurements(0.3f, 1f, 100, application1);
         tester.clock().advance(Duration.ofMinutes(-100 * 5));
         tester.addLoadMeasurements(application1, cluster1.id(), 100, t -> t == 0 ? 20.0 : 10.0, t -> 100.0);
         tester.assertResources("Write load is 10x query load -> scale down",
-                               5, 1, 3.8,  100, 100,
+                               5, 1, 2.9,  100, 100,
                                tester.autoscale(application1, cluster1.id(), min, max).target());
 
         tester.addCpuMeasurements(0.4f, 1f, 100, application1);