Let config enable global bucket space on startup for content node

7 files changed, 65 insertions, 25 deletions

diff --git a/storage/src/vespa/storage/common/content_bucket_space_repo.cpp b/storage/src/vespa/storage/common/content_bucket_space_repo.cpp
index 1846c132c0a..d75e0b4cbb3 100644
--- a/storage/src/vespa/storage/common/content_bucket_space_repo.cpp
+++ b/storage/src/vespa/storage/common/content_bucket_space_repo.cpp
@@ -1,6 +1,7 @@
 // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 
 #include "content_bucket_space_repo.h"
+#include <vespa/persistence/spi/fixed_bucket_spaces.h>
 
 using document::BucketSpace;
 
@@ -9,18 +10,21 @@ namespace storage {
 ContentBucketSpaceRepo::ContentBucketSpaceRepo()
     : _map()
 {
-    _map.emplace(BucketSpace::placeHolder(), std::make_unique<ContentBucketSpace>(BucketSpace::placeHolder()));
+    _map.emplace(spi::FixedBucketSpaces::default_space(), std::make_unique<ContentBucketSpace>(spi::FixedBucketSpaces::default_space()));
 }
 
 ContentBucketSpace &
 ContentBucketSpaceRepo::get(BucketSpace bucketSpace) const
 {
-    assert(bucketSpace == BucketSpace::placeHolder());
     auto itr = _map.find(bucketSpace);
     assert(itr != _map.end());
     return *itr->second;
 }
 
+void ContentBucketSpaceRepo::enableGlobalBucketSpace() {
+    _map.emplace(spi::FixedBucketSpaces::global_space(), std::make_unique<ContentBucketSpace>(spi::FixedBucketSpaces::global_space()));
+}
+
 ContentBucketSpaceRepo::BucketSpaces
 ContentBucketSpaceRepo::getBucketSpaces() const
 {
diff --git a/storage/src/vespa/storage/common/content_bucket_space_repo.h b/storage/src/vespa/storage/common/content_bucket_space_repo.h
index 0d4ddb86bcf..f0803d76282 100644
--- a/storage/src/vespa/storage/common/content_bucket_space_repo.h
+++ b/storage/src/vespa/storage/common/content_bucket_space_repo.h
@@ -24,6 +24,8 @@ public:
     BucketSpaceMap::const_iterator begin() const { return _map.begin(); }
     BucketSpaceMap::const_iterator end() const { return _map.end(); }
 
+    void enableGlobalBucketSpace();
+
     BucketSpaces getBucketSpaces() const;
     size_t getBucketMemoryUsage() const;
 
diff --git a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp
index 6fd75e06d9d..7a882c9539f 100644
--- a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp
+++ b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp
@@ -2,6 +2,8 @@
 
 #include "servicelayercomponentregisterimpl.h"
 #include <vespa/vespalib/util/exceptions.h>
+#include <vespa/persistence/spi/fixed_bucket_spaces.h>
+#include <vespa/storage/common/global_bucket_space_distribution_converter.h>
 
 namespace storage {
 
@@ -39,11 +41,19 @@ ServiceLayerComponentRegisterImpl::setDiskCount(uint16_t count)
 void
 ServiceLayerComponentRegisterImpl::setDistribution(lib::Distribution::SP distribution)
 {
-    // For now, copy distribution to all content bucket spaces
-    for (const auto &elem : _bucketSpaceRepo) {
-        elem.second->setDistribution(distribution);
+    _bucketSpaceRepo.get(spi::FixedBucketSpaces::default_space()).setDistribution(distribution);
+    if (enableMultipleBucketSpaces()) {
+        auto global_distr = GlobalBucketSpaceDistributionConverter::convert_to_global(*distribution);
+        _bucketSpaceRepo.get(spi::FixedBucketSpaces::global_space()).setDistribution(global_distr);
     }
     StorageComponentRegisterImpl::setDistribution(distribution);
 }
 
+void ServiceLayerComponentRegisterImpl::setEnableMultipleBucketSpaces(bool enabled) {
+    StorageComponentRegisterImpl::setEnableMultipleBucketSpaces(enabled);
+    if (enabled) {
+        _bucketSpaceRepo.enableGlobalBucketSpace();
+    }
+}
+
 } // storage
diff --git a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h
index df4047c92c3..fc07f394fff 100644
--- a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h
+++ b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h
@@ -37,7 +37,8 @@ public:
 
     void registerServiceLayerComponent(ServiceLayerManagedComponent&) override;
     void setDiskCount(uint16_t count);
-    virtual void setDistribution(lib::Distribution::SP distribution) override;
+    void setDistribution(lib::Distribution::SP distribution) override;
+    void setEnableMultipleBucketSpaces(bool enabled) override;
 };
 
 } // storage
diff --git a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp
index 2f9430c49bb..62598bce402 100644
--- a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp
+++ b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.cpp
@@ -21,7 +21,8 @@ StorageComponentRegisterImpl::StorageComponentRegisterImpl()
       _bucketIdFactory(),
       _distribution(),
       _nodeStateUpdater(nullptr),
-      _bucketSpacesConfig()
+      _bucketSpacesConfig(),
+      _enableMultipleBucketSpaces(false)
 {
 }
 
@@ -32,9 +33,9 @@ StorageComponentRegisterImpl::registerStorageComponent(StorageComponent& smc)
 {
     vespalib::LockGuard lock(_componentLock);
     _components.push_back(&smc);
-    assert(_nodeType != 0);
+    assert(_nodeType != nullptr);
     smc.setNodeInfo(_clusterName, *_nodeType, _index);
-    if (_nodeStateUpdater != 0) {
+    if (_nodeStateUpdater != nullptr) {
         smc.setNodeStateUpdater(*_nodeStateUpdater);
     }
     smc.setDocumentTypeRepo(_docTypeRepo);
@@ -42,7 +43,7 @@ StorageComponentRegisterImpl::registerStorageComponent(StorageComponent& smc)
     smc.setPriorityConfig(_priorityConfig);
     smc.setBucketIdFactory(_bucketIdFactory);
     smc.setDistribution(_distribution);
-    smc.enableMultipleBucketSpaces(_bucketSpacesConfig.enableMultipleBucketSpaces);
+    smc.enableMultipleBucketSpaces(_enableMultipleBucketSpaces);
 }
 
 void
@@ -51,7 +52,7 @@ StorageComponentRegisterImpl::setNodeInfo(vespalib::stringref clusterName,
                                           uint16_t index)
 {
     vespalib::LockGuard lock(_componentLock);
-    if (_nodeType != 0) {
+    if (_nodeType != nullptr) {
         LOG(warning, "Node info already set. May be valid in tests, but is a "
                      "bug in production. Node info should not be updated live");
     }
@@ -70,8 +71,8 @@ StorageComponentRegisterImpl::setNodeStateUpdater(NodeStateUpdater& updater)
                 VESPA_STRLOC);
     }
     _nodeStateUpdater = &updater;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setNodeStateUpdater(updater);
+    for (auto& component : _components) {
+        component->setNodeStateUpdater(updater);
     }
 }
 
@@ -80,8 +81,8 @@ StorageComponentRegisterImpl::setDocumentTypeRepo(document::DocumentTypeRepo::SP
 {
     vespalib::LockGuard lock(_componentLock);
     _docTypeRepo = repo;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setDocumentTypeRepo(repo);
+    for (auto& component : _components) {
+        component->setDocumentTypeRepo(repo);
     }
 }
 
@@ -90,8 +91,8 @@ StorageComponentRegisterImpl::setLoadTypes(documentapi::LoadTypeSet::SP loadType
 {
     vespalib::LockGuard lock(_componentLock);
     _loadTypes = loadTypes;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setLoadTypes(loadTypes);
+    for (auto& component : _components) {
+        component->setLoadTypes(loadTypes);
     }
 }
 
@@ -100,8 +101,8 @@ StorageComponentRegisterImpl::setPriorityConfig(const PriorityConfig& config)
 {
     vespalib::LockGuard lock(_componentLock);
     _priorityConfig = config;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setPriorityConfig(config);
+    for (auto& component : _components) {
+        component->setPriorityConfig(config);
     }
 }
 
@@ -110,8 +111,8 @@ StorageComponentRegisterImpl::setBucketIdFactory(const document::BucketIdFactory
 {
     vespalib::LockGuard lock(_componentLock);
     _bucketIdFactory = factory;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setBucketIdFactory(factory);
+    for (auto& component : _components) {
+        component->setBucketIdFactory(factory);
     }
 }
 
@@ -120,8 +121,8 @@ StorageComponentRegisterImpl::setDistribution(lib::Distribution::SP distribution
 {
     vespalib::LockGuard lock(_componentLock);
     _distribution = distribution;
-    for (uint32_t i=0; i<_components.size(); ++i) {
-        _components[i]->setDistribution(distribution);
+    for (auto& component : _components) {
+        component->setDistribution(distribution);
     }
 }
 
@@ -130,9 +131,21 @@ StorageComponentRegisterImpl::setBucketSpacesConfig(const BucketspacesConfig& co
 {
     vespalib::LockGuard lock(_componentLock);
     _bucketSpacesConfig = config;
-    for (size_t i = 0; i < _components.size(); ++i) {
-        _components[i]->enableMultipleBucketSpaces(config.enableMultipleBucketSpaces);
+}
+
+void StorageComponentRegisterImpl::setEnableMultipleBucketSpaces(bool enabled) {
+    vespalib::LockGuard lock(_componentLock);
+    assert(!_enableMultipleBucketSpaces); // Cannot disable once enabled.
+    _enableMultipleBucketSpaces = enabled;
+    for (auto& component : _components) {
+        component->enableMultipleBucketSpaces(_enableMultipleBucketSpaces);
     }
 }
 
+bool StorageComponentRegisterImpl::enableMultipleBucketSpaces() const {
+    // We allow reading this outside _componentLock, as it should never be written
+    // again after startup.
+    return _enableMultipleBucketSpaces;
+}
+
 } // storage
diff --git a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h
index d281883856c..ffa91afd931 100644
--- a/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h
+++ b/storage/src/vespa/storage/frameworkimpl/component/storagecomponentregisterimpl.h
@@ -37,6 +37,7 @@ class StorageComponentRegisterImpl
     lib::Distribution::SP _distribution;
     NodeStateUpdater* _nodeStateUpdater;
     BucketspacesConfig _bucketSpacesConfig;
+    bool _enableMultipleBucketSpaces;
 
 public:
     typedef std::unique_ptr<StorageComponentRegisterImpl> UP;
@@ -68,6 +69,9 @@ public:
     virtual void setDistribution(lib::Distribution::SP);
     virtual void setBucketSpacesConfig(const BucketspacesConfig&);
 
+    virtual void setEnableMultipleBucketSpaces(bool enabled); // To be called during startup configuration phase only.
+    bool enableMultipleBucketSpaces() const;
+
 };
 
 } // storage
diff --git a/storage/src/vespa/storage/storageserver/storagenode.cpp b/storage/src/vespa/storage/storageserver/storagenode.cpp
index 8dd532b537c..051606ccfde 100644
--- a/storage/src/vespa/storage/storageserver/storagenode.cpp
+++ b/storage/src/vespa/storage/storageserver/storagenode.cpp
@@ -145,6 +145,11 @@ StorageNode::initialize()
     // and store them away, while having the config lock.
     subscribeToConfigs();
 
+    // Multiple bucket spaces can only be enabled on startup and cannot be live reconfigured.
+    // A process restart is required to either enable or disable after the fact.
+    // TODO ensure config is tagged as 'restart' as a consequence
+    _context.getComponentRegister().setEnableMultipleBucketSpaces(_bucketSpacesConfig->enableMultipleBucketSpaces);
+
     updateUpgradeFlag(*_clusterConfig);
 
     // First update some basics that doesn't depend on anything else to be
@@ -347,6 +352,7 @@ StorageNode::handleLiveConfigUpdate(const InitialGuard & initGuard)
     }
     if (_newBucketSpacesConfig) {
         _bucketSpacesConfig = std::move(_newBucketSpacesConfig);
+        // TODO should we deprecate this in favor of always and only setting at startup time..?
         _context.getComponentRegister().setBucketSpacesConfig(*_bucketSpacesConfig);
         // If we've seen global bucket spaces enabled once, we must continue to update
         // bucket spaces config or we'll get out of sync with doc types config.