diff options
author | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2019-08-19 12:23:07 +0000 |
---|---|---|
committer | Tor Brede Vekterli <vekterli@verizonmedia.com> | 2019-08-19 12:31:45 +0000 |
commit | a945757c1306601ce92905f7fb6e9c836dd4efde (patch) | |
tree | b4f1bceead9bcb3707004ba56e7fc558d85f37d1 /storage | |
parent | 1852407b2f1219eff0eb4aeb50a7c6c7440d2c28 (diff) |
Add minimal snapshot read guard interface to bucket DB
Only exposes enough functionality to be used for Get operations for now.
Enable free-lists for underlying replica `ArrayStore`.
Legacy `MapBucketDatabase` read guard is _not_ thread safe, as it will never
be used for non-blocking reads.
Diffstat (limited to 'storage')
6 files changed, 111 insertions, 18 deletions
diff --git a/storage/src/tests/distributor/bucketdatabasetest.cpp b/storage/src/tests/distributor/bucketdatabasetest.cpp index 6225865b153..43b0e041f03 100644 --- a/storage/src/tests/distributor/bucketdatabasetest.cpp +++ b/storage/src/tests/distributor/bucketdatabasetest.cpp @@ -170,8 +170,15 @@ BucketDatabaseTest::doFindParents(const std::vector<document::BucketId>& ids, } std::vector<BucketDatabase::Entry> entries; + // TODO remove in favor of only read guard once legacy DB usage has been ported over db().getParents(searchId, entries); + std::vector<BucketDatabase::Entry> checked_entries; + db().acquire_read_guard()->find_parents_and_self(searchId, checked_entries); + if(entries != checked_entries) { + return "Mismatch between results from getParents() and ReadGuard!"; + } + std::ostringstream ost; for (uint32_t i = 0; i < ids.size(); ++i) { if (std::find(entries.begin(), entries.end(), diff --git a/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp b/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp index ef198cb1d3f..a901bbdd96b 100644 --- a/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp +++ b/storage/src/vespa/storage/bucketdb/btree_bucket_database.cpp @@ -42,14 +42,18 @@ using document::BucketId; BTreeBucketDatabase::BTreeBucketDatabase() : _tree(), - _store(ReplicaStore::optimizedConfigForHugePage(1023, vespalib::alloc::MemoryAllocator::HUGEPAGE_SIZE, - 4 * 1024, 8 * 1024, 0.2)), + _store(make_default_array_store_config()), _generation_handler() { } BTreeBucketDatabase::~BTreeBucketDatabase() = default; +search::datastore::ArrayStoreConfig BTreeBucketDatabase::make_default_array_store_config() { + return ReplicaStore::optimizedConfigForHugePage(1023, vespalib::alloc::MemoryAllocator::HUGEPAGE_SIZE, + 4 * 1024, 8 * 1024, 0.2).enable_free_lists(true); +} + namespace { Entry entry_from_replica_array_ref(const BucketId& id, uint32_t gc_timestamp, ConstArrayRef<BucketCopy> replicas) { @@ -232,11 +236,11 @@ void BTreeBucketDatabase::remove(const BucketId& bucket) { * skip scanning through many disjoint subtrees. */ BTreeBucketDatabase::BTree::ConstIterator -BTreeBucketDatabase::find_parents_internal(const document::BucketId& bucket, +BTreeBucketDatabase::find_parents_internal(const BTree::FrozenView& frozen_view, + const document::BucketId& bucket, std::vector<Entry>& entries) const { const uint64_t bucket_key = bucket.toKey(); - const auto frozen_view = _tree.getFrozenView(); auto iter = frozen_view.begin(); // Start at the root level, descending towards the bucket itself. // Try skipping as many levels of the tree as possible as we go. @@ -255,6 +259,16 @@ BTreeBucketDatabase::find_parents_internal(const document::BucketId& bucket, return iter; } +void BTreeBucketDatabase::find_parents_and_self_internal(const BTree::FrozenView& frozen_view, + const document::BucketId& bucket, + std::vector<Entry>& entries) const +{ + auto iter = find_parents_internal(frozen_view, bucket, entries); + if (iter.valid() && iter.getKey() == bucket.toKey()) { + entries.emplace_back(entry_from_iterator(iter)); + } +} + /* * Note: due to legacy API reasons, iff the requested bucket itself exists in the * tree, it will be returned in the result set. I.e. it returns all the nodes on @@ -263,16 +277,15 @@ BTreeBucketDatabase::find_parents_internal(const document::BucketId& bucket, void BTreeBucketDatabase::getParents(const BucketId& bucket, std::vector<Entry>& entries) const { - auto iter = find_parents_internal(bucket, entries); - if (iter.valid() && iter.getKey() == bucket.toKey()) { - entries.emplace_back(entry_from_iterator(iter)); - } + auto view = _tree.getFrozenView(); + find_parents_and_self_internal(view, bucket, entries); } void BTreeBucketDatabase::getAll(const BucketId& bucket, std::vector<Entry>& entries) const { - auto iter = find_parents_internal(bucket, entries); + auto view = _tree.getFrozenView(); + auto iter = find_parents_internal(view, bucket, entries); // `iter` is already pointing at, or beyond, one of the bucket's subtrees. for (; iter.valid(); ++iter) { auto candidate = BucketId(BucketId::keyToBucketId(iter.getKey())); @@ -510,4 +523,18 @@ void BTreeBucketDatabase::print(std::ostream& out, bool verbose, (void)indent; } +BTreeBucketDatabase::ReadGuardImpl::ReadGuardImpl(const BTreeBucketDatabase& db) + : _db(&db), + _guard(_db->_generation_handler.takeGuard()), + _frozen_view(_db->_tree.getFrozenView()) +{} + +BTreeBucketDatabase::ReadGuardImpl::~ReadGuardImpl() = default; + +void BTreeBucketDatabase::ReadGuardImpl::find_parents_and_self(const document::BucketId& bucket, + std::vector<Entry>& entries) const +{ + _db->find_parents_and_self_internal(_frozen_view, bucket, entries); +} + } diff --git a/storage/src/vespa/storage/bucketdb/btree_bucket_database.h b/storage/src/vespa/storage/bucketdb/btree_bucket_database.h index eb527071ad7..5c69b57956b 100644 --- a/storage/src/vespa/storage/bucketdb/btree_bucket_database.h +++ b/storage/src/vespa/storage/bucketdb/btree_bucket_database.h @@ -17,13 +17,9 @@ namespace storage { * this is an uint32 we cheekily mangle it into the value, i.e. each bucket key maps to a * composite key of (gc_timestamp_u32 << 32) | array_ref_u32. * - * This is _not_ yet production ready, for several reasons: - * - Underlying ArrayStore does not have freelists enabled for replica entry reuse - * - Current API design for mutable forEach requires O(n) tree structure mutations instead - * of changing the tree in bulk and reusing ArrayStore refs et al. Needs a redesign. - * - * Also note that the DB is currently _not_ thread safe, as read snapshotting is not yet defined - * or exposed. + * Readers from contexts that are not guaranteed to be the main distributor thread MUST + * only access the database via an acquired read guard. + * Writing MUST only take place from the main distributor thread. */ // TODO create and use a new DB interface with better bulk loading, snapshot and iteration support class BTreeBucketDatabase : public BucketDatabase { @@ -66,11 +62,34 @@ private: ConstEntryRef const_entry_ref_from_iterator(const BTree::ConstIterator& iter) const; document::BucketId bucket_from_valid_iterator(const BTree::ConstIterator& iter) const; void commit_tree_changes(); - BTree::ConstIterator find_parents_internal(const document::BucketId& bucket, + BTree::ConstIterator find_parents_internal(const BTree::FrozenView& frozen_view, + const document::BucketId& bucket, std::vector<Entry>& entries) const; + void find_parents_and_self_internal(const BTree::FrozenView& frozen_view, + const document::BucketId& bucket, + std::vector<Entry>& entries) const; + + static search::datastore::ArrayStoreConfig make_default_array_store_config(); + + class ReadGuardImpl : public ReadGuard { + const BTreeBucketDatabase* _db; + GenerationHandler::Guard _guard; + BTree::FrozenView _frozen_view; + public: + explicit ReadGuardImpl(const BTreeBucketDatabase& db); + ~ReadGuardImpl() override; + + void find_parents_and_self(const document::BucketId& bucket, + std::vector<Entry>& entries) const override; + }; + + friend class ReadGuardImpl; + + std::unique_ptr<ReadGuard> acquire_read_guard() const override { + return std::make_unique<ReadGuardImpl>(*this); + } friend struct BTreeBuilderMerger; - friend struct BTreeMergingBuilder; friend struct BTreeTrailingInserter; }; diff --git a/storage/src/vespa/storage/bucketdb/bucketdatabase.h b/storage/src/vespa/storage/bucketdb/bucketdatabase.h index d12c2d82972..0659d02cc15 100644 --- a/storage/src/vespa/storage/bucketdb/bucketdatabase.h +++ b/storage/src/vespa/storage/bucketdb/bucketdatabase.h @@ -232,6 +232,23 @@ public: const document::BucketId& bid); virtual uint32_t childCount(const document::BucketId&) const = 0; + + struct ReadGuard { + ReadGuard() = default; + virtual ~ReadGuard() = default; + + ReadGuard(ReadGuard&&) = delete; + ReadGuard& operator=(ReadGuard&&) = delete; + ReadGuard(const ReadGuard&) = delete; + ReadGuard& operator=(const ReadGuard&) = delete; + + virtual void find_parents_and_self(const document::BucketId& bucket, + std::vector<Entry>& entries) const = 0; + }; + + virtual std::unique_ptr<ReadGuard> acquire_read_guard() const { + return std::unique_ptr<ReadGuard>(); + } }; template <typename BucketInfoType> diff --git a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp index 9dad421324e..463e4a4b8ce 100644 --- a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp +++ b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.cpp @@ -584,4 +584,14 @@ MapBucketDatabase::print(std::ostream& out, bool verbose, out << ')'; } +std::unique_ptr<BucketDatabase::ReadGuard> 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); +} + } // storage diff --git a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h index df3e055bd7a..b295be588a6 100644 --- a/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h +++ b/storage/src/vespa/storage/bucketdb/mapbucketdatabase.h @@ -28,6 +28,8 @@ public: 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<ReadGuard> acquire_read_guard() const override; private: struct E { E() : value(-1), e_0(-1), e_1(-1) {}; @@ -62,6 +64,17 @@ private: 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 : public ReadGuard { + 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; + }; + uint32_t allocate(); uint32_t allocateValue(const document::BucketId& bid); |