From 9cdb2c1c86b2ce5c25cc5d25145659b7d0a2b212 Mon Sep 17 00:00:00 2001 From: Adam Kupczyk Date: Fri, 29 Apr 2022 23:32:43 +0200 Subject: [PATCH] kv/RocksDBStore: Remove feature to make WholeSpaceIterator based on bounded iterator Iterator-bounding feature is introduced to make RocksDB iterators limited, so they would less likely traverse over tombstones. This is used when listing keys in fixed range, for example OMAPS for specific object. It is problematic when extending this logic to WholeSpaceIterator, since prefix must be taken into account. Fixes: https://tracker.ceph.com/issues/55444 Signed-off-by: Adam Kupczyk --- src/kv/KeyValueDB.h | 4 ++-- src/kv/LevelDBStore.h | 2 +- src/kv/MemDB.h | 2 +- src/kv/RocksDBStore.cc | 8 ++++---- src/kv/RocksDBStore.h | 20 ++------------------ src/test/ObjectMap/KeyValueDBMemory.cc | 2 +- src/test/ObjectMap/KeyValueDBMemory.h | 2 +- 7 files changed, 12 insertions(+), 28 deletions(-) diff --git a/src/kv/KeyValueDB.h b/src/kv/KeyValueDB.h index 2ecccf8507134..9e3cb3c3d0bbc 100644 --- a/src/kv/KeyValueDB.h +++ b/src/kv/KeyValueDB.h @@ -321,11 +321,11 @@ public: std::optional upper_bound; }; - virtual WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) = 0; + virtual WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) = 0; virtual Iterator get_iterator(const std::string &prefix, IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) { return std::make_shared( prefix, - get_wholespace_iterator(opts, std::move(bounds))); + get_wholespace_iterator(opts)); } virtual uint64_t get_estimated_size(std::map &extra) = 0; diff --git a/src/kv/LevelDBStore.h b/src/kv/LevelDBStore.h index b48642d12c34a..085193ee03321 100644 --- a/src/kv/LevelDBStore.h +++ b/src/kv/LevelDBStore.h @@ -402,7 +402,7 @@ err: } - WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) override { + WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) override { return std::make_shared( db->NewIterator(leveldb::ReadOptions())); } diff --git a/src/kv/MemDB.h b/src/kv/MemDB.h index 80fed647efbdd..32d81db225ed2 100644 --- a/src/kv/MemDB.h +++ b/src/kv/MemDB.h @@ -212,7 +212,7 @@ public: return 0; } - WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) override { + WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) override { return std::shared_ptr( new MDBWholeSpaceIteratorImpl(&m_map, &m_lock, &iterator_seq_no, m_using_btree)); } diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc index 4c079d3d2333a..05bcdaa26627b 100644 --- a/src/kv/RocksDBStore.cc +++ b/src/kv/RocksDBStore.cc @@ -3007,7 +3007,7 @@ KeyValueDB::Iterator RocksDBStore::get_iterator(const std::string& prefix, Itera std::move(bounds)); } } else { - return KeyValueDB::get_iterator(prefix, opts, std::move(bounds)); + return KeyValueDB::get_iterator(prefix, opts); } } @@ -3016,11 +3016,11 @@ rocksdb::Iterator* RocksDBStore::new_shard_iterator(rocksdb::ColumnFamilyHandle* return db->NewIterator(rocksdb::ReadOptions(), cf); } -RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorOpts opts, IteratorBounds bounds) +RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorOpts opts) { if (cf_handles.size() == 0) { return std::make_shared( - this, default_cf, opts, std::move(bounds)); + this, default_cf, opts); } else { return std::make_shared(this); } @@ -3028,7 +3028,7 @@ RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorO RocksDBStore::WholeSpaceIterator RocksDBStore::get_default_cf_iterator() { - return std::make_shared(this, default_cf, 0, IteratorBounds()); + return std::make_shared(this, default_cf, 0); } int RocksDBStore::prepare_for_reshard(const std::string& new_sharding, diff --git a/src/kv/RocksDBStore.h b/src/kv/RocksDBStore.h index a64e69b1d1d6c..50f2b4488f4dc 100644 --- a/src/kv/RocksDBStore.h +++ b/src/kv/RocksDBStore.h @@ -354,32 +354,16 @@ public: public KeyValueDB::WholeSpaceIteratorImpl { protected: rocksdb::Iterator *dbiter; - const KeyValueDB::IteratorBounds bounds; - const rocksdb::Slice iterate_lower_bound; - const rocksdb::Slice iterate_upper_bound; public: explicit RocksDBWholeSpaceIteratorImpl(const RocksDBStore* db, rocksdb::ColumnFamilyHandle* cf, - const KeyValueDB::IteratorOpts opts, - KeyValueDB::IteratorBounds bounds_) : - bounds(std::move(bounds_)), - iterate_lower_bound(make_slice(bounds.lower_bound)), - iterate_upper_bound(make_slice(bounds.upper_bound)) + const KeyValueDB::IteratorOpts opts) { rocksdb::ReadOptions options = rocksdb::ReadOptions(); if (opts & ITERATOR_NOCACHE) options.fill_cache=false; - if (db->cct->_conf->osd_rocksdb_iterator_bounds_enabled) { - if (bounds.lower_bound) { - options.iterate_lower_bound = &iterate_lower_bound; - } - if (bounds.upper_bound) { - options.iterate_upper_bound = &iterate_upper_bound; - } - } dbiter = db->db->NewIterator(options, cf); } - //virtual ~RocksDBWholeSpaceIteratorImpl() { } ~RocksDBWholeSpaceIteratorImpl() override; int seek_to_first() override; @@ -534,7 +518,7 @@ err: return nullptr; } - WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) override; + WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) override; private: WholeSpaceIterator get_default_cf_iterator(); diff --git a/src/test/ObjectMap/KeyValueDBMemory.cc b/src/test/ObjectMap/KeyValueDBMemory.cc index b2b351baef44b..234e963397e31 100644 --- a/src/test/ObjectMap/KeyValueDBMemory.cc +++ b/src/test/ObjectMap/KeyValueDBMemory.cc @@ -234,7 +234,7 @@ int KeyValueDBMemory::rm_range_keys(const string &prefix, const string &start, c return 0; } -KeyValueDB::WholeSpaceIterator KeyValueDBMemory::get_wholespace_iterator(IteratorOpts opts, IteratorBounds bounds) { +KeyValueDB::WholeSpaceIterator KeyValueDBMemory::get_wholespace_iterator(IteratorOpts opts) { return std::shared_ptr( new WholeSpaceMemIterator(this) ); diff --git a/src/test/ObjectMap/KeyValueDBMemory.h b/src/test/ObjectMap/KeyValueDBMemory.h index da386ccb908bf..5e7d3f0e1675f 100644 --- a/src/test/ObjectMap/KeyValueDBMemory.h +++ b/src/test/ObjectMap/KeyValueDBMemory.h @@ -186,5 +186,5 @@ private: friend class WholeSpaceMemIterator; public: - WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) override; + WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) override; }; -- 2.39.5