Validate copies per node to account for group size changes

vespa-engine · Sep 17, 2024 · 85fecf8 · 85fecf8
1 parent 01348b1
commit 85fecf8
Show file tree

Hide file tree

Showing 2 changed files with 25 additions and 7 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
@@ -18,7 +18,7 @@ public void validate(ChangeContext context) {
  for (ContentCluster currentCluster : context.previousModel().getContentClusters().values()) {
  ContentCluster nextCluster = context.model().getContentClusters().get(currentCluster.getSubId());
  if (nextCluster == null) continue;
- if (redundancyOf(nextCluster) > redundancyOf(currentCluster)) {
+ if (copiesPerNode(nextCluster) >= 1.5 * copiesPerNode(currentCluster)) {
  context.invalid(ValidationId.redundancyIncrease,
  "Increasing redundancy from " + redundancyOf(currentCluster) + " to " +
  redundancyOf(nextCluster) + " in '" + currentCluster + ". " +
@@ -33,4 +33,16 @@ private int redundancyOf(ContentCluster cluster) {
  return cluster.getRedundancy().finalRedundancy();
  }
 
+ private int groupsOf(ContentCluster cluster) {
+ return cluster.getRootGroup().getNumberOfLeafGroups();
+ }
+
+ private int nodesOf(ContentCluster cluster) {
+ return cluster.getRootGroup().countNodes(false);
+ }
+
+ private double copiesPerNode(ContentCluster cluster) {
+ return redundancyOf(cluster) * groupsOf(cluster) / (double)nodesOf(cluster);
+ }
+
 }
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,19 @@ void testRedundancyIncreaseValidation() {
  }
  }
 
+ @Test
+ void testRedundancyIncreaseValidationDoesNotTriggerWhenTotalCopiesAreNotIncreased() {
+ 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 +57,7 @@ private static String getServices(int redundancy) {
  " <documents>" +
  " <document type='music' mode='index'/>" +
  " </documents>" +
- " <nodes count='3'/>" +
+ " <nodes count='" + nodes + "' groups='" + groups + "'/>" +
  " </content>" +
  "</services>";
  }