]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
osd, os: drop ObjectStore::get_omap_iterator in favor of omap_iterate
authorRadoslaw Zarzynski <rzarzyns@redhat.com>
Fri, 29 Nov 2024 20:34:21 +0000 (20:34 +0000)
committerRadoslaw Zarzynski <rzarzyns@redhat.com>
Fri, 4 Apr 2025 18:21:54 +0000 (18:21 +0000)
Please note this also drops a BlueStore's perf counter:
`l_bluestore_omap_seek_to_first_lat`.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>
src/os/ObjectStore.h
src/os/bluestore/BlueStore.cc
src/os/bluestore/BlueStore.h
src/os/kstore/KStore.cc
src/os/kstore/KStore.h
src/os/memstore/MemStore.cc
src/os/memstore/MemStore.h
src/test/objectstore/ObjectStoreImitator.h

index 31cb641e0b1ecca7cc73109392a5cd6ca5cdb2c8..25657cbf2d8b297fd6793ac7fb29d5cf73078f2f 100644 (file)
@@ -744,21 +744,6 @@ public:
     std::set<std::string> *out         ///< [out] Subset of keys defined on oid
     ) = 0;
 
-  /**
-   * Returns an object map iterator
-   *
-   * Warning!  The returned iterator is an implicit lock on filestore
-   * operations in c.  Do not use filestore methods on c while the returned
-   * iterator is live.  (Filling in a transaction is no problem).
-   *
-   * @return iterator, null on error
-   */
-  [[deprecated("in favor of omap_iterate()")]]
-  virtual ObjectMap::ObjectMapIterator get_omap_iterator(
-    CollectionHandle &c,   ///< [in] collection
-    const ghobject_t &oid  ///< [in] object
-    ) = 0;
-
   struct omap_iter_seek_t {
     std::string seek_position;
     enum {
index 3d8a7c7dcfb987992f2ad7bdba72bfd695ad51fe..ddb5616ffd318260dfc93427f0ae443da8cbb87e 100644 (file)
@@ -5561,171 +5561,6 @@ void BlueStore::MempoolThread::_update_cache_settings()
                 << dendl;
 }
 
-// =======================================================
-
-// OmapIteratorImpl
-
-#undef dout_prefix
-#define dout_prefix *_dout << "bluestore.OmapIteratorImpl(" << this << ") "
-
-BlueStore::OmapIteratorImpl::OmapIteratorImpl(
-  PerfCounters* _logger, CollectionRef c, OnodeRef& o, KeyValueDB::Iterator it)
-  : logger(_logger), c(c), o(o), it(it)
-{
-  logger->inc(l_bluestore_omap_iterator_count);
-  std::shared_lock l(c->lock);
-  if (o->onode.has_omap()) {
-    o->get_omap_key(string(), &head);
-    o->get_omap_tail(&tail);
-    auto start1 = mono_clock::now();
-    it->lower_bound(head);
-    c->store->log_latency(
-    __func__,
-    l_bluestore_omap_seek_to_first_lat,
-    mono_clock::now() - start1,
-    c->store->cct->_conf->bluestore_log_omap_iterator_age);
-  }
-}
-BlueStore::OmapIteratorImpl::~OmapIteratorImpl()
-{
-  logger->dec(l_bluestore_omap_iterator_count);
-}
-
-string BlueStore::OmapIteratorImpl::_stringify() const
-{
-  stringstream s;
-  s << " omap_iterator(cid = " << c->cid
-    <<", oid = " << o->oid << ")";
-  return s.str();
-}
-
-int BlueStore::OmapIteratorImpl::seek_to_first()
-{
-  std::shared_lock l(c->lock);
-  auto start1 = mono_clock::now();
-  if (o->onode.has_omap()) {
-    it->lower_bound(head);
-  } else {
-    it = KeyValueDB::Iterator();
-  }
-  c->store->log_latency(
-    __func__,
-    l_bluestore_omap_seek_to_first_lat,
-    mono_clock::now() - start1,
-    c->store->cct->_conf->bluestore_log_omap_iterator_age);
-
-  return 0;
-}
-
-int BlueStore::OmapIteratorImpl::upper_bound(const string& after)
-{
-  std::shared_lock l(c->lock);
-  auto start1 = mono_clock::now();
-  if (o->onode.has_omap()) {
-    string key;
-    o->get_omap_key(after, &key);
-    ldout(c->store->cct,20) << __func__ << " after " << after << " key "
-                           << pretty_binary_string(key) << dendl;
-    it->upper_bound(key);
-  } else {
-    it = KeyValueDB::Iterator();
-  }
-  c->store->log_latency_fn(
-    __func__,
-    l_bluestore_omap_upper_bound_lat,
-    mono_clock::now() - start1,
-    c->store->cct->_conf->bluestore_log_omap_iterator_age,
-    [&] (const ceph::timespan& lat) {
-      return ", after = " + after +
-       _stringify();
-    }
-  );
-  return 0;
-}
-
-int BlueStore::OmapIteratorImpl::lower_bound(const string& to)
-{
-  std::shared_lock l(c->lock);
-  auto start1 = mono_clock::now();
-  if (o->onode.has_omap()) {
-    string key;
-    o->get_omap_key(to, &key);
-    ldout(c->store->cct,20) << __func__ << " to " << to << " key "
-                           << pretty_binary_string(key) << dendl;
-    it->lower_bound(key);
-  } else {
-    it = KeyValueDB::Iterator();
-  }
-  c->store->log_latency_fn(
-    __func__,
-    l_bluestore_omap_lower_bound_lat,
-    mono_clock::now() - start1,
-    c->store->cct->_conf->bluestore_log_omap_iterator_age,
-    [&] (const ceph::timespan& lat) {
-      return ", to = " + to +
-       _stringify();
-    }
-  );
-  return 0;
-}
-
-bool BlueStore::OmapIteratorImpl::valid()
-{
-  std::shared_lock l(c->lock);
-  bool r = o->onode.has_omap() && it && it->valid() &&
-    it->raw_key().second < tail;
-  if (it && it->valid()) {
-    ldout(c->store->cct,20) << __func__ << " is at "
-                           << pretty_binary_string(it->raw_key().second)
-                           << dendl;
-  }
-  return r;
-}
-
-int BlueStore::OmapIteratorImpl::next()
-{
-  int r = -1;
-  std::shared_lock l(c->lock);
-  auto start1 = mono_clock::now();
-  if (o->onode.has_omap()) {
-    it->next();
-    r = 0;
-  }
-  c->store->log_latency(
-    __func__,
-    l_bluestore_omap_next_lat,
-    mono_clock::now() - start1,
-    c->store->cct->_conf->bluestore_log_omap_iterator_age);
-
-  return r;
-}
-
-string BlueStore::OmapIteratorImpl::key()
-{
-  std::shared_lock l(c->lock);
-  ceph_assert(it->valid());
-  string db_key = it->raw_key().second;
-  string user_key;
-  o->decode_omap_key(db_key, &user_key);
-
-  return user_key;
-}
-
-bufferlist BlueStore::OmapIteratorImpl::value()
-{
-  std::shared_lock l(c->lock);
-  ceph_assert(it->valid());
-  return it->value();
-}
-
-std::string_view BlueStore::OmapIteratorImpl::value_as_sv()
-{
-  std::shared_lock l(c->lock);
-  ceph_assert(it->valid());
-  return it->value_as_sv();
-}
-
-
 // =====================================
 
 #undef dout_prefix
@@ -6515,9 +6350,6 @@ void BlueStore::_init_logger()
   //****************************************
   // other client ops latencies
   //****************************************
-  b.add_time_avg(l_bluestore_omap_seek_to_first_lat, "omap_seek_to_first_lat",
-    "Average omap iterator seek_to_first call latency",
-    "osfl", PerfCountersBuilder::PRIO_USEFUL);
   b.add_time_avg(l_bluestore_omap_upper_bound_lat, "omap_upper_bound_lat",
     "Average omap iterator upper_bound call latency",
     "oubl", PerfCountersBuilder::PRIO_USEFUL);
@@ -13892,36 +13724,6 @@ int BlueStore::omap_check_keys(
   return r;
 }
 
-ObjectMap::ObjectMapIterator BlueStore::get_omap_iterator(
-  CollectionHandle &c_,              ///< [in] collection
-  const ghobject_t &oid  ///< [in] object
-  )
-{
-  Collection *c = static_cast<Collection *>(c_.get());
-  dout(10) << __func__ << " " << c->get_cid() << " " << oid << dendl;
-  if (!c->exists) {
-    return ObjectMap::ObjectMapIterator();
-  }
-  std::shared_lock l(c->lock);
-  OnodeRef o = c->get_onode(oid, false);
-  if (!o || !o->exists) {
-    dout(10) << __func__ << " " << oid << "doesn't exist" <<dendl;
-    return ObjectMap::ObjectMapIterator();
-  }
-  o->flush();
-  dout(10) << __func__ << " has_omap = " << (int)o->onode.has_omap() <<dendl;
-  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(logger,c, o, it));
-}
-
 int BlueStore::omap_iterate(
   CollectionHandle &c_,   ///< [in] collection
   const ghobject_t &oid, ///< [in] object
index 9ca48cea44136e96fcc0ce5dfa9d11a20d3b4398..d21139e381900cd07ae78d8c77bb89c76fef19c0 100644 (file)
@@ -208,7 +208,6 @@ enum {
 
   // other client ops latencies
   //****************************************
-  l_bluestore_omap_seek_to_first_lat,
   l_bluestore_omap_upper_bound_lat,
   l_bluestore_omap_lower_bound_lat,
   l_bluestore_omap_next_lat,
@@ -1756,35 +1755,6 @@ public:
     Collection(BlueStore *ns, OnodeCacheShard *oc, BufferCacheShard *bc, coll_t c);
   };
 
-  class OmapIteratorImpl : public ObjectMap::ObjectMapIteratorImpl {
-
-    PerfCounters* logger = nullptr;
-    CollectionRef c;
-    OnodeRef o;
-    KeyValueDB::Iterator it;
-    std::string head, tail;
-
-    std::string _stringify() const;
-  public:
-    OmapIteratorImpl(PerfCounters* l, CollectionRef c, OnodeRef& o, KeyValueDB::Iterator it);
-    virtual ~OmapIteratorImpl();
-    int seek_to_first() override;
-    int upper_bound(const std::string &after) override;
-    int lower_bound(const std::string &to) override;
-    bool valid() override;
-    int next() override;
-    std::string key() override;
-    ceph::buffer::list value() override;
-    std::string_view value_as_sv() override;
-    std::string tail_key() override {
-      return tail;
-    }
-
-    int status() override {
-      return 0;
-    }
-  };
-
   struct volatile_statfs{
     enum {
       STATFS_ALLOCATED = 0,
@@ -3454,11 +3424,6 @@ public:
     std::set<std::string> *out         ///< [out] Subset of keys defined on oid
     ) override;
 
-  ObjectMap::ObjectMapIterator get_omap_iterator(
-    CollectionHandle &c,   ///< [in] collection
-    const ghobject_t &oid  ///< [in] object
-    ) override;
-
   int omap_iterate(
     CollectionHandle &c,   ///< [in] collection
     const ghobject_t &oid, ///< [in] object
index 0007644f2097d057dd9107caec8d957274ecabe5..12cf6f6aef5f33d4100916396239a9eb7436712a 100644 (file)
@@ -1564,100 +1564,6 @@ out:
 
 // omap reads
 
-KStore::OmapIteratorImpl::OmapIteratorImpl(
-  CollectionRef c, OnodeRef o, KeyValueDB::Iterator it)
-  : c(c), o(o), it(it)
-{
-  std::shared_lock l{c->lock};
-  if (o->onode.omap_head) {
-    get_omap_key(o->onode.omap_head, string(), &head);
-    get_omap_tail(o->onode.omap_head, &tail);
-    it->lower_bound(head);
-  }
-}
-
-int KStore::OmapIteratorImpl::seek_to_first()
-{
-  std::shared_lock l{c->lock};
-  if (o->onode.omap_head) {
-    it->lower_bound(head);
-  } else {
-    it = KeyValueDB::Iterator();
-  }
-  return 0;
-}
-
-int KStore::OmapIteratorImpl::upper_bound(const string& after)
-{
-  std::shared_lock l{c->lock};
-  if (o->onode.omap_head) {
-    string key;
-    get_omap_key(o->onode.omap_head, after, &key);
-    it->upper_bound(key);
-  } else {
-    it = KeyValueDB::Iterator();
-  }
-  return 0;
-}
-
-int KStore::OmapIteratorImpl::lower_bound(const string& to)
-{
-  std::shared_lock l{c->lock};
-  if (o->onode.omap_head) {
-    string key;
-    get_omap_key(o->onode.omap_head, to, &key);
-    it->lower_bound(key);
-  } else {
-    it = KeyValueDB::Iterator();
-  }
-  return 0;
-}
-
-bool KStore::OmapIteratorImpl::valid()
-{
-  std::shared_lock l{c->lock};
-  if (o->onode.omap_head && it->valid() && it->raw_key().second <= tail) {
-    return true;
-  } else {
-    return false;
-  }
-}
-
-int KStore::OmapIteratorImpl::next()
-{
-  std::shared_lock l{c->lock};
-  if (o->onode.omap_head) {
-    it->next();
-    return 0;
-  } else {
-    return -1;
-  }
-}
-
-string KStore::OmapIteratorImpl::key()
-{
-  std::shared_lock l{c->lock};
-  ceph_assert(it->valid());
-  string db_key = it->raw_key().second;
-  string user_key;
-  decode_omap_key(db_key, &user_key);
-  return user_key;
-}
-
-bufferlist KStore::OmapIteratorImpl::value()
-{
-  std::shared_lock l{c->lock};
-  ceph_assert(it->valid());
-  return it->value();
-}
-
-std::string_view KStore::OmapIteratorImpl::value_as_sv()
-{
-  std::shared_lock l{c->lock};
-  ceph_assert(it->valid());
-  return it->value_as_sv();
-}
-
 int KStore::omap_get(
   CollectionHandle& ch,                ///< [in] Collection containing oid
   const ghobject_t &oid,   ///< [in] Object containing omap
@@ -1853,26 +1759,6 @@ int KStore::omap_check_keys(
   return r;
 }
 
-ObjectMap::ObjectMapIterator KStore::get_omap_iterator(
-  CollectionHandle& ch,              ///< [in] collection
-  const ghobject_t &oid  ///< [in] object
-  )
-{
-
-  dout(10) << __func__ << " " << ch->cid << " " << oid << dendl;
-  Collection *c = static_cast<Collection*>(ch.get());
-  std::shared_lock l{c->lock};
-  OnodeRef o = c->get_onode(oid, false);
-  if (!o || !o->exists) {
-    dout(10) << __func__ << " " << oid << "doesn't exist" <<dendl;
-    return ObjectMap::ObjectMapIterator();
-  }
-  o->flush();
-  dout(10) << __func__ << " header = " << o->onode.omap_head <<dendl;
-  KeyValueDB::Iterator it = db->get_iterator(PREFIX_OMAP);
-  return ObjectMap::ObjectMapIterator(new OmapIteratorImpl(c, o, it));
-}
-
 int KStore::omap_iterate(
   CollectionHandle &ch,   ///< [in] collection
   const ghobject_t &oid, ///< [in] object
index 3aebd328df2b4cb0db6634238ecca92bc2d84c88..81b9a0c45a2b80806ebca3275cacce34b83a679a 100644 (file)
@@ -166,26 +166,6 @@ public:
   };
   using CollectionRef = ceph::ref_t<Collection>;
 
-  class OmapIteratorImpl : public ObjectMap::ObjectMapIteratorImpl {
-    CollectionRef c;
-    OnodeRef o;
-    KeyValueDB::Iterator it;
-    std::string head, tail;
-  public:
-    OmapIteratorImpl(CollectionRef c, OnodeRef o, KeyValueDB::Iterator it);
-    int seek_to_first() override;
-    int upper_bound(const std::string &after) override;
-    int lower_bound(const std::string &to) override;
-    bool valid() override;
-    int next() override;
-    std::string key() override;
-    ceph::buffer::list value() override;
-    std::string_view value_as_sv() override;
-    int status() override {
-      return 0;
-    }
-  };
-
   struct TransContext {
     typedef enum {
       STATE_PREPARE,
@@ -548,12 +528,6 @@ public:
     std::set<std::string> *out         ///< [out] Subset of keys defined on oid
     ) override;
 
-  using ObjectStore::get_omap_iterator;
-  ObjectMap::ObjectMapIterator get_omap_iterator(
-    CollectionHandle& c,              ///< [in] collection
-    const ghobject_t &oid  ///< [in] object
-    ) override;
-
   int omap_iterate(
     CollectionHandle &c,   ///< [in] collection
     const ghobject_t &oid, ///< [in] object
index 0e7e19665df9ab6b8f59a091dd57f010a34d241a..f1278a0f025b0058eee1cd57b1b94e3a8cb34802 100644 (file)
@@ -556,67 +556,6 @@ int MemStore::omap_check_keys(
   return 0;
 }
 
-class MemStore::OmapIteratorImpl : public ObjectMap::ObjectMapIteratorImpl {
-  CollectionRef c;
-  ObjectRef o;
-  std::map<std::string,ceph::buffer::list>::iterator it;
-public:
-  OmapIteratorImpl(CollectionRef c, ObjectRef o)
-    : c(c), o(o), it(o->omap.begin()) {}
-
-  int seek_to_first() override {
-    std::lock_guard lock{o->omap_mutex};
-    it = o->omap.begin();
-    return 0;
-  }
-  int upper_bound(const std::string &after) override {
-    std::lock_guard lock{o->omap_mutex};
-    it = o->omap.upper_bound(after);
-    return 0;
-  }
-  int lower_bound(const std::string &to) override {
-    std::lock_guard lock{o->omap_mutex};
-    it = o->omap.lower_bound(to);
-    return 0;
-  }
-  bool valid() override {
-    std::lock_guard lock{o->omap_mutex};
-    return it != o->omap.end();
-  }
-  int next() override {
-    std::lock_guard lock{o->omap_mutex};
-    ++it;
-    return 0;
-  }
-  std::string key() override {
-    std::lock_guard lock{o->omap_mutex};
-    return it->first;
-  }
-  ceph::buffer::list value() override {
-    std::lock_guard lock{o->omap_mutex};
-    return it->second;
-  }
-  std::string_view value_as_sv() override {
-    std::lock_guard lock{o->omap_mutex};
-    return std::string_view{it->second.c_str(), it->second.length()};
-  }
-  int status() override {
-    return 0;
-  }
-};
-
-ObjectMap::ObjectMapIterator MemStore::get_omap_iterator(
-  CollectionHandle& ch,
-  const ghobject_t& oid)
-{
-  dout(10) << __func__ << " " << ch->cid << " " << oid << dendl;
-  Collection *c = static_cast<Collection*>(ch.get());
-  ObjectRef o = c->get_object(oid);
-  if (!o)
-    return ObjectMap::ObjectMapIterator();
-  return ObjectMap::ObjectMapIterator(new OmapIteratorImpl(c, o));
-}
-
 int MemStore::omap_iterate(
   CollectionHandle &ch,   ///< [in] collection
   const ghobject_t &oid, ///< [in] object
index 358068fef477d1fbc9948d6e546a28f60aa4b73c..9f9e401c25641b96f2af7b4e5d88e784a69a0836 100644 (file)
@@ -185,9 +185,6 @@ public:
   typedef Collection::Ref CollectionRef;
 
 private:
-  class OmapIteratorImpl;
-
-
   std::unordered_map<coll_t, CollectionRef> coll_map;
   /// rwlock to protect coll_map
   ceph::shared_mutex coll_lock{
@@ -375,12 +372,6 @@ public:
     std::set<std::string> *out         ///< [out] Subset of keys defined on oid
     ) override;
 
-  using ObjectStore::get_omap_iterator;
-  ObjectMap::ObjectMapIterator get_omap_iterator(
-    CollectionHandle& c,              ///< [in] collection
-    const ghobject_t &oid  ///< [in] object
-    ) override;
-
   int omap_iterate(
     CollectionHandle &c,   ///< [in] collection
     const ghobject_t &oid, ///< [in] object
index 875f9041b83275a71e49a461543e84815f3c6a08..e1fcf7ec9335576254764590068e8af16962982a 100644 (file)
@@ -341,12 +341,6 @@ public:
       ) override {
     return 0;
   }
-  ObjectMap::ObjectMapIterator
-  get_omap_iterator(CollectionHandle &c,  ///< [in] collection
-                    const ghobject_t &oid ///< [in] object
-                    ) override {
-    return {};
-  }
 
   int omap_iterate(CollectionHandle &c,   ///< [in] collection
                    const ghobject_t &oid, ///< [in] object