Implicitly enable multiple bucket spaces in config model

Removes validation of redundancy and searchable-copies for
global documents, as these will now be globally distributed by
default.

Experimental override is still present to allow for forced enabling
in upgrade scenarios.

4 files changed, 23 insertions, 136 deletions

diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/GlobalDistributionValidator.java b/config-model/src/main/java/com/yahoo/vespa/model/content/GlobalDistributionValidator.java
index 4bdef0607a2..0661ef67131 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/GlobalDistributionValidator.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/GlobalDistributionValidator.java
@@ -21,45 +21,11 @@ import static java.util.stream.Collectors.toSet;
 public class GlobalDistributionValidator {
 
     public void validate(Map<String, NewDocumentType> documentDefinitions,
-                         Set<NewDocumentType> globallyDistributedDocuments,
-                         Redundancy redundancy,
-                         boolean enableMultipleBucketSpaces) {
-        if (!enableMultipleBucketSpaces) {
-            verifyGlobalDocumentsHaveRequiredRedundancy(globallyDistributedDocuments, redundancy);
-            verifySearchableCopiesIsSameAsRedundancy(globallyDistributedDocuments, redundancy);
-        }
+                         Set<NewDocumentType> globallyDistributedDocuments) {
         verifyReferredDocumentsArePresent(documentDefinitions);
         verifyReferredDocumentsAreGlobal(documentDefinitions, globallyDistributedDocuments);
     }
 
-    private static void verifyGlobalDocumentsHaveRequiredRedundancy(Set<NewDocumentType> globallyDistributedDocuments,
-                                                                    Redundancy redundancy) {
-        if (!globallyDistributedDocuments.isEmpty() && !redundancy.isEffectivelyGloballyDistributed()) {
-            throw new IllegalArgumentException(
-                    String.format(
-                            "The following document types are marked as global, " +
-                                    "but do not have high enough redundancy to make the documents globally distributed: %s. " +
-                                    "Redundancy is %d, expected %d.",
-                            asPrintableString(toDocumentNameStream(globallyDistributedDocuments)),
-                            redundancy.effectiveFinalRedundancy(),
-                            redundancy.totalNodes()));
-        }
-    }
-
-    private static void verifySearchableCopiesIsSameAsRedundancy(Set<NewDocumentType> globallyDistributedDocuments,
-                                                                 Redundancy redundancy) {
-        if (!globallyDistributedDocuments.isEmpty() &&
-                redundancy.effectiveReadyCopies() != redundancy.effectiveFinalRedundancy()) {
-            throw new IllegalArgumentException(
-                    String.format(
-                            "The following document types have the number of searchable copies less than redundancy: %s. " +
-                                    "Searchable copies is %d, while redundancy is %d.",
-                            asPrintableString(toDocumentNameStream(globallyDistributedDocuments)),
-                            redundancy.effectiveReadyCopies(),
-                            redundancy.effectiveFinalRedundancy()));
-        }
-    }
-
     private static void verifyReferredDocumentsArePresent(Map<String, NewDocumentType> documentDefinitions) {
         Set<NewDocumentType.Name> unknowDocuments = getReferencedDocuments(documentDefinitions)
                 .filter(name -> !documentDefinitions.containsKey(name.toString()))
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
index 0119bced095..68ae4d2b242 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/content/cluster/ContentCluster.java
@@ -61,14 +61,12 @@ public class ContentCluster extends AbstractConfigProducer implements
                                                            MessagetyperouteselectorpolicyConfig.Producer,
                                                            BucketspacesConfig.Producer {
 
-    // TODO: Make private
     private String documentSelection;
     private ContentSearchCluster search;
     private final boolean isHostedVespa;
     private final Map<String, NewDocumentType> documentDefinitions;
     private final Set<NewDocumentType> globallyDistributedDocuments;
-    // Experimental flag (TODO: remove when feature is enabled by default)
-    private boolean enableMultipleBucketSpaces = false;
+    private boolean forceEnableMultipleBucketSpaces = false;
     private com.yahoo.vespa.model.content.StorageGroup rootGroup;
     private StorageCluster storageNodes;
     private DistributorCluster distributorNodes;
@@ -250,7 +248,7 @@ public class ContentCluster extends AbstractConfigProducer implements
         private void setupExperimental(ContentCluster cluster, ModelElement experimental) {
             Boolean enableMultipleBucketSpaces = experimental.childAsBoolean("enable-multiple-bucket-spaces");
             if (enableMultipleBucketSpaces != null) {
-                cluster.enableMultipleBucketSpaces = enableMultipleBucketSpaces;
+                cluster.forceEnableMultipleBucketSpaces = enableMultipleBucketSpaces;
             }
         }
 
@@ -596,13 +594,13 @@ public class ContentCluster extends AbstractConfigProducer implements
             builder.min_distributor_up_ratio(0);
             builder.min_storage_up_ratio(0);
         }
-        builder.enable_multiple_bucket_spaces(enableMultipleBucketSpaces);
+        builder.enable_multiple_bucket_spaces(true);
         // Telling the controller whether we actually _have_ global document types lets
         // it selectively enable or disable constraints that aren't needed when these
         // are not are present, even if full protocol and backend support is enabled
         // for multiple bucket spaces. Basically, if you don't use it, you don't
         // pay for it.
-        builder.cluster_has_global_document_types(enableMultipleBucketSpaces && !globallyDistributedDocuments.isEmpty());
+        builder.cluster_has_global_document_types(!globallyDistributedDocuments.isEmpty());
     }
 
     @Override
@@ -646,7 +644,7 @@ public class ContentCluster extends AbstractConfigProducer implements
             }
         }
         new ReservedDocumentTypeNameValidator().validate(documentDefinitions);
-        new GlobalDistributionValidator().validate(documentDefinitions, globallyDistributedDocuments, redundancy, enableMultipleBucketSpaces);
+        new GlobalDistributionValidator().validate(documentDefinitions, globallyDistributedDocuments);
     }
 
     public static Map<String, Integer> METRIC_INDEX_MAP = new TreeMap<>();
@@ -727,11 +725,13 @@ public class ContentCluster extends AbstractConfigProducer implements
         for (NewDocumentType docType : getDocumentDefinitions().values()) {
             BucketspacesConfig.Documenttype.Builder docTypeBuilder = new BucketspacesConfig.Documenttype.Builder();
             docTypeBuilder.name(docType.getName());
-            String bucketSpace = ((enableMultipleBucketSpaces && isGloballyDistributed(docType))
-                    ? GLOBAL_BUCKET_SPACE : DEFAULT_BUCKET_SPACE);
+            String bucketSpace = (isGloballyDistributed(docType) ? GLOBAL_BUCKET_SPACE : DEFAULT_BUCKET_SPACE);
             docTypeBuilder.bucketspace(bucketSpace);
             builder.documenttype(docTypeBuilder);
         }
-        builder.enable_multiple_bucket_spaces(enableMultipleBucketSpaces);
+        // NOTE: this config is kept around to allow the use of multiple bucket spaces
+        // on older versions of Vespa. It is for all intents and purposes a no-op in
+        // newer versions where multiple bucket spaces are enabled by default.
+        builder.enable_multiple_bucket_spaces(forceEnableMultipleBucketSpaces);
     }
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java
index 98177b4ada0..0156128f7ca 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/ContentSearchClusterTest.java
@@ -173,17 +173,17 @@ public class ContentSearchClusterTest {
     }
 
     @Test
-    public void require_that_all_document_types_belong_to_default_bucket_space_by_default() throws Exception {
+    public void require_that_document_types_belong_to_correct_bucket_spaces() throws Exception {
         BucketspacesConfig config = getBucketspacesConfig(createClusterWithGlobalType());
         assertEquals(2, config.documenttype().size());
-        assertDocumentType("global", "default", config.documenttype(0));
+        assertDocumentType("global", "global", config.documenttype(0));
         assertDocumentType("regular", "default", config.documenttype(1));
         // Safeguard against flipping the switch
         assertFalse(config.enable_multiple_bucket_spaces());
     }
 
     @Test
-    public void require_that_multiple_bucket_spaces_can_be_enabled() throws Exception {
+    public void require_that_multiple_bucket_spaces_can_be_force_enabled() throws Exception {
         ContentCluster cluster = createClusterWithMultipleBucketSpacesEnabled();
         {
             BucketspacesConfig config = getBucketspacesConfig(cluster);
@@ -210,9 +210,9 @@ public class ContentSearchClusterTest {
     }
 
     @Test
-    public void controller_global_documents_config_forced_to_false_if_multiple_spaces_not_enabled() throws Exception {
+    public void controller_global_documents_config_always_enabled_even_without_experimental_flag_set() throws Exception {
         ContentCluster cluster = createClusterWithGlobalDocsButNotMultipleSpacesEnabled();
-        assertFalse(getFleetcontrollerConfig(cluster).cluster_has_global_document_types());
+        assertTrue(getFleetcontrollerConfig(cluster).cluster_has_global_document_types());
     }
 
 }
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/content/GlobalDistributionValidatorTest.java b/config-model/src/test/java/com/yahoo/vespa/model/content/GlobalDistributionValidatorTest.java
index b8252f2f081..6506f7a08a8 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/content/GlobalDistributionValidatorTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/content/GlobalDistributionValidatorTest.java
@@ -26,64 +26,16 @@ public class GlobalDistributionValidatorTest {
     public final ExpectedException exceptionRule = ExpectedException.none();
 
     @Test
-    public void throws_exception_if_redudancy_does_not_imply_global_distribution() {
-        Fixture fixture = new Fixture()
-                .addGlobalDocument(createDocumentType("foo"))
-                .addGlobalDocument(createDocumentType("bar"));
-        Redundancy redundancy = createRedundancyWithoutGlobalDistribution();
-
-        exceptionRule.expect(IllegalArgumentException.class);
-        exceptionRule.expectMessage(
-                "The following document types are marked as global, " +
-                        "but do not have high enough redundancy to make the documents globally distributed: " +
-                        "'bar', 'foo'. Redundancy is 2, expected 3.");
-        validate(fixture, redundancy);
-    }
-
-    @Test
-    public void validation_of_redundancy_is_deactivated_if_multiple_bucket_spaces_is_enabled() {
-        Fixture fixture = new Fixture()
-                .addGlobalDocument(createDocumentType("foo"))
-                .addGlobalDocument(createDocumentType("bar"));
-        Redundancy redundancy = createRedundancyWithoutGlobalDistributionAndTooFewSearchableCopies();
-
-        validate(fixture, redundancy, true);
-    }
-
-    @Test
-    public void throws_exception_if_searchable_copies_too_low() {
-        Fixture fixture = new Fixture()
-                .addGlobalDocument(createDocumentType("foo"))
-                .addGlobalDocument(createDocumentType("bar"));
-        Redundancy redundancy = createRedundancyWithTooFewSearchableCopies();
-
-        exceptionRule.expect(IllegalArgumentException.class);
-        exceptionRule.expectMessage(
-                "The following document types have the number of searchable copies less than redundancy: " +
-                "'bar', 'foo'. Searchable copies is 1, while redundancy is 2.");
-        validate(fixture, redundancy);
-    }
-
-    @Test
-    public void validation_succeeds_when_globally_distributed_and_enough_searchable_copies() {
-        Fixture fixture = new Fixture()
-                .addGlobalDocument(createDocumentType("foo"));
-        Redundancy redundancy = createRedundancyWithGlobalDistribution();
-        validate(fixture, redundancy);
-    }
-
-    @Test
     public void validation_succeeds_on_no_documents() {
         new GlobalDistributionValidator()
-                .validate(emptyMap(), emptySet(), createRedundancyWithoutGlobalDistribution(), false);
+                .validate(emptyMap(), emptySet());
     }
 
     @Test
     public void validation_succeeds_on_no_global_documents() {
         Fixture fixture = new Fixture()
                 .addNonGlobalDocument(createDocumentType("foo"));
-        Redundancy redundancy = createRedundancyWithoutGlobalDistribution();
-        validate(fixture, redundancy);
+        validate(fixture);
     }
 
     @Test
@@ -92,11 +44,10 @@ public class GlobalDistributionValidatorTest {
         Fixture fixture = new Fixture()
                 .addNonGlobalDocument(parent)
                 .addNonGlobalDocument(createDocumentType("child", parent));
-        Redundancy redundancy = createRedundancyWithoutGlobalDistribution();
         exceptionRule.expect(IllegalArgumentException.class);
         exceptionRule.expectMessage(
                 "The following document types are referenced from other documents, but are not globally distributed: 'parent'");
-        validate(fixture, redundancy);
+        validate(fixture);
     }
 
     @Test
@@ -105,8 +56,7 @@ public class GlobalDistributionValidatorTest {
         Fixture fixture = new Fixture()
                 .addGlobalDocument(parent)
                 .addNonGlobalDocument(createDocumentType("child", parent));
-        Redundancy redundancy = createRedundancyWithGlobalDistribution();
-        validate(fixture, redundancy);
+        validate(fixture);
     }
 
     @Test
@@ -115,11 +65,10 @@ public class GlobalDistributionValidatorTest {
         NewDocumentType child = createDocumentType("child", unknown);
         Fixture fixture = new Fixture()
                 .addNonGlobalDocument(child);
-        Redundancy redundancy = createRedundancyWithGlobalDistribution();
         exceptionRule.expect(IllegalArgumentException.class);
         exceptionRule.expectMessage(
                 "The following document types are referenced from other documents, but are not listed in services.xml: 'unknown'");
-        validate(fixture, redundancy);
+        validate(fixture);
     }
 
     @Test
@@ -130,42 +79,14 @@ public class GlobalDistributionValidatorTest {
         new VespaModelCreatorWithFilePkg("src/test/cfg/application/validation/global_distribution_validation/").create();
     }
 
-    private static Redundancy createRedundancyWithGlobalDistribution() {
-        Redundancy redundancy = new Redundancy(2, 2, 2);
-        redundancy.setTotalNodes(2);
-        return redundancy;
-    }
-
-    private static Redundancy createRedundancyWithoutGlobalDistribution() {
-        Redundancy redundancy = new Redundancy(2, 2, 2);
-        redundancy.setTotalNodes(3);
-        return redundancy;
-    }
-
-    private static Redundancy createRedundancyWithTooFewSearchableCopies() {
-        Redundancy redundancy = new Redundancy(2, 2, 1);
-        redundancy.setTotalNodes(2);
-        return redundancy;
-    }
-
-    private static Redundancy createRedundancyWithoutGlobalDistributionAndTooFewSearchableCopies() {
-        Redundancy redundancy = new Redundancy(2, 2, 1);
-        redundancy.setTotalNodes(3);
-        return redundancy;
-    }
-
     private static NewDocumentType createDocumentType(String name, NewDocumentType... references) {
         Set<NewDocumentType.Name> documentReferences = Stream.of(references).map(NewDocumentType::getFullName).collect(toSet());
         return new NewDocumentType(new NewDocumentType.Name(name), documentReferences);
     }
 
-    private static void validate(Fixture fixture, Redundancy redundancy) {
-        validate(fixture, redundancy, false);
-    }
-
-    private static void validate(Fixture fixture, Redundancy redundancy, boolean enableMultipleBucketSpaces) {
+    private static void validate(Fixture fixture) {
         new GlobalDistributionValidator()
-                .validate(fixture.getDocumentTypes(), fixture.getGloballyDistributedDocuments(), redundancy, enableMultipleBucketSpaces);
+                .validate(fixture.getDocumentTypes(), fixture.getGloballyDistributedDocuments());
     }
 
     private static class Fixture {