Correct impossible redundancy

3 files changed, 72 insertions, 2 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
index fc4fddceb66..f7ee09f6dec 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/StorageGroup.java
@@ -203,6 +203,10 @@ public class StorageGroup {
         }
 
         public StorageGroup buildRootGroup(DeployState deployState, RedundancyBuilder redundancyBuilder, ContentCluster owner) {
+            Optional<Integer> maxRedundancy = Optional.empty();
+            if (owner.isHosted())
+                maxRedundancy = validateRedundancyAndGroups();
+
             Optional<ModelElement> group = Optional.ofNullable(clusterElement.child("group"));
             Optional<ModelElement> nodes = getNodes(clusterElement);
 
@@ -215,7 +219,9 @@ public class StorageGroup {
             StorageGroup storageGroup = (owner.isHosted())
                     ? groupBuilder.buildHosted(deployState, owner, Optional.empty())
                     : groupBuilder.buildNonHosted(deployState, owner, Optional.empty());
-            Redundancy redundancy = redundancyBuilder.build(owner.getName(), owner.isHosted(), storageGroup.subgroups.size(), storageGroup.getNumberOfLeafGroups(), storageGroup.countNodes());
+            Redundancy redundancy = redundancyBuilder.build(owner.getName(), owner.isHosted(), storageGroup.subgroups.size(),
+                                                            storageGroup.getNumberOfLeafGroups(), storageGroup.countNodes(),
+                                                            maxRedundancy);
             owner.setRedundancy(redundancy);
             if (storageGroup.partitions.isEmpty() && (redundancy.groups() > 1)) {
                 storageGroup.partitions = Optional.of(computePartitions(redundancy.finalRedundancy(), redundancy.groups()));
@@ -223,6 +229,29 @@ public class StorageGroup {
             return storageGroup;
         }
 
+        private Optional<Integer> validateRedundancyAndGroups() {
+            var redundancyElement = clusterElement.child("redundancy");
+            if (redundancyElement == null) return Optional.empty();
+            long redundancy = redundancyElement.asLong();
+
+            var nodesElement = clusterElement.child("nodes");
+            if (nodesElement == null) return Optional.empty();
+            var nodesSpec = NodesSpecification.from(nodesElement, context);
+
+            int minNodesPerGroup = (int)Math.ceil((double)nodesSpec.minResources().nodes() / nodesSpec.minResources().groups());
+
+            if (minNodesPerGroup < redundancy) { // TODO: Fail on this on Vespa 8, and simplify
+                context.getDeployLogger().log(Level.WARNING,
+                                              "Cluster '" + clusterElement.stringAttribute("id") + "' " +
+                                              "specifies redundancy " + redundancy + " but cannot be higher than " +
+                                              "the minimum nodes per group, which is " + minNodesPerGroup);
+                return Optional.of(minNodesPerGroup);
+            }
+            else {
+                return Optional.empty();
+            }
+        }
+
         /** This returns a partition string which specifies equal distribution between all groups */
         // TODO: Make a partitions object
         static private String computePartitions(int redundancyPerGroup, int numGroups) {
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/RedundancyBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/RedundancyBuilder.java
index 2e97cdabd73..669d4ff6a1d 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/RedundancyBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/RedundancyBuilder.java
@@ -5,6 +5,8 @@ import com.yahoo.vespa.model.builder.xml.dom.ModelElement;
 import com.yahoo.vespa.model.content.IndexedHierarchicDistributionValidator;
 import com.yahoo.vespa.model.content.Redundancy;
 
+import java.util.Optional;
+
 /**
  * Builds redundancy config for a content cluster.
  */
@@ -37,7 +39,13 @@ public class RedundancyBuilder {
             }
         }
     }
-    public Redundancy build(String clusterName, boolean isHosted, int subGroups, int leafGroups,  int totalNodes) {
+    public Redundancy build(String clusterName, boolean isHosted, int subGroups, int leafGroups,  int totalNodes,
+                            Optional<Integer> maxRedundancy) {
+        if (maxRedundancy.isPresent()) {
+            initialRedundancy = Math.min(initialRedundancy, maxRedundancy.get());
+            finalRedundancy = Math.min(finalRedundancy, maxRedundancy.get());
+            readyCopies = Math.min(readyCopies, maxRedundancy.get());
+        }
         if (isHosted) {
             return new Redundancy(initialRedundancy, finalRedundancy, readyCopies, leafGroups, totalNodes);
         } else {
diff --git a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
index 5497564e32f..c6ddc6b0c36 100644
--- a/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
+++ b/config-model/src/test/java/com/yahoo/config/model/provision/ModelProvisioningTest.java
@@ -997,6 +997,39 @@ public class ModelProvisioningTest {
     }
 
     @Test
+    public void testRedundancyWithGroupsTooHighRedundancyAndOneRetiredNode() {
+        String services =
+                "<?xml version='1.0' encoding='utf-8' ?>" +
+                "<services>" +
+                "  <content version='1.0' id='bar'>" +
+                "     <redundancy>2</redundancy>" + // Should have been illegal since we only have 1 node per group
+                "     <documents>" +
+                "       <document type='type1' mode='index'/>" +
+                "     </documents>" +
+                "     <nodes count='2' groups='2'/>" +
+                "  </content>" +
+                "</services>";
+
+        int numberOfHosts = 3;
+        VespaModelTester tester = new VespaModelTester();
+        tester.addHosts(numberOfHosts);
+        VespaModel model = tester.createModel(services, false, "node-1-3-10-03");
+        assertEquals(numberOfHosts, model.getRoot().hostSystem().getHosts().size());
+
+        ContentCluster cluster = model.getContentClusters().get("bar");
+        assertEquals(2, cluster.redundancy().effectiveInitialRedundancy());
+        assertEquals(2, cluster.redundancy().effectiveFinalRedundancy());
+        assertEquals(2, cluster.redundancy().effectiveReadyCopies());
+        assertEquals("1|*", cluster.getRootGroup().getPartitions().get());
+        assertEquals(0, cluster.getRootGroup().getNodes().size());
+        assertEquals(2, cluster.getRootGroup().getSubgroups().size());
+        System.out.println("Nodes in group 0: ");
+        cluster.getRootGroup().getSubgroups().get(0).getNodes().forEach(n -> System.out.println("  " + n));
+        System.out.println("Nodes in group 1: ");
+        cluster.getRootGroup().getSubgroups().get(1).getNodes().forEach(n -> System.out.println("  " + n));
+    }
+
+    @Test
     public void testRedundancyWithGroupsAndThreeRetiredNodes() {
         String services =
                 "<?xml version='1.0' encoding='utf-8' ?>" +