bluestore: set upper and lower bounds on rocksdb omap iterators

Limits RocksDB omap Seek operations to the relevant key range of the object's omap.
This prevents RocksDB from unnecessarily iterating over delete range tombstones in
irrelevant omap CF shards. Avoids extreme performance degradation commonly caused
by tombstones generated from RGW bucket resharding cleanup. Also prefer CFIteratorImpl
over ShardMergeIteratorImpl when we can determine that all keys within specified
IteratorBounds must be in a single CF.

Fixes: https://tracker.ceph.com/issues/55324
Signed-off-by: Cory Snyder <csnyder@iland.com>
(cherry picked from commit 850c16c2468c3200a340493c12930543f326b0e1)

diff --git a/src/kv/KeyValueDB.h b/src/kv/KeyValueDB.h
index a9a7965117c0dc0dc62d5e8af55df32aa008a185..2ecccf85071342b94dbe37f9dcc7035a1d1e10f7 100644 (file)
--- a/src/kv/KeyValueDB.h
+++ b/src/kv/KeyValueDB.h
@@ -7,6 +7,7 @@
 #include <ostream>
 #include <set>
 #include <map>
+#include <optional>
 #include <string>
 #include <boost/scoped_ptr.hpp>
 #include "include/encoding.h"
@@ -314,11 +315,17 @@ private:
 public:
   typedef uint32_t IteratorOpts;
   static const uint32_t ITERATOR_NOCACHE = 1;
-  virtual WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) = 0;
-  virtual Iterator get_iterator(const std::string &prefix, IteratorOpts opts = 0) {
+
+  struct IteratorBounds {
+    std::optional<std::string> lower_bound;
+    std::optional<std::string> upper_bound;
+  };
+
+  virtual WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) = 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));
+      get_wholespace_iterator(opts, std::move(bounds)));
   }
 
   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 085193ee03321a5859928cb33371857be272690b..b48642d12c34a16aeb13c0b472668a4cca11ee3c 100644 (file)
--- a/src/kv/LevelDBStore.h
+++ b/src/kv/LevelDBStore.h
@@ -402,7 +402,7 @@ err:
   }
 
 
-  WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) override {
+  WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) override {
     return std::make_shared<LevelDBWholeSpaceIteratorImpl>(
        db->NewIterator(leveldb::ReadOptions()));
   }

diff --git a/src/kv/MemDB.h b/src/kv/MemDB.h
index 32d81db225ed22b887377f84aadc699b7bcd6405..80fed647efbdd6c7927cfc685150a0526a562100 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) override {
+  WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) 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 7d7fee68c73d5f4f0eb53bb6ffaf4f64806f5185..dcfa645dc01c545ab541a27c542919ecee76d082 100644 (file)
--- a/src/kv/RocksDBStore.cc
+++ b/src/kv/RocksDBStore.cc
@@ -626,6 +626,18 @@ bool RocksDBStore::is_column_family(const std::string& prefix) {
   return cf_handles.count(prefix);
 }
 
+std::string_view RocksDBStore::get_key_hash_view(const prefix_shards& shards, const char* key, const size_t keylen) {
+  uint32_t hash_l = std::min<uint32_t>(shards.hash_l, keylen);
+  uint32_t hash_h = std::min<uint32_t>(shards.hash_h, keylen);
+  return { key + hash_l, hash_h - hash_l };
+}
+
+rocksdb::ColumnFamilyHandle *RocksDBStore::get_key_cf(const prefix_shards& shards, const char* key, const size_t keylen) {
+  auto sv = get_key_hash_view(shards, key, keylen);
+  uint32_t hash = ceph_str_hash_rjenkins(sv.data(), sv.size());
+  return shards.handles[hash % shards.handles.size()];
+}
+
 rocksdb::ColumnFamilyHandle *RocksDBStore::get_cf_handle(const std::string& prefix, const std::string& key) {
   auto iter = cf_handles.find(prefix);
   if (iter == cf_handles.end()) {
@@ -634,10 +646,7 @@ rocksdb::ColumnFamilyHandle *RocksDBStore::get_cf_handle(const std::string& pref
     if (iter->second.handles.size() == 1) {
       return iter->second.handles[0];
     } else {
-      uint32_t hash_l = std::min<uint32_t>(iter->second.hash_l, key.size());
-      uint32_t hash_h = std::min<uint32_t>(iter->second.hash_h, key.size());
-      uint32_t hash = ceph_str_hash_rjenkins(&key[hash_l], hash_h - hash_l);
-      return iter->second.handles[hash % iter->second.handles.size()];
+      return get_key_cf(iter->second, key.data(), key.size());
     }
   }
 }
@@ -650,10 +659,36 @@ rocksdb::ColumnFamilyHandle *RocksDBStore::get_cf_handle(const std::string& pref
     if (iter->second.handles.size() == 1) {
       return iter->second.handles[0];
     } else {
-      uint32_t hash_l = std::min<uint32_t>(iter->second.hash_l, keylen);
-      uint32_t hash_h = std::min<uint32_t>(iter->second.hash_h, keylen);
-      uint32_t hash = ceph_str_hash_rjenkins(&key[hash_l], hash_h - hash_l);
-      return iter->second.handles[hash % iter->second.handles.size()];
+      return get_key_cf(iter->second, key, keylen);
+    }
+  }
+}
+
+/**
+ * If the specified IteratorBounds arg has both an upper and a lower bound defined, and they have equal placement hash
+ * strings, we can be sure that the entire iteration range exists in a single CF. In that case, we return the relevant
+ * CF handle. In all other cases, we return a nullptr to indicate that the specified bounds cannot necessarily be mapped
+ * to a single CF.
+ */
+rocksdb::ColumnFamilyHandle *RocksDBStore::get_cf_handle(const std::string& prefix, const IteratorBounds& bounds) {
+  if (!bounds.lower_bound || !bounds.upper_bound) {
+    return nullptr;
+  }
+  auto iter = cf_handles.find(prefix);
+  if (iter == cf_handles.end() || iter->second.hash_l != 0) {
+    return nullptr;
+  } else {
+    if (iter->second.handles.size() == 1) {
+      return iter->second.handles[0];
+    } else {
+      auto lower_bound_hash_str = get_key_hash_view(iter->second, bounds.lower_bound->data(), bounds.lower_bound->size());
+      auto upper_bound_hash_str = get_key_hash_view(iter->second, bounds.upper_bound->data(), bounds.upper_bound->size());
+      if (lower_bound_hash_str == upper_bound_hash_str) {
+        auto key = *bounds.lower_bound;
+        return get_key_cf(iter->second, key.data(), key.size());
+      } else {
+        return nullptr;
+      }
     }
   }
 }
@@ -2202,10 +2237,27 @@ class CFIteratorImpl : public KeyValueDB::IteratorImpl {
 protected:
   string prefix;
   rocksdb::Iterator *dbiter;
+  const KeyValueDB::IteratorBounds bounds;
+  const rocksdb::Slice iterate_lower_bound;
+  const rocksdb::Slice iterate_upper_bound;
 public:
-  explicit CFIteratorImpl(const std::string& p,
-                                rocksdb::Iterator *iter)
-    : prefix(p), dbiter(iter) { }
+  explicit CFIteratorImpl(const RocksDBStore* db,
+                          const std::string& p,
+                          rocksdb::ColumnFamilyHandle* cf,
+                          KeyValueDB::IteratorBounds bounds_)
+    : prefix(p), bounds(std::move(bounds_)),
+      iterate_lower_bound(make_slice(bounds.lower_bound)),
+      iterate_upper_bound(make_slice(bounds.upper_bound))
+      {
+      auto options = rocksdb::ReadOptions();
+      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);
+  }
   ~CFIteratorImpl() {
     delete dbiter;
   }
@@ -2737,16 +2789,29 @@ private:
   const RocksDBStore* db;
   KeyLess keyless;
   string prefix;
+  const KeyValueDB::IteratorBounds bounds;
+  const rocksdb::Slice iterate_lower_bound;
+  const rocksdb::Slice iterate_upper_bound;
   std::vector<rocksdb::Iterator*> iters;
 public:
   explicit ShardMergeIteratorImpl(const RocksDBStore* db,
                                  const std::string& prefix,
-                                 const std::vector<rocksdb::ColumnFamilyHandle*>& shards)
-    : db(db), keyless(db->comparator), prefix(prefix)
+                                 const std::vector<rocksdb::ColumnFamilyHandle*>& shards,
+                  KeyValueDB::IteratorBounds bounds_)
+    : db(db), keyless(db->comparator), prefix(prefix), bounds(std::move(bounds_)),
+      iterate_lower_bound(make_slice(bounds.lower_bound)),
+      iterate_upper_bound(make_slice(bounds.upper_bound))
   {
     iters.reserve(shards.size());
+    auto options = rocksdb::ReadOptions();
+    if (bounds.lower_bound) {
+      options.iterate_lower_bound = &iterate_lower_bound;
+    }
+    if (bounds.upper_bound) {
+      options.iterate_upper_bound = &iterate_upper_bound;
+    }
     for (auto& s : shards) {
-      iters.push_back(db->db->NewIterator(rocksdb::ReadOptions(), s));
+      iters.push_back(db->db->NewIterator(options, s));
     }
   }
   ~ShardMergeIteratorImpl() {
@@ -2917,22 +2982,31 @@ public:
   }
 };
 
-KeyValueDB::Iterator RocksDBStore::get_iterator(const std::string& prefix, IteratorOpts opts)
+KeyValueDB::Iterator RocksDBStore::get_iterator(const std::string& prefix, IteratorOpts opts, IteratorBounds bounds)
 {
   auto cf_it = cf_handles.find(prefix);
   if (cf_it != cf_handles.end()) {
+    rocksdb::ColumnFamilyHandle* cf = nullptr;
     if (cf_it->second.handles.size() == 1) {
+      cf = cf_it->second.handles[0];
+    } else {
+      cf = get_cf_handle(prefix, bounds);
+    }
+    if (cf) {
       return std::make_shared<CFIteratorImpl>(
-        prefix,
-        db->NewIterator(rocksdb::ReadOptions(), cf_it->second.handles[0]));
+              this,
+              prefix,
+              cf,
+              std::move(bounds));
     } else {
       return std::make_shared<ShardMergeIteratorImpl>(
         this,
         prefix,
-        cf_it->second.handles);
+        cf_it->second.handles,
+        std::move(bounds));
     }
   } else {
-    return KeyValueDB::get_iterator(prefix, opts);
+    return KeyValueDB::get_iterator(prefix, opts, std::move(bounds));
   }
 }
 
@@ -2941,14 +3015,11 @@ rocksdb::Iterator* RocksDBStore::new_shard_iterator(rocksdb::ColumnFamilyHandle*
   return db->NewIterator(rocksdb::ReadOptions(), cf);
 }
 
-RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorOpts opts)
+RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorOpts opts, IteratorBounds bounds)
 {
   if (cf_handles.size() == 0) {
-    rocksdb::ReadOptions opt = rocksdb::ReadOptions();
-    if (opts & ITERATOR_NOCACHE)
-      opt.fill_cache=false;
     return std::make_shared<RocksDBWholeSpaceIteratorImpl>(
-      db->NewIterator(opt, default_cf));
+      this, default_cf, opts, std::move(bounds));
   } else {
     return std::make_shared<WholeMergeIteratorImpl>(this);
   }
@@ -2956,8 +3027,7 @@ RocksDBStore::WholeSpaceIterator RocksDBStore::get_wholespace_iterator(IteratorO
 
 RocksDBStore::WholeSpaceIterator RocksDBStore::get_default_cf_iterator()
 {
-  return std::make_shared<RocksDBWholeSpaceIteratorImpl>(
-    db->NewIterator(rocksdb::ReadOptions(), default_cf));
+  return std::make_shared<RocksDBWholeSpaceIteratorImpl>(this, default_cf, 0, IteratorBounds());
 }
 
 int RocksDBStore::prepare_for_reshard(const std::string& new_sharding,

diff --git a/src/kv/RocksDBStore.h b/src/kv/RocksDBStore.h
index 529b4ffa621ab26564b521b2ddef6e90d11569bb..37833bf74a4d3da8fa82b04b2e8a85355e7ae2f4 100644 (file)
--- a/src/kv/RocksDBStore.h
+++ b/src/kv/RocksDBStore.h
@@ -65,6 +65,14 @@ namespace rocksdb{
 
 extern rocksdb::Logger *create_rocksdb_ceph_logger();
 
+inline rocksdb::Slice make_slice(const std::optional<std::string>& bound) {
+  if (bound) {
+    return {*bound};
+  } else {
+    return {};
+  }
+}
+
 /**
  * Uses RocksDB to implement the KeyValueDB interface
  */
@@ -84,6 +92,7 @@ class RocksDBStore : public KeyValueDB {
   uint64_t cache_size = 0;
   bool set_cache_flag = false;
   friend class ShardMergeIteratorImpl;
+  friend class CFIteratorImpl;
   friend class WholeMergeIteratorImpl;
   /*
    *  See RocksDB's definition of a column family(CF) and how to use it.
@@ -120,8 +129,11 @@ private:
   void add_column_family(const std::string& cf_name, uint32_t hash_l, uint32_t hash_h,
                         size_t shard_idx, rocksdb::ColumnFamilyHandle *handle);
   bool is_column_family(const std::string& prefix);
+  std::string_view get_key_hash_view(const prefix_shards& shards, const char* key, const size_t keylen);
+  rocksdb::ColumnFamilyHandle *get_key_cf(const prefix_shards& shards, const char* key, const size_t keylen);
   rocksdb::ColumnFamilyHandle *get_cf_handle(const std::string& prefix, const std::string& key);
   rocksdb::ColumnFamilyHandle *get_cf_handle(const std::string& prefix, const char* key, size_t keylen);
+  rocksdb::ColumnFamilyHandle *get_cf_handle(const std::string& prefix, const IteratorBounds& bounds);
 
   int submit_common(rocksdb::WriteOptions& woptions, KeyValueDB::Transaction t);
   int install_cf_mergeop(const std::string &cf_name, rocksdb::ColumnFamilyOptions *cf_opt);
@@ -342,9 +354,29 @@ 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(rocksdb::Iterator *iter) :
-      dbiter(iter) { }
+    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))
+      {
+        rocksdb::ReadOptions options = rocksdb::ReadOptions();
+        if (opts & ITERATOR_NOCACHE)
+          options.fill_cache=false;
+        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;
 
@@ -367,7 +399,7 @@ public:
     size_t value_size() override;
   };
 
-  Iterator get_iterator(const std::string& prefix, IteratorOpts opts = 0) override;
+  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);
@@ -500,7 +532,7 @@ err:
     return nullptr;
   }
 
-  WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0) override;
+  WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) override;
 private:
   WholeSpaceIterator get_default_cf_iterator();
 

diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
index 6ab4922da6bb18651cc3ec7c4b500b38596acefa..43a206e552512ba74034b722b67f7d4a295b2939 100644 (file)
--- a/src/os/bluestore/BlueStore.cc
+++ b/src/os/bluestore/BlueStore.cc
@@ -11001,10 +11001,13 @@ int BlueStore::_onode_omap_get(
   o->flush();
   {
     const string& prefix = o->get_omap_prefix();
-    KeyValueDB::Iterator it = db->get_iterator(prefix);
     string head, tail;
     o->get_omap_header(&head);
     o->get_omap_tail(&tail);
+    auto bounds = KeyValueDB::IteratorBounds();
+    bounds.lower_bound = head;
+    bounds.upper_bound = tail;
+    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, std::move(bounds));
     it->lower_bound(head);
     while (it->valid()) {
       if (it->key() == head) {
@@ -11086,10 +11089,13 @@ int BlueStore::omap_get_keys(
   o->flush();
   {
     const string& prefix = o->get_omap_prefix();
-    KeyValueDB::Iterator it = db->get_iterator(prefix);
     string head, tail;
     o->get_omap_key(string(), &head);
     o->get_omap_tail(&tail);
+    auto bounds = KeyValueDB::IteratorBounds();
+    bounds.lower_bound = head;
+    bounds.upper_bound = tail;
+    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, std::move(bounds));
     it->lower_bound(head);
     while (it->valid()) {
       if (it->key() >= tail) {
@@ -11274,7 +11280,15 @@ ObjectMap::ObjectMapIterator BlueStore::get_omap_iterator(
   }
   o->flush();
   dout(10) << __func__ << " has_omap = " << (int)o->onode.has_omap() <<dendl;
-  KeyValueDB::Iterator it = db->get_iterator(o->get_omap_prefix());
+  auto bounds = KeyValueDB::IteratorBounds();
+  if (o->onode.has_omap()) {
+    std::string lower_bound, upper_bound;
+    o->get_omap_key(string(), &lower_bound);
+    o->get_omap_tail(&upper_bound);
+    bounds.lower_bound = std::move(lower_bound);
+    bounds.upper_bound = std::move(upper_bound);
+  }
+  KeyValueDB::Iterator it = db->get_iterator(o->get_omap_prefix(), 0, std::move(bounds));
   return ObjectMap::ObjectMapIterator(new OmapIteratorImpl(c, o, it));
 }
 
@@ -15540,10 +15554,13 @@ int BlueStore::_clone(TransContext *txc,
       newo->onode.set_omap_flags(per_pool_omap == OMAP_BULK);
     }
     const string& prefix = newo->get_omap_prefix();
-    KeyValueDB::Iterator it = db->get_iterator(prefix);
     string head, tail;
     oldo->get_omap_header(&head);
     oldo->get_omap_tail(&tail);
+    auto bounds = KeyValueDB::IteratorBounds();
+    bounds.lower_bound = head;
+    bounds.upper_bound = tail;
+    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, std::move(bounds));
     it->lower_bound(head);
     while (it->valid()) {
       if (it->key() >= tail) {

diff --git a/src/test/ObjectMap/KeyValueDBMemory.cc b/src/test/ObjectMap/KeyValueDBMemory.cc
index 234e963397e31ea92c0748685d45f0995b00fecb..b2b351baef44b01850c021022f6301195143ff4c 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) {
+KeyValueDB::WholeSpaceIterator KeyValueDBMemory::get_wholespace_iterator(IteratorOpts opts, IteratorBounds bounds) {
   return std::shared_ptr<KeyValueDB::WholeSpaceIteratorImpl>(
     new WholeSpaceMemIterator(this)
   );

diff --git a/src/test/ObjectMap/KeyValueDBMemory.h b/src/test/ObjectMap/KeyValueDBMemory.h
index 5e7d3f0e1675f33f7c171ba1ce8d5df5672e3577..da386ccb908bfc3634520c40707f926b8b7a989b 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) override;
+  WholeSpaceIterator get_wholespace_iterator(IteratorOpts opts = 0, IteratorBounds bounds = IteratorBounds()) override;
 };