From: Radoslaw Zarzynski Date: Fri, 29 Nov 2024 20:34:21 +0000 (+0000) Subject: osd, os: drop ObjectStore::get_omap_iterator in favor of omap_iterate X-Git-Tag: v20.3.0~156^2~6 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=5e4b5ae2dbb84e77836dbf42fe3288b4e44d80f3;p=ceph.git osd, os: drop ObjectStore::get_omap_iterator in favor of omap_iterate Please note this also drops a BlueStore's perf counter: `l_bluestore_omap_seek_to_first_lat`. Signed-off-by: Radoslaw Zarzynski --- diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h index 31cb641e0b1ec..25657cbf2d8b2 100644 --- a/src/os/ObjectStore.h +++ b/src/os/ObjectStore.h @@ -744,21 +744,6 @@ public: std::set *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 { diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc index 3d8a7c7dcfb98..ddb5616ffd318 100644 --- a/src/os/bluestore/BlueStore.cc +++ b/src/os/bluestore/BlueStore.cc @@ -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(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" <flush(); - dout(10) << __func__ << " has_omap = " << (int)o->onode.has_omap() <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 diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h index 9ca48cea44136..d21139e381900 100644 --- a/src/os/bluestore/BlueStore.h +++ b/src/os/bluestore/BlueStore.h @@ -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 *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 diff --git a/src/os/kstore/KStore.cc b/src/os/kstore/KStore.cc index 0007644f2097d..12cf6f6aef5f3 100644 --- a/src/os/kstore/KStore.cc +++ b/src/os/kstore/KStore.cc @@ -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(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" <flush(); - dout(10) << __func__ << " header = " << o->onode.omap_head <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 diff --git a/src/os/kstore/KStore.h b/src/os/kstore/KStore.h index 3aebd328df2b4..81b9a0c45a2b8 100644 --- a/src/os/kstore/KStore.h +++ b/src/os/kstore/KStore.h @@ -166,26 +166,6 @@ public: }; using CollectionRef = ceph::ref_t; - 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 *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 diff --git a/src/os/memstore/MemStore.cc b/src/os/memstore/MemStore.cc index 0e7e19665df9a..f1278a0f025b0 100644 --- a/src/os/memstore/MemStore.cc +++ b/src/os/memstore/MemStore.cc @@ -556,67 +556,6 @@ int MemStore::omap_check_keys( return 0; } -class MemStore::OmapIteratorImpl : public ObjectMap::ObjectMapIteratorImpl { - CollectionRef c; - ObjectRef o; - std::map::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(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 diff --git a/src/os/memstore/MemStore.h b/src/os/memstore/MemStore.h index 358068fef477d..9f9e401c25641 100644 --- a/src/os/memstore/MemStore.h +++ b/src/os/memstore/MemStore.h @@ -185,9 +185,6 @@ public: typedef Collection::Ref CollectionRef; private: - class OmapIteratorImpl; - - std::unordered_map coll_map; /// rwlock to protect coll_map ceph::shared_mutex coll_lock{ @@ -375,12 +372,6 @@ public: std::set *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 diff --git a/src/test/objectstore/ObjectStoreImitator.h b/src/test/objectstore/ObjectStoreImitator.h index 875f9041b8327..e1fcf7ec93355 100644 --- a/src/test/objectstore/ObjectStoreImitator.h +++ b/src/test/objectstore/ObjectStoreImitator.h @@ -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