Merge pull request #32412 from vespa-engine/hmusum/validate-redundanc…

…y-using-effective-final-redundancy Change how we validate redundancy changes
vespa-engine · Sep 17, 2024 · 99308e3 · 99308e3
2 parents 8287265 + 7c92aa0
commit 99308e3
Show file tree

Hide file tree

Showing 2 changed files with 15 additions and 9 deletions.
diff --git a/...java/com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidator.java b/...java/com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidator.java
@@ -29,8 +29,6 @@ public void validate(ChangeContext context) {
         }
     }
 
-    private int redundancyOf(ContentCluster cluster) {
-        return cluster.getRedundancy().finalRedundancy();
-    }
+    private int redundancyOf(ContentCluster cluster) { return cluster.getRedundancy().effectiveFinalRedundancy(); }
 
 }
diff --git a/.../com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidatorTest.java b/.../com/yahoo/vespa/model/application/validation/change/RedundancyIncreaseValidatorTest.java
@@ -21,9 +21,9 @@ public class RedundancyIncreaseValidatorTest {
 
     @Test
     void testRedundancyIncreaseValidation() {
-        VespaModel previous = tester.deploy(null, getServices(2), Environment.prod, null, "contentClusterId.indexing").getFirst();
+        VespaModel previous = tester.deploy(null, getServices(2, 3, 1), Environment.prod, null, "contentClusterId.indexing").getFirst();
         try {
-            tester.deploy(previous, getServices(3), Environment.prod, null, "contentClusterId.indexing");
+            tester.deploy(previous, getServices(3, 3, 1), Environment.prod, null, "contentClusterId.indexing");
             fail("Expected exception due to redundancy increase");
         }
         catch (IllegalArgumentException expected) {
@@ -35,13 +35,21 @@ void testRedundancyIncreaseValidation() {
         }
     }
 
+    @Test
+    void testRedundancyIncreaseValidationWithGroups() {
+        // Changing redundancy from 1 to 2 is allowed when having 2 nodes in 2 groups versus 3 nodes in 1 group
+        // (effective redundancy for the cluster is 2 in both cases)
+        VespaModel previous = tester.deploy(null, getServices(1, 2, 2), Environment.prod, null, "contentClusterId.indexing").getFirst();
+        tester.deploy(previous, getServices(2, 3, 1), Environment.prod, null, "contentClusterId.indexing");
+    }
+
     @Test
     void testOverridingContentRemovalValidation() {
-        VespaModel previous = tester.deploy(null, getServices(2), Environment.prod, null, "contentClusterId.indexing").getFirst();
-        tester.deploy(previous, getServices(3), Environment.prod, redundancyIncreaseOverride, "contentClusterId.indexing"); // Allowed due to override
+        VespaModel previous = tester.deploy(null, getServices(2, 3, 1), Environment.prod, null, "contentClusterId.indexing").getFirst();
+        tester.deploy(previous, getServices(3, 3, 1), Environment.prod, redundancyIncreaseOverride, "contentClusterId.indexing"); // Allowed due to override
     }
 
-    private static String getServices(int redundancy) {
+    private static String getServices(int redundancy, int nodes, int groups) {
         return "<services version='1.0'>" +
                "  <content id='contentClusterId' version='1.0'>" +
                "    <redundancy>" + redundancy + "</redundancy>" +
@@ -51,7 +59,7 @@ private static String getServices(int redundancy) {
                "    <documents>" +
                "      <document type='music' mode='index'/>" +
                "    </documents>" +
-               "    <nodes count='3'/>" +
+               "    <nodes count='" + nodes + "' groups='" + groups + "'/>" +
                "   </content>" +
                "</services>";
     }