From 6d917736806d1e69b0b662657b5ab09e1bc5b904 Mon Sep 17 00:00:00 2001 From: Tor Brede Vekterli Date: Fri, 30 Oct 2020 12:02:38 +0000 Subject: Add stripe bits config and wire to implementation Default is zero bits, which causes the standard, non-striped implementation to be used. --- storage/src/tests/common/teststorageapp.cpp | 5 +++-- storage/src/vespa/storage/bucketdb/const_iterator.h | 2 +- storage/src/vespa/storage/bucketdb/storbucketdb.cpp | 11 +++++++++-- storage/src/vespa/storage/bucketdb/storbucketdb.h | 4 +++- .../vespa/storage/bucketdb/striped_btree_lockable_map.h | 6 +++--- .../storage/bucketdb/striped_btree_lockable_map.hpp | 4 +--- .../src/vespa/storage/common/content_bucket_db_options.h | 16 ++++++++++++++++ .../src/vespa/storage/common/content_bucket_space.cpp | 5 +++-- storage/src/vespa/storage/common/content_bucket_space.h | 2 +- .../vespa/storage/common/content_bucket_space_repo.cpp | 6 +++--- .../src/vespa/storage/common/content_bucket_space_repo.h | 2 +- storage/src/vespa/storage/config/stor-server.def | 9 +++++++++ .../component/servicelayercomponentregisterimpl.cpp | 4 ++-- .../component/servicelayercomponentregisterimpl.h | 2 +- .../storage/storageserver/servicelayernodecontext.cpp | 4 ++-- .../storage/storageserver/servicelayernodecontext.h | 2 +- .../src/vespa/storageserver/app/servicelayerprocess.cpp | 12 +++++++++--- 17 files changed, 68 insertions(+), 28 deletions(-) create mode 100644 storage/src/vespa/storage/common/content_bucket_db_options.h diff --git a/storage/src/tests/common/teststorageapp.cpp b/storage/src/tests/common/teststorageapp.cpp index 2fc33bc1360..14f1e78f5ca 100644 --- a/storage/src/tests/common/teststorageapp.cpp +++ b/storage/src/tests/common/teststorageapp.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 "teststorageapp.h" +#include #include #include #include @@ -135,7 +136,7 @@ namespace { } TestServiceLayerApp::TestServiceLayerApp(vespalib::stringref configId) - : TestStorageApp(std::make_unique(true), // TODO remove B-tree flag once default + : TestStorageApp(std::make_unique(ContentBucketDbOptions()), lib::NodeType::STORAGE, getIndexFromConfig(configId), configId), _compReg(dynamic_cast(TestStorageApp::getComponentRegister())), _persistenceProvider(), @@ -148,7 +149,7 @@ TestServiceLayerApp::TestServiceLayerApp(vespalib::stringref configId) TestServiceLayerApp::TestServiceLayerApp(NodeIndex index, vespalib::stringref configId) - : TestStorageApp(std::make_unique(true), // TODO remove B-tree flag once default + : TestStorageApp(std::make_unique(ContentBucketDbOptions()), lib::NodeType::STORAGE, index, configId), _compReg(dynamic_cast(TestStorageApp::getComponentRegister())), _persistenceProvider(), diff --git a/storage/src/vespa/storage/bucketdb/const_iterator.h b/storage/src/vespa/storage/bucketdb/const_iterator.h index fd42ba971e9..66f7b41554f 100644 --- a/storage/src/vespa/storage/bucketdb/const_iterator.h +++ b/storage/src/vespa/storage/bucketdb/const_iterator.h @@ -15,4 +15,4 @@ public: [[nodiscard]] virtual ConstRefT value() const = 0; }; -} \ No newline at end of file +} diff --git a/storage/src/vespa/storage/bucketdb/storbucketdb.cpp b/storage/src/vespa/storage/bucketdb/storbucketdb.cpp index c92436c26ab..63e00d3804c 100644 --- a/storage/src/vespa/storage/bucketdb/storbucketdb.cpp +++ b/storage/src/vespa/storage/bucketdb/storbucketdb.cpp @@ -2,6 +2,8 @@ #include "storbucketdb.h" #include "btree_lockable_map.h" +#include "striped_btree_lockable_map.h" +#include #include LOG_SETUP(".storage.bucketdb.stor_bucket_db"); @@ -42,12 +44,17 @@ std::unique_ptr> make_btree_db_impl() { return std::make_unique>(); } +std::unique_ptr> make_striped_btree_db_impl(uint8_t n_stripes) { + return std::make_unique>(n_stripes); +} + } } // bucketdb -StorBucketDatabase::StorBucketDatabase([[maybe_unused]] bool use_btree_db) - : _impl(bucketdb::make_btree_db_impl()) +StorBucketDatabase::StorBucketDatabase(const ContentBucketDbOptions& opts) + : _impl((opts.n_stripe_bits > 0) ? bucketdb::make_striped_btree_db_impl(opts.n_stripe_bits) + : bucketdb::make_btree_db_impl()) {} void diff --git a/storage/src/vespa/storage/bucketdb/storbucketdb.h b/storage/src/vespa/storage/bucketdb/storbucketdb.h index e7ccac30c37..cf6401d9478 100644 --- a/storage/src/vespa/storage/bucketdb/storbucketdb.h +++ b/storage/src/vespa/storage/bucketdb/storbucketdb.h @@ -10,6 +10,8 @@ namespace storage { +struct ContentBucketDbOptions; + class StorBucketDatabase { std::unique_ptr> _impl; public: @@ -25,7 +27,7 @@ public: CREATE_IF_NONEXISTING = 1 }; - explicit StorBucketDatabase(bool use_btree_db = false); + explicit StorBucketDatabase(const ContentBucketDbOptions&); void insert(const document::BucketId&, const bucketdb::StorageBucketInfo&, const char* clientId); diff --git a/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.h b/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.h index 690db6e08d0..267f1905da8 100644 --- a/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.h +++ b/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.h @@ -25,14 +25,14 @@ public: using Decision = typename ParentType::Decision; using BucketId = document::BucketId; - constexpr static uint16_t MaxStripeBits = 8; + constexpr static uint8_t MaxStripeBits = 8; private: using StripedDBType = BTreeLockableMap; - uint16_t _n_stripe_bits; + uint8_t _n_stripe_bits; size_t _n_stripes; std::vector> _stripes; public: - explicit StripedBTreeLockableMap(uint16_t n_stripe_bits = 4); + explicit StripedBTreeLockableMap(uint8_t n_stripe_bits = 4); ~StripedBTreeLockableMap() override; size_t size() const noexcept override; diff --git a/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.hpp b/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.hpp index ba7d435846c..e4f6efa30a3 100644 --- a/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.hpp +++ b/storage/src/vespa/storage/bucketdb/striped_btree_lockable_map.hpp @@ -17,10 +17,8 @@ constexpr uint8_t used_bits_of(uint64_t key) noexcept { } -// TODO rename to sharded_btree_lockable_map instead? - template -StripedBTreeLockableMap::StripedBTreeLockableMap(uint16_t n_stripe_bits) +StripedBTreeLockableMap::StripedBTreeLockableMap(uint8_t n_stripe_bits) : _n_stripe_bits(n_stripe_bits), _n_stripes(1ULL << _n_stripe_bits), _stripes() diff --git a/storage/src/vespa/storage/common/content_bucket_db_options.h b/storage/src/vespa/storage/common/content_bucket_db_options.h new file mode 100644 index 00000000000..f4a7e84a7b8 --- /dev/null +++ b/storage/src/vespa/storage/common/content_bucket_db_options.h @@ -0,0 +1,16 @@ +// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +#pragma once + +#include + +namespace storage { + +// Type-safe encapsulation of any options that can be passed from config +// to the content node bucket database implementation. +struct ContentBucketDbOptions { + // The number of DB stripes created will be 2^n (within implementation limits) + // TODO expose max limit here? + uint8_t n_stripe_bits = 0; +}; + +} diff --git a/storage/src/vespa/storage/common/content_bucket_space.cpp b/storage/src/vespa/storage/common/content_bucket_space.cpp index e293c4bc336..da2f78e2a6a 100644 --- a/storage/src/vespa/storage/common/content_bucket_space.cpp +++ b/storage/src/vespa/storage/common/content_bucket_space.cpp @@ -4,9 +4,10 @@ namespace storage { -ContentBucketSpace::ContentBucketSpace(document::BucketSpace bucketSpace, bool use_btree_db) +ContentBucketSpace::ContentBucketSpace(document::BucketSpace bucketSpace, + const ContentBucketDbOptions& db_opts) : _bucketSpace(bucketSpace), - _bucketDatabase(use_btree_db), + _bucketDatabase(db_opts), _lock(), _clusterState(), _distribution(), diff --git a/storage/src/vespa/storage/common/content_bucket_space.h b/storage/src/vespa/storage/common/content_bucket_space.h index bf001f8b6f2..98f287a3154 100644 --- a/storage/src/vespa/storage/common/content_bucket_space.h +++ b/storage/src/vespa/storage/common/content_bucket_space.h @@ -26,7 +26,7 @@ private: public: using UP = std::unique_ptr; - explicit ContentBucketSpace(document::BucketSpace bucketSpace, bool use_btree_db = false); + explicit ContentBucketSpace(document::BucketSpace bucketSpace, const ContentBucketDbOptions& db_opts); document::BucketSpace bucketSpace() const noexcept { return _bucketSpace; } StorBucketDatabase &bucketDatabase() { return _bucketDatabase; } 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 2cc03e4d1e5..a76ad9576e0 100644 --- a/storage/src/vespa/storage/common/content_bucket_space_repo.cpp +++ b/storage/src/vespa/storage/common/content_bucket_space_repo.cpp @@ -7,13 +7,13 @@ using document::BucketSpace; namespace storage { -ContentBucketSpaceRepo::ContentBucketSpaceRepo(bool use_btree_db) +ContentBucketSpaceRepo::ContentBucketSpaceRepo(const ContentBucketDbOptions& db_opts) : _map() { _map.emplace(document::FixedBucketSpaces::default_space(), - std::make_unique(document::FixedBucketSpaces::default_space(), use_btree_db)); + std::make_unique(document::FixedBucketSpaces::default_space(), db_opts)); _map.emplace(document::FixedBucketSpaces::global_space(), - std::make_unique(document::FixedBucketSpaces::global_space(), use_btree_db)); + std::make_unique(document::FixedBucketSpaces::global_space(), db_opts)); } ContentBucketSpace & 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 1c858e47bcd..01698eb6e01 100644 --- a/storage/src/vespa/storage/common/content_bucket_space_repo.h +++ b/storage/src/vespa/storage/common/content_bucket_space_repo.h @@ -19,7 +19,7 @@ private: BucketSpaceMap _map; public: - explicit ContentBucketSpaceRepo(bool use_btree_db = false); + explicit ContentBucketSpaceRepo(const ContentBucketDbOptions&); ContentBucketSpace &get(document::BucketSpace bucketSpace) const; BucketSpaceMap::const_iterator begin() const { return _map.begin(); } BucketSpaceMap::const_iterator end() const { return _map.end(); } diff --git a/storage/src/vespa/storage/config/stor-server.def b/storage/src/vespa/storage/config/stor-server.def index cd940079a70..54160bc53fe 100644 --- a/storage/src/vespa/storage/config/stor-server.def +++ b/storage/src/vespa/storage/config/stor-server.def @@ -88,3 +88,12 @@ simulated_bucket_request_latency_msec int default=0 ## If set, content node processes will use a B-tree backed bucket database implementation ## instead of the legacy Judy-based implementation. use_content_node_btree_bucket_db bool default=true restart + +## If non-zero, the bucket DB will be striped into 2^bits sub-databases, each handling +## a disjoint subset of the node's buckets, in order to reduce locking contention. +## Max value is unspecified, but will be clamped internally. +## WARNING: +## Setting this to a non-zero value requires the minimum split bit level in the cluster +## to be enforced, so only set this value if you know exactly what you're doing! +content_node_bucket_db_stripe_bits int default=0 + diff --git a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp index 7a342c1df25..e2a06ee640b 100644 --- a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp +++ b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.cpp @@ -9,8 +9,8 @@ namespace storage { using vespalib::IllegalStateException; -ServiceLayerComponentRegisterImpl::ServiceLayerComponentRegisterImpl(bool use_btree_db) - : _bucketSpaceRepo(use_btree_db) +ServiceLayerComponentRegisterImpl::ServiceLayerComponentRegisterImpl(const ContentBucketDbOptions& db_opts) + : _bucketSpaceRepo(db_opts) { } void diff --git a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h index 967adf8f787..a77b8061842 100644 --- a/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h +++ b/storage/src/vespa/storage/frameworkimpl/component/servicelayercomponentregisterimpl.h @@ -26,7 +26,7 @@ class ServiceLayerComponentRegisterImpl public: typedef std::unique_ptr UP; - ServiceLayerComponentRegisterImpl(bool use_btree_db = false); + explicit ServiceLayerComponentRegisterImpl(const ContentBucketDbOptions&); ContentBucketSpaceRepo& getBucketSpaceRepo() { return _bucketSpaceRepo; } MinimumUsedBitsTracker& getMinUsedBitsTracker() { diff --git a/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp b/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp index bbfc9afc7aa..4e4e18a7f01 100644 --- a/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp +++ b/storage/src/vespa/storage/storageserver/servicelayernodecontext.cpp @@ -4,8 +4,8 @@ namespace storage { -ServiceLayerNodeContext::ServiceLayerNodeContext(framework::Clock::UP clock, bool use_btree_db) - : StorageNodeContext(std::make_unique(use_btree_db), +ServiceLayerNodeContext::ServiceLayerNodeContext(framework::Clock::UP clock, const ContentBucketDbOptions& db_opts) + : StorageNodeContext(std::make_unique(db_opts), std::move(clock)), _componentRegister(dynamic_cast(StorageNodeContext::getComponentRegister())) { diff --git a/storage/src/vespa/storage/storageserver/servicelayernodecontext.h b/storage/src/vespa/storage/storageserver/servicelayernodecontext.h index 0d7ac5bff69..0516c9e3bda 100644 --- a/storage/src/vespa/storage/storageserver/servicelayernodecontext.h +++ b/storage/src/vespa/storage/storageserver/servicelayernodecontext.h @@ -29,7 +29,7 @@ struct ServiceLayerNodeContext : public StorageNodeContext { * You can provide your own clock implementation. Useful in testing where * you want to fake the clock. */ - ServiceLayerNodeContext(framework::Clock::UP clock, bool use_btree_db); + ServiceLayerNodeContext(framework::Clock::UP clock, const ContentBucketDbOptions& db_opts); /** * Get the actual component register. Available as the actual type as the diff --git a/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp b/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp index bdb53ce6a60..7c7816f33ac 100644 --- a/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp +++ b/storageserver/src/vespa/storageserver/app/servicelayerprocess.cpp @@ -2,6 +2,7 @@ #include "servicelayerprocess.h" #include +#include #include #include #include @@ -14,11 +15,16 @@ namespace storage { namespace { -bool configured_to_use_btree_db(const config::ConfigUri& config_uri) { +ContentBucketDbOptions bucket_db_options_from_config(const config::ConfigUri& config_uri) { using vespa::config::content::core::StorServerConfig; auto server_config = config::ConfigGetter::getConfig( config_uri.getConfigId(), config_uri.getContext()); - return server_config->useContentNodeBtreeBucketDb; + // For now, limit to max 8 bits, i.e. 256 sub DBs. + // 0 bits (the default value) disables striping entirely. + auto n_stripe_bits = std::min(std::max(server_config->contentNodeBucketDbStripeBits, 0), 8); + ContentBucketDbOptions opts; + opts.n_stripe_bits = n_stripe_bits; + return opts; } } @@ -29,7 +35,7 @@ ServiceLayerProcess::ServiceLayerProcess(const config::ConfigUri& configUri) _node(), _storage_chain_builder(), _context(std::make_unique(), - configured_to_use_btree_db(configUri)) + bucket_db_options_from_config(configUri)) { } -- cgit v1.2.3