Merge pull request #30985 from vespa-engine/bratseth/group-size-reduction

Bratseth/group size reduction

4 files changed, 44 insertions, 3 deletions

diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
index 8c52f389daf..b149a9af2c2 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeAllocation.java
@@ -179,6 +179,13 @@ class NodeAllocation {
         if (violatesExclusivity(candidate) != NodeCandidate.ExclusivityViolation.NONE) return Retirement.violatesExclusivity;
         if (requiredHostFlavor.isPresent() && ! candidate.parent.map(node -> node.flavor().name()).equals(requiredHostFlavor)) return Retirement.violatesHostFlavor;
         if (candidate.violatesSpares) return Retirement.violatesSpares;
+
+        var group = candidate.allocation().get().membership().cluster().group();
+        if (cluster.isStateful() && group.isPresent() && requested.count().isPresent()) {
+            long nodesInGroup = nodes.values().stream().filter(n -> groupOf(n).equals(group) && ! isRetired(n)).count();
+            if (nodesInGroup >= requested.groupSize())
+                return Retirement.groupSurplus;
+        }
         return Retirement.none;
     }
 
@@ -290,6 +297,10 @@ class NodeAllocation {
         return candidate.allocation().flatMap(a -> a.membership().cluster().group());
     }
 
+    private boolean isRetired(NodeCandidate candidate) {
+        return candidate.allocation().map(a -> a.membership().retired()).orElse(false);
+    }
+
     private Node resize(Node node) {
         NodeResources hostResources = allNodes.parentOf(node).get().flavor().resources();
         return node.with(new Flavor(requested.resources().get()
@@ -463,6 +474,7 @@ class NodeAllocation {
         violatesHostFlavor("node violates host flavor"),
         violatesHostFlavorGeneration("node violates host flavor generation"),
         violatesSpares("node is assigned to a host we want to use as a spare"),
+        groupSurplus("group has enough nodes"),
         none("");
 
         private final String description;
diff --git a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
index d4c4e86f0a3..d8565b81e41 100644
--- a/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
+++ b/node-repository/src/main/java/com/yahoo/vespa/hosted/provision/provisioning/NodeCandidate.java
@@ -55,6 +55,7 @@ public abstract class NodeCandidate implements Nodelike, Comparable<NodeCandidat
     final boolean exclusiveSwitch;
 
     /** True if this node belongs to a group which will not be needed after this deployment */
+    // TODO: Always false
     final boolean isSurplus;
 
     /** This node does not exist in the node repository yet */
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
index abcef421b4c..78a34326949 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/DynamicProvisioningTest.java
@@ -553,6 +553,34 @@ public class DynamicProvisioningTest {
                        2, 1, resources);
     }
 
+    @Test
+    public void split_into_two_groups() {
+        List<Flavor> flavors = List.of(new Flavor("2x",  new NodeResources(2, 20, 200, 0.1, fast, local)));
+
+        ProvisioningTester tester = new ProvisioningTester.Builder().dynamicProvisioning(true, false)
+                                                                    .flavors(flavors)
+                                                                    .hostProvisioner(new MockHostProvisioner(flavors))
+                                                                    .nameResolver(nameResolver)
+                                                                    .build();
+
+        tester.activateTenantHosts();
+
+        ApplicationId app1 = applicationId("app1");
+        ClusterSpec cluster1 = ClusterSpec.request(content, new ClusterSpec.Id("cluster1")).vespaVersion("8").build();
+
+        System.out.println("Initial deployment ----------------------");
+        tester.activate(app1, cluster1, Capacity.from(resources(6, 1, 2, 20, 200, fast, StorageType.any)));
+        tester.assertNodes("Initial deployment: 1 group",
+                           6, 1, 2, 20, 200, fast, remote, app1, cluster1);
+
+        System.out.println("Split into 2 groups ---------------------");
+        tester.activate(app1, cluster1, Capacity.from(resources(6, 2, 2, 20, 200, fast, StorageType.any)));
+        tester.assertNodes("Change to 2 groups: Gets 6 active non-retired nodes",
+                           6, 2, 2, 20, 200, fast, remote, app1, cluster1);
+        List<Node> retired = tester.nodeRepository().nodes().list().owner(app1).cluster(cluster1.id()).state(Node.State.active).retired().asList();
+        assertEquals("... and in addition 3 retired nodes", 3, retired.size());
+    }
+
     private ProvisioningTester tester(boolean sharing) {
         var hostProvisioner = new MockHostProvisioner(new NodeFlavors(ProvisioningTester.createConfig()).getFlavors(), nameResolver, 0);
         return new ProvisioningTester.Builder().dynamicProvisioning(true, sharing).hostProvisioner(hostProvisioner).nameResolver(nameResolver).build();
diff --git a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
index 5f2790e886a..7b690b880c2 100644
--- a/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
+++ b/node-repository/src/test/java/com/yahoo/vespa/hosted/provision/provisioning/ProvisioningTest.java
@@ -288,13 +288,13 @@ public class ProvisioningTest {
         assertEquals("Superfluous container nodes are also dirtyed",
                      4-2 + 5-2 + 1 + 4-2, tester.nodeRepository().nodes().list(Node.State.dirty).size());
         assertEquals("Superfluous content nodes are retired",
-                     5-3 + 6-3 - 1, tester.getNodes(application1, Node.State.active).retired().size());
+                     5-3 + 6-3 -1, tester.getNodes(application1, Node.State.active).retired().size());
 
         // increase content slightly
         SystemState state6 = prepare(application1, 2, 2, 4, 3, defaultResources, tester);
         tester.activate(application1, state6.allHosts);
         assertEquals("One content node is unretired",
-                     5-4 + 6-3 - 1, tester.getNodes(application1, Node.State.active).retired().size());
+                     5-4 + 6-3 -1, tester.getNodes(application1, Node.State.active).retired().size());
 
         // Then reserve more
         SystemState state7 = prepare(application1, 8, 2, 2, 2, defaultResources, tester);
@@ -505,7 +505,7 @@ public class ProvisioningTest {
         ProvisioningTester tester = new ProvisioningTester.Builder().zone(new Zone(Environment.prod, RegionName.from("us-east")))
                                                                     .flavors(List.of(hostFlavor))
                                                                     .build();
-        tester.makeReadyHosts(31, hostFlavor.resources()).activateTenantHosts();
+        tester.makeReadyHosts(32, hostFlavor.resources()).activateTenantHosts();
 
         ApplicationId app1 = ProvisioningTester.applicationId("app1");
         ClusterSpec cluster1 = ClusterSpec.request(ClusterSpec.Type.content, new ClusterSpec.Id("cluster1")).vespaVersion("7").build();