diff options
8 files changed, 149 insertions, 36 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 { diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp index 6bb2ca31ec1..ad98d64b173 100644 --- a/storage/src/vespa/storage/storageserver/storagenode.cpp +++ b/storage/src/vespa/storage/storageserver/storagenode.cpp @@ -206,8 +206,10 @@ StorageNode::initialize() _chain.reset(createChain().release()); - assert(_communicationManager != nullptr); - _communicationManager->updateBucketSpacesConfig(*_bucketSpacesConfig); + if (_component->enableMultipleBucketSpaces()) { + assert(_communicationManager != nullptr); + _communicationManager->updateBucketSpacesConfig(*_bucketSpacesConfig); + } // Start the metric manager, such that it starts generating snapshots // and the like. Note that at this time, all metrics should hopefully @@ -357,7 +359,9 @@ StorageNode::handleLiveConfigUpdate(const InitialGuard & initGuard) if (_newBucketSpacesConfig) { _bucketSpacesConfig = std::move(_newBucketSpacesConfig); _context.getComponentRegister().setBucketSpacesConfig(*_bucketSpacesConfig); - _communicationManager->updateBucketSpacesConfig(*_bucketSpacesConfig); + if (_component->enableMultipleBucketSpaces()) { + _communicationManager->updateBucketSpacesConfig(*_bucketSpacesConfig); + } } } diff --git a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp index f83188f7dd8..33fca0f161a 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp +++ b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp @@ -17,12 +17,12 @@ mbus::string StorageProtocol::NAME = "StorageProtocol"; StorageProtocol::StorageProtocol(const std::shared_ptr<const document::DocumentTypeRepo> repo, const documentapi::LoadTypeSet& loadTypes, - bool configForcedBucketSpaceSerialization) + bool activateBucketSpaceSerialization) : _serializer5_0(repo, loadTypes), _serializer5_1(repo, loadTypes), _serializer5_2(repo, loadTypes), _serializer6_0(repo, loadTypes), - _configForcedBucketSpaceSerialization(configForcedBucketSpaceSerialization) + _activateBucketSpaceSerialization(activateBucketSpaceSerialization) { } @@ -106,7 +106,7 @@ StorageProtocol::encode(const vespalib::Version& version, } else if (version < version5_2) { return encodeMessage(_serializer5_1, routable, message, version5_1, version); } else { - if (_configForcedBucketSpaceSerialization) { + if (_activateBucketSpaceSerialization) { return encodeMessage(_serializer6_0, routable, message, version6_0, version); } else { if (version < version6_0) { @@ -184,7 +184,7 @@ StorageProtocol::decode(const vespalib::Version & version, } else if (version < version5_2) { return decodeMessage(_serializer5_1, data, type, version5_1, version); } else { - if (_configForcedBucketSpaceSerialization) { + if (_activateBucketSpaceSerialization) { return decodeMessage(_serializer6_0, data, type, version6_0, version); } else { if (version < version6_0) { diff --git a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h index 56f271db1d0..d85e9d55d1a 100644 --- a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h +++ b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.h @@ -29,7 +29,7 @@ private: ProtocolSerialization5_1 _serializer5_1; ProtocolSerialization5_2 _serializer5_2; ProtocolSerialization6_0 _serializer6_0; - bool _configForcedBucketSpaceSerialization; + bool _activateBucketSpaceSerialization; }; } diff --git a/storageserver/src/tests/testhelper.cpp b/storageserver/src/tests/testhelper.cpp index 5e6a71b078f..b245a6500bd 100644 --- a/storageserver/src/tests/testhelper.cpp +++ b/storageserver/src/tests/testhelper.cpp @@ -96,11 +96,7 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode) { // By default, need "old" behaviour of maxconcurrent config->set("maxconcurrentvisitors_fixed", "4"); config->set("maxconcurrentvisitors_variable", "0"); - dc.addConfig("stor-visitordispatcher"); - config = &dc.addConfig("bucketspaces"); - config->set("documenttype[1]"); - config->set("documenttype[0].name", "testdoctype1"); - config->set("documenttype[0].bucketspace", "default"); + config = &dc.addConfig("stor-visitordispatcher"); addFileConfig(dc, "documenttypes", "config-doctypes.cfg"); addStorageDistributionConfig(dc); return dc; |