diff options
21 files changed, 25 insertions, 789 deletions
diff --git a/storage/src/tests/distributor/CMakeLists.txt b/storage/src/tests/distributor/CMakeLists.txt index 1403021a9c3..f983fb32e1c 100644 --- a/storage/src/tests/distributor/CMakeLists.txt +++ b/storage/src/tests/distributor/CMakeLists.txt @@ -22,7 +22,6 @@ vespa_add_executable(storage_distributor_gtest_runner_app TEST idealstatemanagertest.cpp joinbuckettest.cpp maintenanceschedulertest.cpp - mapbucketdatabasetest.cpp mergelimitertest.cpp mergeoperationtest.cpp nodeinfotest.cpp diff --git a/storage/src/tests/distributor/distributortest.cpp b/storage/src/tests/distributor/distributortest.cpp index b110e99f8a4..0da860d0542 100644 --- a/storage/src/tests/distributor/distributortest.cpp +++ b/storage/src/tests/distributor/distributortest.cpp @@ -1072,7 +1072,7 @@ TEST_F(DistributorTest, pending_to_no_pending_global_merges_edge_immediately_sen } TEST_F(DistributorTest, stale_reads_config_is_propagated_to_external_operation_handler) { - createLinks(true); + createLinks(); setupDistributor(Redundancy(1), NodeCount(1), "distributor:1 storage:1"); configure_stale_reads_enabled(true); @@ -1083,7 +1083,7 @@ TEST_F(DistributorTest, stale_reads_config_is_propagated_to_external_operation_h } TEST_F(DistributorTest, fast_path_on_consistent_gets_config_is_propagated_to_internal_config) { - createLinks(true); + createLinks(); setupDistributor(Redundancy(1), NodeCount(1), "distributor:1 storage:1"); configure_update_fast_path_restart_enabled(true); @@ -1094,7 +1094,7 @@ TEST_F(DistributorTest, fast_path_on_consistent_gets_config_is_propagated_to_int } TEST_F(DistributorTest, merge_disabling_config_is_propagated_to_internal_config) { - createLinks(true); + createLinks(); setupDistributor(Redundancy(1), NodeCount(1), "distributor:1 storage:1"); configure_merge_operations_disabled(true); @@ -1105,7 +1105,7 @@ TEST_F(DistributorTest, merge_disabling_config_is_propagated_to_internal_config) } TEST_F(DistributorTest, metadata_update_phase_config_is_propagated_to_internal_config) { - createLinks(true); + createLinks(); setupDistributor(Redundancy(1), NodeCount(1), "distributor:1 storage:1"); configure_metadata_update_phase_enabled(true); @@ -1116,7 +1116,7 @@ TEST_F(DistributorTest, metadata_update_phase_config_is_propagated_to_internal_c } TEST_F(DistributorTest, weak_internal_read_consistency_config_is_propagated_to_internal_configs) { - createLinks(true); + createLinks(); setupDistributor(Redundancy(1), NodeCount(1), "distributor:1 storage:1"); configure_use_weak_internal_read_consistency(true); @@ -1128,15 +1128,8 @@ TEST_F(DistributorTest, weak_internal_read_consistency_config_is_propagated_to_i EXPECT_FALSE(getExternalOperationHandler().use_weak_internal_read_consistency_for_gets()); } -TEST_F(DistributorTest, concurrent_reads_not_enabled_if_btree_db_is_not_enabled) { - createLinks(false); - setupDistributor(Redundancy(1), NodeCount(1), "distributor:1 storage:1"); - configure_stale_reads_enabled(true); - EXPECT_FALSE(getExternalOperationHandler().concurrent_gets_enabled()); -} - void DistributorTest::set_up_and_start_get_op_with_stale_reads_enabled(bool enabled) { - createLinks(true); + createLinks(); setupDistributor(Redundancy(1), NodeCount(1), "distributor:1 storage:1"); configure_stale_reads_enabled(enabled); @@ -1145,7 +1138,7 @@ void DistributorTest::set_up_and_start_get_op_with_stale_reads_enabled(bool enab _distributor->onDown(make_dummy_get_command_for_bucket_1()); } -TEST_F(DistributorTest, gets_are_started_outside_main_distributor_logic_if_btree_db_and_stale_reads_enabled) { +TEST_F(DistributorTest, gets_are_started_outside_main_distributor_logic_if_stale_reads_enabled) { set_up_and_start_get_op_with_stale_reads_enabled(true); ASSERT_THAT(_sender.commands(), SizeIs(1)); EXPECT_THAT(_sender.replies(), SizeIs(0)); diff --git a/storage/src/tests/distributor/distributortestutil.cpp b/storage/src/tests/distributor/distributortestutil.cpp index 9f0c56c3fa5..3c26ea01c19 100644 --- a/storage/src/tests/distributor/distributortestutil.cpp +++ b/storage/src/tests/distributor/distributortestutil.cpp @@ -20,7 +20,7 @@ DistributorTestUtil::DistributorTestUtil() DistributorTestUtil::~DistributorTestUtil() { } void -DistributorTestUtil::createLinks(bool use_btree_db) +DistributorTestUtil::createLinks() { _node.reset(new TestDistributorApp(_config.getConfigId())); _threadPool = framework::TickingThreadPool::createDefault("distributor"); @@ -29,7 +29,6 @@ DistributorTestUtil::createLinks(bool use_btree_db) *_threadPool, *this, true, - use_btree_db, _hostInfo, &_messageSender)); _component.reset(new storage::DistributorComponent(_node->getComponentRegister(), "distrtestutil")); diff --git a/storage/src/tests/distributor/distributortestutil.h b/storage/src/tests/distributor/distributortestutil.h index a6e7ea16798..3dc71bcb433 100644 --- a/storage/src/tests/distributor/distributortestutil.h +++ b/storage/src/tests/distributor/distributortestutil.h @@ -34,7 +34,7 @@ public: /** * Sets up the storage link chain. */ - void createLinks(bool use_btree_db = false); + void createLinks(); void setTypeRepo(const std::shared_ptr<const document::DocumentTypeRepo> &repo); void close(); diff --git a/storage/src/tests/distributor/maintenanceschedulertest.cpp b/storage/src/tests/distributor/maintenanceschedulertest.cpp index 53408bbf6b6..f4b1b687158 100644 --- a/storage/src/tests/distributor/maintenanceschedulertest.cpp +++ b/storage/src/tests/distributor/maintenanceschedulertest.cpp @@ -2,7 +2,6 @@ #include <vespa/document/test/make_document_bucket.h> #include <vespa/storage/distributor/maintenance/simplebucketprioritydatabase.h> #include <vespa/storage/distributor/maintenance/maintenancescheduler.h> -#include <vespa/storage/bucketdb/mapbucketdatabase.h> #include <tests/distributor/maintenancemocks.h> #include <vespa/vespalib/gtest/gtest.h> #include <memory> diff --git a/storage/src/tests/distributor/mapbucketdatabasetest.cpp b/storage/src/tests/distributor/mapbucketdatabasetest.cpp deleted file mode 100644 index 2c000f6b5db..00000000000 --- a/storage/src/tests/distributor/mapbucketdatabasetest.cpp +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include <vespa/storage/bucketdb/mapbucketdatabase.h> -#include <tests/distributor/bucketdatabasetest.h> - -namespace storage::distributor { - -VESPA_GTEST_INSTANTIATE_TEST_SUITE_P(MapDatabase, BucketDatabaseTest, - ::testing::Values(std::make_shared<MapBucketDatabase>())); - -} diff --git a/storage/src/tests/distributor/simplemaintenancescannertest.cpp b/storage/src/tests/distributor/simplemaintenancescannertest.cpp index abf061dd990..b21a10c319e 100644 --- a/storage/src/tests/distributor/simplemaintenancescannertest.cpp +++ b/storage/src/tests/distributor/simplemaintenancescannertest.cpp @@ -36,7 +36,7 @@ void SimpleMaintenanceScannerTest::SetUp() { _priorityGenerator = std::make_unique<MockMaintenancePriorityGenerator>(); - _bucketSpaceRepo = std::make_unique<DistributorBucketSpaceRepo>(false); + _bucketSpaceRepo = std::make_unique<DistributorBucketSpaceRepo>(); _priorityDb = std::make_unique<SimpleBucketPriorityDatabase>(); _scanner = std::make_unique<SimpleMaintenanceScanner>(*_priorityDb, *_priorityGenerator, *_bucketSpaceRepo); } @@ -79,7 +79,7 @@ TEST_F(SimpleMaintenanceScannerTest, prioritize_single_bucket) { TEST_F(SimpleMaintenanceScannerTest, prioritize_single_bucket_alt_bucket_space) { document::BucketSpace bucketSpace(4); - _bucketSpaceRepo->add(bucketSpace, std::make_unique<DistributorBucketSpace>(false)); + _bucketSpaceRepo->add(bucketSpace, std::make_unique<DistributorBucketSpace>()); _scanner->reset(); addBucketToDb(bucketSpace, 1); std::string expected("PrioritizedBucket(Bucket(BucketSpace(0x0000000000000004), BucketId(0x4000000000000001)), pri VERY_HIGH)\n"); diff --git a/storage/src/vespa/storage/bucketdb/CMakeLists.txt b/storage/src/vespa/storage/bucketdb/CMakeLists.txt index 5bd966ae0e1..abf310d9bae 100644 --- a/storage/src/vespa/storage/bucketdb/CMakeLists.txt +++ b/storage/src/vespa/storage/bucketdb/CMakeLists.txt @@ -11,7 +11,6 @@ vespa_add_library(storage_bucketdb OBJECT generic_btree_bucket_database.cpp judyarray.cpp lockablemap.cpp - mapbucketdatabase.cpp storagebucketdbinitializer.cpp storbucketdb.cpp DEPENDS diff --git a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp deleted file mode 100644 index 7556b80b29c..00000000000 --- a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp +++ /dev/null @@ -1,625 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. - -#include "mapbucketdatabase.h" -#include <vespa/storage/common/bucketoperationlogger.h> -#include <vespa/vespalib/util/backtrace.h> -#include <ostream> -#include <cassert> - -#include <vespa/log/bufferedlogger.h> -LOG_SETUP(".mapbucketdatabase"); - -namespace storage { - -MapBucketDatabase::MapBucketDatabase() -{ - // Allocate the root element. - allocate(); -} - -MapBucketDatabase::~MapBucketDatabase() {} - -MapBucketDatabase::E::~E() { } - -uint32_t -MapBucketDatabase::allocate() -{ - if (!_free.empty()) { - uint32_t retVal = _free[_free.size() - 1]; - _free.pop_back(); - return retVal; - } - - _db.push_back(E()); - return _db.size() - 1; -} - -uint32_t -MapBucketDatabase::allocateValue(const document::BucketId& bid) -{ - if (!_freeValues.empty()) { - uint32_t retVal = _freeValues[_freeValues.size() - 1]; - _freeValues.pop_back(); - return retVal; - } - - _values.push_back(BucketDatabase::Entry(bid)); - return _values.size() - 1; -} - -BucketDatabase::Entry* -MapBucketDatabase::find(int index, uint8_t bitCount, - const document::BucketId& bid, bool create) -{ - if (index == -1) { - return NULL; - } - - E& e = _db[index]; - if (bitCount == bid.getUsedBits()) { - if (e.value == -1) { - if (create) { - e.value = allocateValue(bid); - } else { - return NULL; - } - } - - return &_values[e.value]; - } - - // Must reference _db[index] rather than E, since the address of E may change - // in allocate(). - if (bid.getBit(bitCount) == 0) { - if (e.e_0 == -1 && create) { - int val = allocate(); - _db[index].e_0 = val; - } - - return find(_db[index].e_0, bitCount + 1, bid, create); - } else { - if (e.e_1 == -1 && create) { - int val = allocate(); - _db[index].e_1 = val; - } - - return find(_db[index].e_1, bitCount + 1, bid, create); - } -} - -BucketDatabase::Entry -MapBucketDatabase::get(const document::BucketId& bucket) const -{ - MapBucketDatabase& mutableSelf(const_cast<MapBucketDatabase&>(*this)); - Entry* found = mutableSelf.find(0, 0, bucket, false); - if (found) { - return *found; - } else { - return BucketDatabase::Entry(); - } -} - -bool -MapBucketDatabase::remove(int index, - uint8_t bitCount, - const document::BucketId& bid) -{ - if (index == -1) { - return false; - } - - E& e = _db[index]; - if (bitCount == bid.getUsedBits()) { - if (e.value != -1) { - _freeValues.push_back(e.value); - e.value = -1; - } - } - - if (bid.getBit(bitCount) == 0) { - if (remove(e.e_0, bitCount + 1, bid)) { - e.e_0 = -1; - } - } else { - if (remove(e.e_1, bitCount + 1, bid)) { - e.e_1 = -1; - } - } - - if (e.empty() && index > 0) { - _free.push_back(index); - return true; - } else { - return false; - } -} - -void -MapBucketDatabase::remove(const document::BucketId& bucket) -{ - LOG_BUCKET_OPERATION_NO_LOCK(bucket, "REMOVING from bucket db!"); - remove(0, 0, bucket); -} - -namespace { - -void __attribute__((noinline)) log_empty_bucket_insertion(const document::BucketId& id) { - // Use buffered logging to avoid spamming the logs in case this is triggered for - // many buckets simultaneously. - LOGBP(error, "Inserted empty bucket %s into database.\n%s", - id.toString().c_str(), vespalib::getStackTrace(2).c_str()); -} - -} - -template <typename EntryType> -void MapBucketDatabase::update_internal(EntryType&& new_entry) { - assert(new_entry.valid()); - if (new_entry->getNodeCount() == 0) { - log_empty_bucket_insertion(new_entry.getBucketId()); - } - Entry* found = find(0, 0, new_entry.getBucketId(), true); - assert(found); - *found = std::forward<EntryType>(new_entry); -} - -void -MapBucketDatabase::update(const Entry& newEntry) -{ - update_internal(newEntry); -} - -void -MapBucketDatabase::findParents(int index, - uint8_t bitCount, - const document::BucketId& bid, - std::vector<Entry>& entries) const -{ - if (index == -1) { - return; - } - - const E& e = _db[index]; - if (e.value != -1) { - entries.push_back(_values[e.value]); - } - - if (bitCount >= bid.getUsedBits()) { - return; - } - - if (bid.getBit(bitCount) == 0) { - findParents(e.e_0, bitCount + 1, bid, entries); - } else { - findParents(e.e_1, bitCount + 1, bid, entries); - } -} - - -void -MapBucketDatabase::getParents(const document::BucketId& childBucket, - std::vector<Entry>& entries) const -{ - findParents(0, 0, childBucket, entries); -} - -void -MapBucketDatabase::findAll(int index, - uint8_t bitCount, - const document::BucketId& bid, - std::vector<Entry>& entries) const -{ - if (index == -1) { - return; - } - - const E& e = _db[index]; - if (e.value != -1) { - entries.push_back(_values[e.value]); - } - - if (bitCount >= bid.getUsedBits()) { - findAll(e.e_0, bitCount + 1, bid, entries); - findAll(e.e_1, bitCount + 1, bid, entries); - } else { - if (bid.getBit(bitCount) == 0) { - findAll(e.e_0, bitCount + 1, bid, entries); - } else { - findAll(e.e_1, bitCount + 1, bid, entries); - } - } -} - -void -MapBucketDatabase::getAll(const document::BucketId& bucket, - std::vector<Entry>& entries) const -{ - findAll(0, 0, bucket, entries); -} - -/** - * Any child bucket under a bucket held in an inner node will be ordered after - * (i.e. be greater than) the inner node bucket. This is because in bucket key - * order these have the same bit prefix but are guaranteed to have a suffix that - * make them greater. From our bucket ordering spec, a bucket with 5 bits of - * 00000 is greater than a bucket of 3 bits of 000 because the suffix logically - * takes into account the number of used bucket bits (meaning the actual - * values are more akin to 000000000:5 and 00000000:3). When traversing the bit - * tree, we mirror this behavior since all child nodes by definition have a - * higher used bit value from their depth in the tree. - */ -int -MapBucketDatabase::findFirstInOrderNodeInclusive(int index) const -{ - if (index == -1) { - return -1; - } - - int follow = index; - while (true) { - const E& e = _db[follow]; - if (e.value != -1) { - return follow; - } - // In-order 0 bits sort before 1 bits so we follow the 0 branch if - // at all possible. It is illegal for a branch to exist without there - // existing a leaf somewhere underneath it, so we're destined to hit - // something if it exists. - follow = (e.e_0 != -1 ? e.e_0 : e.e_1); - if (follow == -1) { - return -1; - } - } -} - -/** - * Follow the bit tree as far as we can based on upper bound `value`. To get a - * bucket with an ID greater than `value` we must try to follow the bit tree - * as far down as possible, taking the branches that correspond to our input - * value: - * 1) If input value has a 0 bit in the `depth` position but no such branch - * exists at the current node we look in its 1 branch (if one exists), - * returning the first in-order child. - * 2) If we've reached a node that equals the input value (current depth - * equals used bits), look for the first in-order child under the node - * in question. - * 3) Otherwise, keep recursing down the same bit prefix subtree. - */ -int -MapBucketDatabase::upperBoundImpl(int index, - uint8_t depth, - const document::BucketId& value) const -{ - if (index == -1) { - return -1; // Branch with no children; bail out and up. - } - - const E& e = _db[index]; - if (depth < value.getUsedBits()) { - if (value.getBit(depth) == 0) { - int candidate = upperBoundImpl(e.e_0, depth + 1, value); - if (candidate != -1) { - return candidate; - } - // No choice but to try to follow 1-branch. - return findFirstInOrderNodeInclusive(e.e_1); - } else { - return upperBoundImpl(e.e_1, depth + 1, value); - } - } else { - // We've hit a node whose bucket ID corresponds exactly to that given - // in `value`. Find the first in-order child node, if one exists. - // Please see findFirstInOrderNodeInclusive() comments for an - // explanation of why this satisfies the upper bound ordering - // requirements. - // Due to Funky Business(tm) inside BucketId, asking for getBit beyond - // usedBits returns potentially undefined values, so we have to treat - // this case by itself. - int candidate = findFirstInOrderNodeInclusive(e.e_0); - if (candidate == -1) { - candidate = findFirstInOrderNodeInclusive(e.e_1); - } - return candidate; - } -} - -BucketDatabase::Entry -MapBucketDatabase::upperBound(const document::BucketId& value) const -{ - int index = upperBoundImpl(0, 0, value); - if (index != -1) { - assert(_db[index].value != -1); - return _values[_db[index].value]; - } - return Entry::createInvalid(); -} - -namespace { - -inline BucketDatabase::ConstEntryRef -to_entry_ref(const BucketDatabase::Entry& e) { - return BucketDatabase::ConstEntryRef( - e.getBucketId(), - ConstBucketInfoRef(e->getLastGarbageCollectionTime(), e->getRawNodes())); -} - -} - -bool -MapBucketDatabase::forEach(int index, - EntryProcessor& processor, - uint8_t bitCount, - const document::BucketId& lowerBound, - bool& process) const -{ - if (index == -1) { - return true; - } - - const E& e = _db[index]; - if (e.value != -1 && process - && !processor.process(to_entry_ref(_values[e.value]))) - { - return false; - } - - // We have followed the bucket to where we want to start, - // start processing. - if (!process && bitCount >= lowerBound.getUsedBits()) { - process = true; - } - - if (process || lowerBound.getBit(bitCount) == 0) { - if (!forEach(e.e_0, processor, bitCount + 1, lowerBound, process)) { - return false; - } - } - - if (process || lowerBound.getBit(bitCount) != 0) { - if (!forEach(e.e_1, processor, bitCount + 1, lowerBound, process)) { - return false; - } - } - - return true; -} - -void -MapBucketDatabase::forEach(EntryProcessor& processor, - const document::BucketId& after) const -{ - bool process = false; - forEach(0, processor, 0, after, process); -} - -struct MapDbMerger final : BucketDatabase::Merger { - MapBucketDatabase& _db; - BucketDatabase::Entry& _current_entry; - std::vector<BucketDatabase::Entry>& _to_insert; - - MapDbMerger(MapBucketDatabase& db, - BucketDatabase::Entry& current_entry, - std::vector<BucketDatabase::Entry>& to_insert) - : _db(db), - _current_entry(current_entry), - _to_insert(to_insert) - {} - - uint64_t bucket_key() const noexcept override { - return _current_entry.getBucketId().toKey(); - } - document::BucketId bucket_id() const noexcept override { - return _current_entry.getBucketId(); - } - BucketDatabase::Entry& current_entry() override { - return _current_entry; - } - void insert_before_current([[maybe_unused]] const document::BucketId& bucket_id, - const BucketDatabase::Entry& e) override { - _to_insert.emplace_back(e); // TODO movable - } -}; - -struct MapDbTrailingInserter final : BucketDatabase::TrailingInserter { - MapBucketDatabase& _db; - explicit MapDbTrailingInserter(MapBucketDatabase& db) : _db(db) {} - - void insert_at_end([[maybe_unused]] const document::BucketId& bucket_id, - const BucketDatabase::Entry& e) override { - _db.update(e); - } -}; - -void MapBucketDatabase::merge_internal(int index, - MergingProcessor& processor, - std::vector<Entry>& to_insert, - std::vector<document::BucketId>& to_remove) -{ - if (index == -1) { - return; - } - E& e = _db[index]; - if (e.value != -1) { - Entry& entry = _values[e.value]; - MapDbMerger merger(*this, entry, to_insert); - auto result = processor.merge(merger); - if (result == MergingProcessor::Result::KeepUnchanged) { - // No-op - } else if (result == MergingProcessor::Result::Update) { - // Also no-op since it's all in-place - } else if (result == MergingProcessor::Result::Skip) { - to_remove.emplace_back(entry.getBucketId()); - } - } - merge_internal(e.e_0, processor, to_insert, to_remove); - merge_internal(e.e_1, processor, to_insert, to_remove); -} - -void MapBucketDatabase::merge(MergingProcessor& processor) { - std::vector<document::BucketId> to_remove; - std::vector<Entry> to_insert; - merge_internal(0, processor, to_insert, to_remove); - for (const auto& bucket : to_remove) { - remove(bucket); - } - for (auto& e : to_insert) { - update_internal(std::move(e)); - } - MapDbTrailingInserter inserter(*this); - processor.insert_remaining_at_end(inserter); -} - -void -MapBucketDatabase::clear() -{ - _db.clear(); - _values.clear(); - _free.clear(); - _freeValues.clear(); - allocate(); -} - -uint8_t -MapBucketDatabase::getHighestSplitBit(int index, - uint8_t bitCount, - const document::BucketId& bid, - uint8_t minCount) -{ - if (index == -1) { - return minCount; - } - - E& e = _db[index]; - if (bitCount == bid.getUsedBits()) { - return minCount; - } - - if (bid.getBit(bitCount) == 0) { - if (e.e_0 != -1) { - minCount = getHighestSplitBit(e.e_0, - bitCount + 1, - bid, - minCount); - } - - if (e.e_1 != -1) { - return std::max((int)minCount, bitCount + 1); - } - } else { - if (e.e_1 != -1) { - minCount = getHighestSplitBit(e.e_1, - bitCount + 1, - bid, - minCount); - } - - if (e.e_0 != -1) { - return std::max((int)minCount, bitCount + 1); - } - } - - return minCount; - -} - -document::BucketId -MapBucketDatabase::getAppropriateBucket( - uint16_t minBits, - const document::BucketId& bid) -{ - return document::BucketId(getHighestSplitBit(0, 0, bid, minBits), - bid.getRawId()); -} - -uint32_t -MapBucketDatabase::childCountImpl(int index, - uint8_t bitCount, - const document::BucketId& b) const -{ - if (index == -1) { - // A non-existing node cannot have any subtrees (obviously). - return 0; - } - const E& e(_db[index]); - if (bitCount == b.getUsedBits()) { - // If a child has a valid index, it counts as a subtree. - return ((e.e_0 != -1) + (e.e_1 != -1)); - } - if (b.getBit(bitCount) == 0) { - return childCountImpl(e.e_0, bitCount + 1, b); - } else { - return childCountImpl(e.e_1, bitCount + 1, b); - } -} - -uint32_t -MapBucketDatabase::childCount(const document::BucketId& b) const -{ - return childCountImpl(0, 0, b); -} - - -namespace { - struct Writer : public BucketDatabase::EntryProcessor { - std::ostream& _ost; - explicit Writer(std::ostream& ost) : _ost(ost) {} - bool process(const BucketDatabase::ConstEntryRef& e) override { - _ost << e.toString() << "\n"; - return true; - } - }; -} - -void -MapBucketDatabase::print(std::ostream& out, bool verbose, - const std::string& indent) const -{ - out << "MapBucketDatabase("; - (void) indent; - if (verbose) { - Writer writer(out); - forEach(writer); - } else { - out << "Size(" << size() << ") Nodes(" - << (_db.size() - _free.size() - 1) << ")"; - } - out << ')'; -} - -std::unique_ptr<bucketdb::ReadGuard<BucketDatabase::Entry>> MapBucketDatabase::acquire_read_guard() const { - return std::make_unique<ReadGuardImpl>(*this); -} - -void MapBucketDatabase::ReadGuardImpl::find_parents_and_self(const document::BucketId& bucket, - std::vector<Entry>& entries) const -{ - _db->getParents(bucket, entries); -} - -namespace { - -template <typename T> -size_t allocated_by_vec(const T& vec) noexcept { - return (vec.capacity() * sizeof(typename T::value_type)); -} - -template <typename T> -size_t used_by_vec(const T& vec) noexcept { - return (vec.size() * sizeof(typename T::value_type)); -} - -} - -vespalib::MemoryUsage MapBucketDatabase::memory_usage() const noexcept { - // We don't have a concept of hold lists here, nor do we know the exact size of the - // entries on our free list (these wrap a secondary replica vector allocation). - // So we fudge the numbers a bit, returning a lower bound approximation only. - // That's OK since this is a legacy database that's on the way out anyway. - vespalib::MemoryUsage mem_usage; - mem_usage.incAllocatedBytes(allocated_by_vec(_values) + allocated_by_vec(_db)); - mem_usage.incUsedBytes(used_by_vec(_values) + used_by_vec(_db)); - mem_usage.incDeadBytes(allocated_by_vec(_free) + allocated_by_vec(_freeValues)); - return mem_usage; -} - -} // storage diff --git a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h deleted file mode 100644 index e8d5688068f..00000000000 --- a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h +++ /dev/null @@ -1,90 +0,0 @@ -// Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. -#pragma once - -#include "bucketdatabase.h" -#include <map> - -namespace storage { - -class MapBucketDatabase : public BucketDatabase -{ -public: - MapBucketDatabase(); - ~MapBucketDatabase(); - - Entry get(const document::BucketId& bucket) const override; - void remove(const document::BucketId& bucket) override; - void getParents(const document::BucketId& childBucket, std::vector<Entry>& entries) const override; - void getAll(const document::BucketId& bucket, std::vector<Entry>& entries) const override; - void update(const Entry& newEntry) override; - void forEach(EntryProcessor&, const document::BucketId& after = document::BucketId()) const override; - uint64_t size() const override { return _values.size() - _freeValues.size(); }; - void clear() override; - - uint32_t childCount(const document::BucketId&) const override; - Entry upperBound(const document::BucketId& value) const override; - - void merge(MergingProcessor&) override; - - document::BucketId getAppropriateBucket(uint16_t minBits, const document::BucketId& bid) override; - void print(std::ostream& out, bool verbose, const std::string& indent) const override; - - std::unique_ptr<bucketdb::ReadGuard<Entry>> acquire_read_guard() const override; - vespalib::MemoryUsage memory_usage() const noexcept override; -private: - struct E { - E() : value(-1), e_0(-1), e_1(-1) {}; - ~E(); - - bool empty() { - return (value == -1 && e_0 == -1 && e_1 == -1); - }; - - int value; - int e_0; - int e_1; - }; - - template <typename EntryType> - void update_internal(EntryType&& new_entry); - - BucketDatabase::Entry* find(int idx, uint8_t bitCount, const document::BucketId& bid, bool create); - bool remove(int index, uint8_t bitCount, const document::BucketId& bId); - int findFirstInOrderNodeInclusive(int index) const; - int upperBoundImpl(int index, uint8_t depth, const document::BucketId& value) const; - - bool forEach(int index, EntryProcessor& processor, uint8_t bitCount, - const document::BucketId& lowerBound, bool& process) const; - - void merge_internal(int index, MergingProcessor& processor, - std::vector<Entry>& to_insert, - std::vector<document::BucketId>& to_remove); - - void findParents(int index, uint8_t bitCount, const document::BucketId& bid, std::vector<Entry>& entries) const; - void findAll(int index, uint8_t bitCount, const document::BucketId& bid, std::vector<Entry>& entries) const; - uint8_t getHighestSplitBit(int index, uint8_t bitCount, const document::BucketId& bid, uint8_t minCount); - uint32_t childCountImpl(int index, uint8_t bitCount, const document::BucketId& b) const; - - // NOT thread-safe for concurrent reads! - class ReadGuardImpl final : public bucketdb::ReadGuard<Entry> { - const MapBucketDatabase* _db; - public: - explicit ReadGuardImpl(const MapBucketDatabase& db) : _db(&db) {} - ~ReadGuardImpl() override = default; - - void find_parents_and_self(const document::BucketId& bucket, - std::vector<Entry>& entries) const override; - uint64_t generation() const noexcept override { return 0; } - }; - - uint32_t allocate(); - uint32_t allocateValue(const document::BucketId& bid); - - std::vector<E> _db; - std::vector<uint32_t> _free; - - std::vector<BucketDatabase::Entry> _values; - std::vector<uint32_t> _freeValues; -}; - -} diff --git a/storage/src/vespa/storage/distributor/distributor.cpp b/storage/src/vespa/storage/distributor/distributor.cpp index 5080b16c727..c74d4135556 100644 --- a/storage/src/vespa/storage/distributor/distributor.cpp +++ b/storage/src/vespa/storage/distributor/distributor.cpp @@ -61,7 +61,6 @@ Distributor::Distributor(DistributorComponentRegister& compReg, framework::TickingThreadPool& threadPool, DoneInitializeHandler& doneInitHandler, bool manageActiveBucketCopies, - bool use_btree_database, HostInfo& hostInfoReporterRegistrar, ChainedMessageSender* messageSender) : StorageLink("distributor"), @@ -70,8 +69,8 @@ Distributor::Distributor(DistributorComponentRegister& compReg, _clusterStateBundle(lib::ClusterState()), _compReg(compReg), _component(compReg, "distributor"), - _bucketSpaceRepo(std::make_unique<DistributorBucketSpaceRepo>(use_btree_database)), - _readOnlyBucketSpaceRepo(std::make_unique<DistributorBucketSpaceRepo>(use_btree_database)), + _bucketSpaceRepo(std::make_unique<DistributorBucketSpaceRepo>()), + _readOnlyBucketSpaceRepo(std::make_unique<DistributorBucketSpaceRepo>()), _metrics(new DistributorMetricSet(_component.getLoadTypes()->getMetricLoadTypes())), _operationOwner(*this, _component.getClock()), _maintenanceOperationOwner(*this, _component.getClock()), @@ -106,8 +105,7 @@ Distributor::Distributor(DistributorComponentRegister& compReg, _db_memory_sample_interval(30s), _last_db_memory_sample_time_point(), _inhibited_maintenance_tick_count(0), - _must_send_updated_host_info(false), - _use_btree_database(use_btree_database) + _must_send_updated_host_info(false) { _component.registerMetric(*_metrics); _component.registerMetricUpdateHook(_metricUpdateHook, @@ -886,9 +884,8 @@ Distributor::enableNextConfig() _ownershipSafeTimeCalc->setMaxClusterClockSkew(getConfig().getMaxClusterClockSkew()); _pendingMessageTracker.setNodeBusyDuration(getConfig().getInhibitMergesOnBusyNodeDuration()); _bucketDBUpdater.set_stale_reads_enabled(getConfig().allowStaleReadsDuringClusterStateTransitions()); - // Concurrent reads are only safe if the B-tree DB implementation is used. _externalOperationHandler.set_concurrent_gets_enabled( - _use_btree_database && getConfig().allowStaleReadsDuringClusterStateTransitions()); + getConfig().allowStaleReadsDuringClusterStateTransitions()); _externalOperationHandler.set_use_weak_internal_read_consistency_for_gets( getConfig().use_weak_internal_read_consistency_for_client_gets()); } diff --git a/storage/src/vespa/storage/distributor/distributor.h b/storage/src/vespa/storage/distributor/distributor.h index 36d399b0a9f..84e195fdff2 100644 --- a/storage/src/vespa/storage/distributor/distributor.h +++ b/storage/src/vespa/storage/distributor/distributor.h @@ -51,7 +51,6 @@ public: framework::TickingThreadPool&, DoneInitializeHandler&, bool manageActiveBucketCopies, - bool use_btree_database, HostInfo& hostInfoReporterRegistrar, ChainedMessageSender* = nullptr); @@ -341,7 +340,6 @@ private: std::chrono::steady_clock::time_point _last_db_memory_sample_time_point; size_t _inhibited_maintenance_tick_count; bool _must_send_updated_host_info; - const bool _use_btree_database; }; } // distributor diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp index 3f7dbda62d9..7b7970228e7 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.cpp @@ -1,26 +1,14 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. #include "distributor_bucket_space.h" -#include <vespa/storage/bucketdb/mapbucketdatabase.h> #include <vespa/storage/bucketdb/btree_bucket_database.h> #include <vespa/vdslib/state/clusterstate.h> #include <vespa/vdslib/distribution/distribution.h> namespace storage::distributor { -namespace { - -std::unique_ptr<BucketDatabase> make_default_db_impl(bool use_btree_db) { - if (use_btree_db) { - return std::make_unique<BTreeBucketDatabase>(); - } - return std::make_unique<MapBucketDatabase>(); -} - -} - -DistributorBucketSpace::DistributorBucketSpace(bool use_btree_db) - : _bucketDatabase(make_default_db_impl(use_btree_db)), +DistributorBucketSpace::DistributorBucketSpace() + : _bucketDatabase(std::make_unique<BTreeBucketDatabase>()), _clusterState(), _distribution() { diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space.h b/storage/src/vespa/storage/distributor/distributor_bucket_space.h index 8fbb99dfe89..c137414ecfb 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space.h @@ -30,7 +30,7 @@ class DistributorBucketSpace { std::shared_ptr<const lib::ClusterState> _clusterState; std::shared_ptr<const lib::Distribution> _distribution; public: - explicit DistributorBucketSpace(bool use_btree_db); + explicit DistributorBucketSpace(); ~DistributorBucketSpace(); DistributorBucketSpace(const DistributorBucketSpace&) = delete; diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp index 54287d32666..744c54676ae 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.cpp @@ -13,11 +13,11 @@ using document::BucketSpace; namespace storage::distributor { -DistributorBucketSpaceRepo::DistributorBucketSpaceRepo(bool use_btree_db) +DistributorBucketSpaceRepo::DistributorBucketSpaceRepo() : _map() { - add(document::FixedBucketSpaces::default_space(), std::make_unique<DistributorBucketSpace>(use_btree_db)); - add(document::FixedBucketSpaces::global_space(), std::make_unique<DistributorBucketSpace>(use_btree_db)); + add(document::FixedBucketSpaces::default_space(), std::make_unique<DistributorBucketSpace>()); + add(document::FixedBucketSpaces::global_space(), std::make_unique<DistributorBucketSpace>()); } DistributorBucketSpaceRepo::~DistributorBucketSpaceRepo() = default; diff --git a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h index bc42ae8bb3a..ee36842969a 100644 --- a/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h +++ b/storage/src/vespa/storage/distributor/distributor_bucket_space_repo.h @@ -19,7 +19,7 @@ private: BucketSpaceMap _map; public: - explicit DistributorBucketSpaceRepo(bool use_btree_db); // TODO remove param once B-tree is default + DistributorBucketSpaceRepo(); ~DistributorBucketSpaceRepo(); DistributorBucketSpaceRepo(const DistributorBucketSpaceRepo&&) = delete; diff --git a/storage/src/vespa/storage/frameworkimpl/component/distributorcomponentregisterimpl.h b/storage/src/vespa/storage/frameworkimpl/component/distributorcomponentregisterimpl.h index c2a92dd2c69..61fba7f9921 100644 --- a/storage/src/vespa/storage/frameworkimpl/component/distributorcomponentregisterimpl.h +++ b/storage/src/vespa/storage/frameworkimpl/component/distributorcomponentregisterimpl.h @@ -8,7 +8,6 @@ #pragma once #include "storagecomponentregisterimpl.h" -#include <vespa/storage/bucketdb/mapbucketdatabase.h> #include <vespa/storage/common/distributorcomponent.h> #include <vespa/storage/common/nodestateupdater.h> @@ -34,7 +33,7 @@ public: typedef std::unique_ptr<DistributorComponentRegisterImpl> UP; DistributorComponentRegisterImpl(); - ~DistributorComponentRegisterImpl(); + ~DistributorComponentRegisterImpl() override; void registerDistributorComponent(DistributorManagedComponent&) override; void setTimeCalculator(UniqueTimeCalculator& calc); diff --git a/storage/src/vespa/storage/storageserver/distributornode.cpp b/storage/src/vespa/storage/storageserver/distributornode.cpp index 20b5dd641f7..1cd1477e769 100644 --- a/storage/src/vespa/storage/storageserver/distributornode.cpp +++ b/storage/src/vespa/storage/storageserver/distributornode.cpp @@ -19,7 +19,6 @@ DistributorNode::DistributorNode( DistributorNodeContext& context, ApplicationGenerationFetcher& generationFetcher, NeedActiveState activeState, - bool use_btree_database, StorageLink::UP communicationManager) : StorageNode(configUri, context, generationFetcher, std::unique_ptr<HostInfo>(new HostInfo()), @@ -30,7 +29,6 @@ DistributorNode::DistributorNode( _lastUniqueTimestampRequested(0), _uniqueTimestampCounter(0), _manageActiveBucketCopies(activeState == NEED_ACTIVE_BUCKET_STATES_SET), - _use_btree_database(use_btree_database), _retrievedCommunicationManager(std::move(communicationManager)) { try{ @@ -110,7 +108,6 @@ DistributorNode::createChain() new storage::distributor::Distributor( dcr, *_threadPool, getDoneInitializeHandler(), _manageActiveBucketCopies, - _use_btree_database, stateManager->getHostInfo()))); chain->push_back(StorageLink::UP(stateManager.release())); diff --git a/storage/src/vespa/storage/storageserver/distributornode.h b/storage/src/vespa/storage/storageserver/distributornode.h index 3a81e31fc56..4db8876dc24 100644 --- a/storage/src/vespa/storage/storageserver/distributornode.h +++ b/storage/src/vespa/storage/storageserver/distributornode.h @@ -24,7 +24,6 @@ class DistributorNode uint64_t _lastUniqueTimestampRequested; uint32_t _uniqueTimestampCounter; bool _manageActiveBucketCopies; - bool _use_btree_database; std::unique_ptr<StorageLink> _retrievedCommunicationManager; public: @@ -39,7 +38,6 @@ public: DistributorNodeContext&, ApplicationGenerationFetcher& generationFetcher, NeedActiveState, - bool use_btree_database, std::unique_ptr<StorageLink> communicationManager); ~DistributorNode() override; diff --git a/storageserver/src/vespa/storageserver/app/distributorprocess.cpp b/storageserver/src/vespa/storageserver/app/distributorprocess.cpp index 938ca63b50f..d9972116559 100644 --- a/storageserver/src/vespa/storageserver/app/distributorprocess.cpp +++ b/storageserver/src/vespa/storageserver/app/distributorprocess.cpp @@ -11,8 +11,7 @@ namespace storage { DistributorProcess::DistributorProcess(const config::ConfigUri & configUri) : Process(configUri), - _activeFlag(DistributorNode::NO_NEED_FOR_ACTIVE_STATES), - _use_btree_database(false) + _activeFlag(DistributorNode::NO_NEED_FOR_ACTIVE_STATES) { } @@ -39,8 +38,6 @@ DistributorProcess::setupConfig(milliseconds subscribeTimeout) if (stor_config->persistenceProvider.type != StorServerConfig::PersistenceProvider::Type::STORAGE) { _activeFlag = DistributorNode::NEED_ACTIVE_BUCKET_STATES_SET; } - auto dist_config = config::ConfigGetter<StorDistributormanagerConfig>::getConfig(_configUri.getConfigId(), _configUri.getContext(), subscribeTimeout); - _use_btree_database = dist_config->useBtreeDatabase; _distributorConfigHandler = _configSubscriber.subscribe<StorDistributormanagerConfig>(_configUri.getConfigId(), subscribeTimeout); _visitDispatcherConfigHandler = _configSubscriber.subscribe<StorVisitordispatcherConfig>(_configUri.getConfigId(), subscribeTimeout); Process::setupConfig(subscribeTimeout); @@ -76,7 +73,7 @@ DistributorProcess::configUpdated() void DistributorProcess::createNode() { - _node.reset(new DistributorNode(_configUri, _context, *this, _activeFlag, _use_btree_database, StorageLink::UP())); + _node.reset(new DistributorNode(_configUri, _context, *this, _activeFlag, StorageLink::UP())); _node->handleConfigChange(*_distributorConfigHandler->getConfig()); _node->handleConfigChange(*_visitDispatcherConfigHandler->getConfig()); } diff --git a/storageserver/src/vespa/storageserver/app/distributorprocess.h b/storageserver/src/vespa/storageserver/app/distributorprocess.h index e416f285268..4a2289e6151 100644 --- a/storageserver/src/vespa/storageserver/app/distributorprocess.h +++ b/storageserver/src/vespa/storageserver/app/distributorprocess.h @@ -15,7 +15,6 @@ namespace storage { class DistributorProcess final : public Process { DistributorNodeContext _context; DistributorNode::NeedActiveState _activeFlag; - bool _use_btree_database; DistributorNode::UP _node; config::ConfigHandle<vespa::config::content::core::StorDistributormanagerConfig>::UP _distributorConfigHandler; |