diff options
8 files changed, 36 insertions, 149 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 { diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp index ad98d64b173..6bb2ca31ec1 100644 --- a/storage/src/vespa/storage/storageserver/storagenode.cpp +++ b/storage/src/vespa/storage/storageserver/storagenode.cpp @@ -206,10 +206,8 @@ StorageNode::initialize() _chain.reset(createChain().release()); - if (_component->enableMultipleBucketSpaces()) { - assert(_communicationManager != nullptr); - _communicationManager->updateBucketSpacesConfig(*_bucketSpacesConfig); - } + 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 @@ -359,9 +357,7 @@ StorageNode::handleLiveConfigUpdate(const InitialGuard & initGuard) if (_newBucketSpacesConfig) { _bucketSpacesConfig = std::move(_newBucketSpacesConfig); _context.getComponentRegister().setBucketSpacesConfig(*_bucketSpacesConfig); - if (_component->enableMultipleBucketSpaces()) { - _communicationManager->updateBucketSpacesConfig(*_bucketSpacesConfig); - } + _communicationManager->updateBucketSpacesConfig(*_bucketSpacesConfig); } } diff --git a/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp b/storageapi/src/vespa/storageapi/mbusprot/storageprotocol.cpp index 33fca0f161a..f83188f7dd8 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 activateBucketSpaceSerialization) + bool configForcedBucketSpaceSerialization) : _serializer5_0(repo, loadTypes), _serializer5_1(repo, loadTypes), _serializer5_2(repo, loadTypes), _serializer6_0(repo, loadTypes), - _activateBucketSpaceSerialization(activateBucketSpaceSerialization) + _configForcedBucketSpaceSerialization(configForcedBucketSpaceSerialization) { } @@ -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 (_activateBucketSpaceSerialization) { + if (_configForcedBucketSpaceSerialization) { 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 (_activateBucketSpaceSerialization) { + if (_configForcedBucketSpaceSerialization) { 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 d85e9d55d1a..56f271db1d0 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 _activateBucketSpaceSerialization; + bool _configForcedBucketSpaceSerialization; }; } diff --git a/storageserver/src/tests/testhelper.cpp b/storageserver/src/tests/testhelper.cpp index b245a6500bd..5e6a71b078f 100644 --- a/storageserver/src/tests/testhelper.cpp +++ b/storageserver/src/tests/testhelper.cpp @@ -96,7 +96,11 @@ vdstestlib::DirConfig getStandardConfig(bool storagenode) { // By default, need "old" behaviour of maxconcurrent config->set("maxconcurrentvisitors_fixed", "4"); config->set("maxconcurrentvisitors_variable", "0"); - config = &dc.addConfig("stor-visitordispatcher"); + dc.addConfig("stor-visitordispatcher"); + config = &dc.addConfig("bucketspaces"); + config->set("documenttype[1]"); + config->set("documenttype[0].name", "testdoctype1"); + config->set("documenttype[0].bucketspace", "default"); addFileConfig(dc, "documenttypes", "config-doctypes.cfg"); addStorageDistributionConfig(dc); return dc; |