Merge pull request ceph#49748 from ifed01/wip-ifed-bound-rm-range-keys

kv/RocksDBStore: use bounded iterators in rm_range_keys Reviewed-by: Cory Snyder <[email protected]>
ssbandjl · Mar 22, 2023 · 369aaa7 · 369aaa7
2 parents 2042e95 + b8e669d
commit 369aaa7
Show file tree

Hide file tree

Showing 2 changed files with 34 additions and 17 deletions.
diff --git a/src/kv/RocksDBStore.cc b/src/kv/RocksDBStore.cc
@@ -1729,7 +1729,7 @@ void RocksDBStore::RocksDBTransactionImpl::rmkeys_by_prefix(const string &prefix
       uint64_t cnt = db->get_delete_range_threshold();
       bat.SetSavePoint();
       auto it = db->new_shard_iterator(cf);
-      for (it->SeekToFirst(); it->Valid() && (--cnt) != 0; it->Next()) {
+      for (it->seek_to_first(); it->valid() && (--cnt) != 0; it->next()) {
 	bat.Delete(cf, it->key());
       }
       if (cnt == 0) {
@@ -1747,10 +1747,10 @@ void RocksDBStore::RocksDBTransactionImpl::rm_range_keys(const string &prefix,
                                                          const string &start,
                                                          const string &end)
 {
-  ldout(db->cct, 10) << __func__ 
+  ldout(db->cct, 10) << __func__
                      << " enter prefix=" << prefix
-                     << " start=" << start
-		     << " end=" << end << dendl;
+                     << " start=" << pretty_binary_string(start)
+		     << " end=" << pretty_binary_string(end) << dendl;
   auto p_iter = db->cf_handles.find(prefix);
   uint64_t cnt = db->get_delete_range_threshold();
   if (p_iter == db->cf_handles.end()) {
@@ -1762,8 +1762,8 @@ void RocksDBStore::RocksDBTransactionImpl::rm_range_keys(const string &prefix,
 	 it->next()) {
       bat.Delete(db->default_cf, combine_strings(prefix, it->key()));
     }
-    ldout(db->cct, 15) << __func__ << " count = "
-                       << cnt0 - cnt
+    ldout(db->cct, 15) << __func__
+                       << " count = " << cnt0 - cnt
                        << dendl;
     if (cnt == 0) {
       ldout(db->cct, 10) << __func__ << " p_iter == end(), resorting to DeleteRange"
@@ -1783,20 +1783,22 @@ void RocksDBStore::RocksDBTransactionImpl::rm_range_keys(const string &prefix,
 	bat.DeleteRange(cf, rocksdb::Slice(start), rocksdb::Slice(end));
     }
   } else {
+    auto bounds = KeyValueDB::IteratorBounds();
+    bounds.lower_bound = start;
+    bounds.upper_bound = end;
     ceph_assert(p_iter->second.handles.size() >= 1);
     for (auto cf : p_iter->second.handles) {
       cnt = db->get_delete_range_threshold();
       uint64_t cnt0 = cnt;
       bat.SetSavePoint();
-      rocksdb::Iterator* it = db->new_shard_iterator(cf);
-      ceph_assert(it != nullptr);
-      for (it->Seek(start);
-	   it->Valid() && db->comparator->Compare(it->key(), end) < 0 && (--cnt) != 0;
-	   it->Next()) {
+      auto it = db->new_shard_iterator(cf, prefix, bounds);
+      for (it->lower_bound(start);
+	   it->valid() && (--cnt) != 0;
+	   it->next()) {
 	bat.Delete(cf, it->key());
       }
-      ldout(db->cct, 10) << __func__ << " count = "
-                         << cnt0 - cnt
+      ldout(db->cct, 10) << __func__
+                         << " count = " << cnt0 - cnt
                          << dendl;
       if (cnt == 0) {
         ldout(db->cct, 10) << __func__ << " p_iter != end(), resorting to DeleteRange"
@@ -1806,7 +1808,6 @@ void RocksDBStore::RocksDBTransactionImpl::rm_range_keys(const string &prefix,
       } else {
 	bat.PopSavePoint();
       }
-      delete it;
     }
   }
   ldout(db->cct, 10) << __func__ << " end" << dendl;
@@ -3038,9 +3039,23 @@ KeyValueDB::Iterator RocksDBStore::get_iterator(const std::string& prefix, Itera
   }
 }
 
-rocksdb::Iterator* RocksDBStore::new_shard_iterator(rocksdb::ColumnFamilyHandle* cf)
+RocksDBStore::WholeSpaceIterator RocksDBStore::new_shard_iterator(rocksdb::ColumnFamilyHandle* cf)
+{
+  return std::make_shared<RocksDBWholeSpaceIteratorImpl>(
+    this,
+    cf,
+    0);
+}
+
+KeyValueDB::Iterator RocksDBStore::new_shard_iterator(rocksdb::ColumnFamilyHandle* cf,
+						      const std::string& prefix,
+						      IteratorBounds bounds)
 {
-  return db->NewIterator(rocksdb::ReadOptions(), cf);
+  return std::make_shared<CFIteratorImpl>(
+    this,
+    prefix,
+    cf,
+    std::move(bounds));
 }
 
 RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorOpts opts)

diff --git a/src/kv/RocksDBStore.h b/src/kv/RocksDBStore.h
@@ -390,7 +390,9 @@ class RocksDBStore : public KeyValueDB {
   Iterator get_iterator(const std::string& prefix, IteratorOpts opts = 0, IteratorBounds = IteratorBounds()) override;
 private:
   /// this iterator spans single cf
-  rocksdb::Iterator* new_shard_iterator(rocksdb::ColumnFamilyHandle* cf);
+  WholeSpaceIterator new_shard_iterator(rocksdb::ColumnFamilyHandle* cf);
+  Iterator new_shard_iterator(rocksdb::ColumnFamilyHandle* cf,
+			      const std::string& prefix, IteratorBounds bound);
 public:
   /// Utility
   static std::string combine_strings(const std::string &prefix, const std::string &value) {