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 <akupczyk@redhat.com>

diff --git a/src/kv/KeyValueDB.h b/src/kv/KeyValueDB.h
index 2ecccf85071342b94dbe37f9dcc7035a1d1e10f7..9e3cb3c3d0bbc35b60428686d602ead4b6dfd869 100644 (file)
--- a/src/kv/KeyValueDB.h
+++ b/src/kv/KeyValueDB.h
@@ -321,11 +321,11 @@ public:
     std::optional<std::string> 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<PrefixIteratorImpl>(
       prefix,
-      get_wholespace_iterator(opts, std::move(bounds)));
+      get_wholespace_iterator(opts));
   }
 
   virtual uint64_t get_estimated_size(std::map<std::string,uint64_t> &extra) = 0;

diff --git a/src/kv/LevelDBStore.h b/src/kv/LevelDBStore.h
index b48642d12c34a16aeb13c0b472668a4cca11ee3c..085193ee03321a5859928cb33371857be272690b 100644 (file)
--- 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<LevelDBWholeSpaceIteratorImpl>(
        db->NewIterator(leveldb::ReadOptions()));
   }

diff --git a/src/kv/MemDB.h b/src/kv/MemDB.h
index 80fed647efbdd6c7927cfc685150a0526a562100..32d81db225ed22b887377f84aadc699b7bcd6405 100644 (file)
--- 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<KeyValueDB::WholeSpaceIteratorImpl>(
       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 4c079d3d2333ab48875a74eee86bc02378e27337..05bcdaa26627b78aead2937cd0c889c203f53ecd 100644 (file)
--- 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<RocksDBWholeSpaceIteratorImpl>(
-      this, default_cf, opts, std::move(bounds));
+      this, default_cf, opts);
   } else {
     return std::make_shared<WholeMergeIteratorImpl>(this);
   }
@@ -3028,7 +3028,7 @@ RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorO
 
 RocksDBStore::WholeSpaceIterator RocksDBStore::get_default_cf_iterator()
 {
-  return std::make_shared<RocksDBWholeSpaceIteratorImpl>(this, default_cf, 0, IteratorBounds());
+  return std::make_shared<RocksDBWholeSpaceIteratorImpl>(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 a64e69b1d1d6cb0f6b1afef9e982d83dd8c6c01e..50f2b4488f4dcfa89e56187af0641d7bdda38641 100644 (file)
--- 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 b2b351baef44b01850c021022f6301195143ff4c..234e963397e31ea92c0748685d45f0995b00fecb 100644 (file)
--- 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<KeyValueDB::WholeSpaceIteratorImpl>(
     new WholeSpaceMemIterator(this)
   );

diff --git a/src/test/ObjectMap/KeyValueDBMemory.h b/src/test/ObjectMap/KeyValueDBMemory.h
index da386ccb908bfc3634520c40707f926b8b7a989b..5e7d3f0e1675f33f7c171ba1ce8d5df5672e3577 100644 (file)
--- 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;
 };