diff options
author | Tor Brede Vekterli <vekterli@oath.com> | 2018-05-30 13:58:13 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2018-05-30 13:58:13 +0200 |
commit | 8860284d5a84bb62096009ce134702ae5b3200c0 (patch) | |
tree | fcb73f384f71d55488715298ba22608585a3bd60 /config-model | |
parent | 89c889a357afee2bbd6bbfae097875e6f4da658d (diff) |
Revert "Implicitly enable multiple bucket spaces"
Diffstat (limited to 'config-model')
4 files changed, 136 insertions, 23 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 0661ef67131..4bdef0607a2 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,11 +21,45 @@ import static java.util.stream.Collectors.toSet; public class GlobalDistributionValidator { public void validate(Map<String, NewDocumentType> documentDefinitions, - Set<NewDocumentType> globallyDistributedDocuments) { + Set<NewDocumentType> globallyDistributedDocuments, + Redundancy redundancy, + boolean enableMultipleBucketSpaces) { + if (!enableMultipleBucketSpaces) { + verifyGlobalDocumentsHaveRequiredRedundancy(globallyDistributedDocuments, redundancy); + verifySearchableCopiesIsSameAsRedundancy(globallyDistributedDocuments, redundancy); + } 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 68ae4d2b242..0119bced095 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,12 +61,14 @@ 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; - private boolean forceEnableMultipleBucketSpaces = false; + // Experimental flag (TODO: remove when feature is enabled by default) + private boolean enableMultipleBucketSpaces = false; private com.yahoo.vespa.model.content.StorageGroup rootGroup; private StorageCluster storageNodes; private DistributorCluster distributorNodes; @@ -248,7 +250,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.forceEnableMultipleBucketSpaces = enableMultipleBucketSpaces; + cluster.enableMultipleBucketSpaces = enableMultipleBucketSpaces; } } @@ -594,13 +596,13 @@ public class ContentCluster extends AbstractConfigProducer implements builder.min_distributor_up_ratio(0); builder.min_storage_up_ratio(0); } - builder.enable_multiple_bucket_spaces(true); + builder.enable_multiple_bucket_spaces(enableMultipleBucketSpaces); // 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(!globallyDistributedDocuments.isEmpty()); + builder.cluster_has_global_document_types(enableMultipleBucketSpaces && !globallyDistributedDocuments.isEmpty()); } @Override @@ -644,7 +646,7 @@ public class ContentCluster extends AbstractConfigProducer implements } } new ReservedDocumentTypeNameValidator().validate(documentDefinitions); - new GlobalDistributionValidator().validate(documentDefinitions, globallyDistributedDocuments); + new GlobalDistributionValidator().validate(documentDefinitions, globallyDistributedDocuments, redundancy, enableMultipleBucketSpaces); } public static Map<String, Integer> METRIC_INDEX_MAP = new TreeMap<>(); @@ -725,13 +727,11 @@ 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 = (isGloballyDistributed(docType) ? GLOBAL_BUCKET_SPACE : DEFAULT_BUCKET_SPACE); + String bucketSpace = ((enableMultipleBucketSpaces && isGloballyDistributed(docType)) + ? GLOBAL_BUCKET_SPACE : DEFAULT_BUCKET_SPACE); docTypeBuilder.bucketspace(bucketSpace); builder.documenttype(docTypeBuilder); } - // 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); + builder.enable_multiple_bucket_spaces(enableMultipleBucketSpaces); } } 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 0156128f7ca..98177b4ada0 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_document_types_belong_to_correct_bucket_spaces() throws Exception { + public void require_that_all_document_types_belong_to_default_bucket_space_by_default() throws Exception { BucketspacesConfig config = getBucketspacesConfig(createClusterWithGlobalType()); assertEquals(2, config.documenttype().size()); - assertDocumentType("global", "global", config.documenttype(0)); + assertDocumentType("global", "default", 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_force_enabled() throws Exception { + public void require_that_multiple_bucket_spaces_can_be_enabled() throws Exception { ContentCluster cluster = createClusterWithMultipleBucketSpacesEnabled(); { BucketspacesConfig config = getBucketspacesConfig(cluster); @@ -210,9 +210,9 @@ public class ContentSearchClusterTest { } @Test - public void controller_global_documents_config_always_enabled_even_without_experimental_flag_set() throws Exception { + public void controller_global_documents_config_forced_to_false_if_multiple_spaces_not_enabled() throws Exception { ContentCluster cluster = createClusterWithGlobalDocsButNotMultipleSpacesEnabled(); - assertTrue(getFleetcontrollerConfig(cluster).cluster_has_global_document_types()); + assertFalse(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 6506f7a08a8..b8252f2f081 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,16 +26,64 @@ 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()); + .validate(emptyMap(), emptySet(), createRedundancyWithoutGlobalDistribution(), false); } @Test public void validation_succeeds_on_no_global_documents() { Fixture fixture = new Fixture() .addNonGlobalDocument(createDocumentType("foo")); - validate(fixture); + Redundancy redundancy = createRedundancyWithoutGlobalDistribution(); + validate(fixture, redundancy); } @Test @@ -44,10 +92,11 @@ 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); + validate(fixture, redundancy); } @Test @@ -56,7 +105,8 @@ public class GlobalDistributionValidatorTest { Fixture fixture = new Fixture() .addGlobalDocument(parent) .addNonGlobalDocument(createDocumentType("child", parent)); - validate(fixture); + Redundancy redundancy = createRedundancyWithGlobalDistribution(); + validate(fixture, redundancy); } @Test @@ -65,10 +115,11 @@ 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); + validate(fixture, redundancy); } @Test @@ -79,14 +130,42 @@ 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) { + private static void validate(Fixture fixture, Redundancy redundancy) { + validate(fixture, redundancy, false); + } + + private static void validate(Fixture fixture, Redundancy redundancy, boolean enableMultipleBucketSpaces) { new GlobalDistributionValidator() - .validate(fixture.getDocumentTypes(), fixture.getGloballyDistributedDocuments()); + .validate(fixture.getDocumentTypes(), fixture.getGloballyDistributedDocuments(), redundancy, enableMultipleBucketSpaces); } private static class Fixture { |