Prepare to iterate over groups

2 files changed, 30 insertions, 8 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
index aa778700941..a556dcf9ced 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/autoscale/AllocationOptimizer.java
@@ -47,23 +47,27 @@ public class AllocationOptimizer {
         int nodeIncrement = singleGroupMode ? 1 : current.groupSize();
 
         Optional<AllocatableClusterResources> bestAllocation = Optional.empty();
+
         for (int nodes = minNodes(limits, singleGroupMode, current); nodes <= maxNodes(limits, singleGroupMode, current); nodes += nodeIncrement) {
-            int nodesAdjustedForRedundancy = nodes;
-            if (target.adjustForRedundancy())
-                nodesAdjustedForRedundancy = nodes - (singleGroupMode ? 1 : current.groupSize());
-            if (nodesAdjustedForRedundancy < 1) continue;
-            int groups = singleGroupMode ? 1 : nodesAdjustedForRedundancy / current.groupSize();
-            System.out.println("nodes: " + nodesAdjustedForRedundancy + " singleGrouopMode: " + singleGroupMode + " current.groupSize " + current.groupSize() + " groups: " + groups);
+            int groups = singleGroupMode ? 1 : nodes / current.groupSize();
             if ( ! limits.isEmpty() && ( groups < limits.min().groups() || groups > limits.max().groups())) continue;
+
+            // Adjust for redundancy: Node in group if groups = 1, an extra group if multiple groups
+            // TODO: Make the best choice based on size and redundancy setting instead
+            int nodesAdjustedForRedundancy = target.adjustForRedundancy() ? nodes - nodeIncrement : nodes;
+            if (nodesAdjustedForRedundancy < 1) continue;
+            int groupsAdjustedForRedundancy = target.adjustForRedundancy() ? ( groups == 1 ? 1 : groups - 1 ) : groups;
+
             ClusterResources next = new ClusterResources(nodes,
-                                                         singleGroupMode ? 1 : nodes / current.groupSize(),
-                                                         nodeResourcesWith(nodesAdjustedForRedundancy, groups, limits, current, target));
+                                                         groups,
+                                                         nodeResourcesWith(nodesAdjustedForRedundancy, groupsAdjustedForRedundancy, limits, current, target));
 
             var allocatableResources = AllocatableClusterResources.from(next, current.clusterType(), limits, nodeRepository);
             if (allocatableResources.isEmpty()) continue;
             if (bestAllocation.isEmpty() || allocatableResources.get().preferableTo(bestAllocation.get()))
                 bestAllocation = allocatableResources;
         }
+
         return bestAllocation;
     }
 
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 9979313f3d7..8721eabf5e4 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
@@ -254,6 +254,24 @@ public class AutoscalingTest {
     }
 
     @Test
+    public void testAutoscalingGroupSize() {
+        NodeResources resources = new NodeResources(3, 100, 100, 1);
+        ClusterResources min = new ClusterResources( 3, 2, new NodeResources(1, 1, 1, 1));
+        ClusterResources max = new ClusterResources(30, 10, new NodeResources(100, 1000, 1000, 1));
+        AutoscalingTester tester = new AutoscalingTester(resources);
+
+        ApplicationId application1 = tester.applicationId("application1");
+        ClusterSpec cluster1 = tester.clusterSpec(ClusterSpec.Type.container, "cluster1");
+
+        // deploy
+        tester.deploy(application1, cluster1, 6, 2, resources);
+        tester.addMeasurements(Resource.memory,  0.9f, 1f, 120, application1);
+        tester.assertResources("Should increase cluster size to reduce memory usage",
+                               9, 3, 2.7,  83.3, 83.3,
+                               tester.autoscale(application1, cluster1.id(), min, max));
+    }
+
+    @Test
     public void testAutoscalingAvoidsIllegalConfigurations() {
         NodeResources resources = new NodeResources(3, 100, 100, 1);
         ClusterResources min = new ClusterResources( 2, 1, new NodeResources(1, 1, 1, 1));