os: eliminate get_omap_keys method

gardran · gardran · commit 415fb348a2e0 · 2025-10-06T22:57:10.000+03:00
One can use omap_iterate instead.

The primary goal of this change is to cleanup ObjectStore API

Signed-off-by: Garry Drankovich &lt;garry.drankovich@clyso.com&gt;
diff --git a/src/os/FuseStore.cc b/src/os/FuseStore.cc
@@ -538,7 +538,14 @@ static int os_readdir(const char *path,
   case FN_OBJECT_OMAP:
     {
       set<string> keys;
-      fs->store->omap_get_keys(ch, oid, &keys);
+      fs->store->omap_iterate(
+	ch, oid,
+	ObjectStore::omap_iter_seek_t::min_lower_bound(),
+	[&keys]
+	(std::string_view key, std::string_view) mutable {
+	  keys.emplace(key);
+	  return ObjectStore::omap_iter_ret_t::NEXT;
+	});
       unsigned skip = offset;
       for (auto k : keys) {
 	if (skip) {
diff --git a/src/os/ObjectStore.h b/src/os/ObjectStore.h
@@ -721,13 +721,6 @@ class ObjectStore {
     bool allow_eio = false ///< [in] don't assert on eio
     ) = 0;
 
-  /// Get keys defined on oid
-  virtual int omap_get_keys(
-    CollectionHandle &c,   ///< [in] Collection containing oid
-    const ghobject_t &oid, ///< [in] Object containing omap
-    std::set<std::string> *keys      ///< [out] Keys defined on oid
-    ) = 0;
-
   /// Get key values
   virtual int omap_get_values(
     CollectionHandle &c,         ///< [in] Collection containing oid
diff --git a/src/os/bluestore/BlueStore.cc b/src/os/bluestore/BlueStore.cc
@@ -13731,59 +13731,6 @@ int BlueStore::omap_get_header(
   return r;
 }
 
-int BlueStore::omap_get_keys(
-  CollectionHandle &c_,              ///< [in] Collection containing oid
-  const ghobject_t &oid, ///< [in] Object containing omap
-  set<string> *keys      ///< [out] Keys defined on oid
-  )
-{
-  Collection *c = static_cast<Collection *>(c_.get());
-  dout(15) << __func__ << " " << c->get_cid() << " oid " << oid << dendl;
-  if (!c->exists)
-    return -ENOENT;
-  auto start1 = mono_clock::now();
-  std::shared_lock l(c->lock);
-  int r = 0;
-  OnodeRef o = c->get_onode(oid, false);
-  if (!o || !o->exists) {
-    r = -ENOENT;
-    goto out;
-  }
-  if (!o->onode.has_omap())
-    goto out;
-  o->flush();
-  {
-    const string& prefix = o->get_omap_prefix();
-    string head, tail;
-    o->get_omap_key(string(), &head);
-    o->get_omap_tail(&tail);
-    KeyValueDB::Iterator it = db->get_iterator(prefix, 0, KeyValueDB::IteratorBounds{head, tail});
-    it->lower_bound(head);
-    while (it->valid()) {
-      if (it->key() >= tail) {
-	dout(30) << __func__ << "  reached tail" << dendl;
-	break;
-      }
-      string user_key;
-      o->decode_omap_key(it->key(), &user_key);
-      dout(20) << __func__ << "  got " << pretty_binary_string(it->key())
-	       << " -> " << user_key << dendl;
-      keys->insert(user_key);
-      it->next();
-    }
-  }
- out:
-  c->store->log_latency(
-    __func__,
-    l_bluestore_omap_get_keys_lat,
-    mono_clock::now() - start1,
-    c->store->cct->_conf->bluestore_log_omap_iterator_age);
-
-  dout(10) << __func__ << " " << c->get_cid() << " oid " << oid << " = " << r
-	   << dendl;
-  return r;
-}
-
 int BlueStore::omap_get_values(
   CollectionHandle &c_,        ///< [in] Collection containing oid
   const ghobject_t &oid,       ///< [in] Object containing omap
diff --git a/src/os/bluestore/BlueStore.h b/src/os/bluestore/BlueStore.h
@@ -3367,13 +3367,6 @@ class BlueStore : public ObjectStore,
     bool allow_eio = false ///< [in] don't assert on eio
     ) override;
 
-  /// Get keys defined on oid
-  int omap_get_keys(
-    CollectionHandle &c,              ///< [in] Collection containing oid
-    const ghobject_t &oid, ///< [in] Object containing omap
-    std::set<std::string> *keys      ///< [out] Keys defined on oid
-    ) override;
-
   /// Get key values
   int omap_get_values(
     CollectionHandle &c,         ///< [in] Collection containing oid
diff --git a/src/os/kstore/KStore.cc b/src/os/kstore/KStore.cc
@@ -1645,49 +1645,6 @@ int KStore::omap_get_header(
   return r;
 }
 
-int KStore::omap_get_keys(
-  CollectionHandle& ch,              ///< [in] Collection containing oid
-  const ghobject_t &oid, ///< [in] Object containing omap
-  set<string> *keys      ///< [out] Keys defined on oid
-  )
-{
-  dout(15) << __func__ << " " << ch->cid << " oid " << oid << dendl;
-  Collection *c = static_cast<Collection*>(ch.get());
-  std::shared_lock l{c->lock};
-  int r = 0;
-  OnodeRef o = c->get_onode(oid, false);
-  if (!o || !o->exists) {
-    r = -ENOENT;
-    goto out;
-  }
-  if (!o->onode.omap_head)
-    goto out;
-  o->flush();
-  {
-    KeyValueDB::Iterator it = db->get_iterator(PREFIX_OMAP);
-    string head, tail;
-    get_omap_key(o->onode.omap_head, string(), &head);
-    get_omap_tail(o->onode.omap_head, &tail);
-    it->lower_bound(head);
-    while (it->valid()) {
-      if (it->key() >= tail) {
-	dout(30) << __func__ << "  reached tail" << dendl;
-	break;
-      }
-      string user_key;
-      decode_omap_key(it->key(), &user_key);
-      dout(30) << __func__ << "  got " << pretty_binary_string(it->key())
-	       << " -> " << user_key << dendl;
-      ceph_assert(it->key() < tail);
-      keys->insert(user_key);
-      it->next();
-    }
-  }
- out:
-  dout(10) << __func__ << " " << ch->cid << " oid " << oid << " = " << r << dendl;
-  return r;
-}
-
 int KStore::omap_get_values(
   CollectionHandle& ch,                    ///< [in] Collection containing oid
   const ghobject_t &oid,       ///< [in] Object containing omap
diff --git a/src/os/kstore/KStore.h b/src/os/kstore/KStore.h
@@ -502,14 +502,6 @@ class KStore : public ObjectStore {
     bool allow_eio = false ///< [in] don't assert on eio
     ) override;
 
-  using ObjectStore::omap_get_keys;
-  /// Get keys defined on oid
-  int omap_get_keys(
-    CollectionHandle& c,              ///< [in] Collection containing oid
-    const ghobject_t &oid, ///< [in] Object containing omap
-    std::set<std::string> *keys      ///< [out] Keys defined on oid
-    ) override;
-
   using ObjectStore::omap_get_values;
   /// Get key values
   int omap_get_values(
diff --git a/src/os/memstore/MemStore.cc b/src/os/memstore/MemStore.cc
@@ -497,23 +497,6 @@ int MemStore::omap_get_header(
   return 0;
 }
 
-int MemStore::omap_get_keys(
-  CollectionHandle& ch,              ///< [in] Collection containing oid
-  const ghobject_t &oid, ///< [in] Object containing omap
-  std::set<std::string> *keys      ///< [out] Keys defined on oid
-  )
-{
-  dout(10) << __func__ << " " << ch->cid << " " << oid << dendl;
-  Collection *c = static_cast<Collection*>(ch.get());
-  ObjectRef o = c->get_object(oid);
-  if (!o)
-    return -ENOENT;
-  std::lock_guard lock{o->omap_mutex};
-  for (auto p = o->omap.begin(); p != o->omap.end(); ++p)
-    keys->insert(p->first);
-  return 0;
-}
-
 int MemStore::omap_get_values(
   CollectionHandle& ch,                    ///< [in] Collection containing oid
   const ghobject_t &oid,       ///< [in] Object containing omap
diff --git a/src/os/memstore/MemStore.h b/src/os/memstore/MemStore.h
@@ -346,14 +346,6 @@ class MemStore : public ObjectStore {
     bool allow_eio = false ///< [in] don't assert on eio
     ) override;
 
-  using ObjectStore::omap_get_keys;
-  /// Get keys defined on oid
-  int omap_get_keys(
-    CollectionHandle& c,              ///< [in] Collection containing oid
-    const ghobject_t &oid, ///< [in] Object containing omap
-    std::set<std::string> *keys      ///< [out] Keys defined on oid
-    ) override;
-
   using ObjectStore::omap_get_values;
   /// Get key values
   int omap_get_values(
diff --git a/src/test/objectstore/ObjectStoreImitator.h b/src/test/objectstore/ObjectStoreImitator.h
@@ -319,12 +319,6 @@ class ObjectStoreImitator : public ObjectStore {
                       ) override {
     return 0;
   }
-  int omap_get_keys(CollectionHandle &c,   ///< [in] Collection containing oid
-                    const ghobject_t &oid, ///< [in] Object containing omap
-                    std::set<std::string> *keys ///< [out] Keys defined on oid
-                    ) override {
-    return 0;
-  }
   int omap_get_values(CollectionHandle &c,   ///< [in] Collection containing oid
                       const ghobject_t &oid, ///< [in] Object containing omap
                       const std::set<std::string> &keys, ///< [in] Keys to get
diff --git a/src/test/objectstore/store_test.cc b/src/test/objectstore/store_test.cc
@@ -6139,10 +6139,6 @@ TEST_P(StoreTest, OMapTest) {
     ASSERT_EQ(cur_attrs.size(), size_t(1));
     ASSERT_TRUE(bl_eq(bl1, bl3));
  
-    set<string> keys;
-    r = store->omap_get_keys(ch, hoid, &keys);
-    ASSERT_EQ(r, 0);
-    ASSERT_EQ(keys.size(), size_t(1));
   }
 
   // test omap_clear, omap_rmkey_range

Original file line number	Diff line number	Diff line change
`@@ -6139,10 +6139,6 @@ TEST_P(StoreTest, OMapTest) {`
`6139`	`6139`	`ASSERT_EQ(cur_attrs.size(), size_t(1));`
`6140`	`6140`	`ASSERT_TRUE(bl_eq(bl1, bl3));`
`6141`	`6141`
`6142`		`- set<string> keys;`
`6143`		`- r = store->omap_get_keys(ch, hoid, &keys);`
`6144`		`- ASSERT_EQ(r, 0);`
`6145`		`- ASSERT_EQ(keys.size(), size_t(1));`
`6146`	`6142`	`}`
`6147`	`6143`
`6148`	`6144`	`// test omap_clear, omap_rmkey_range`