crimson/os: drop the OmapIterator concept

`get_omap_values()` is powerful enough to be used instead.

Signed-off-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

src/crimson/os/alienstore/alien_store.cc

@@ -606,94 +606,6 @@ AlienStore::read_errorator::future<std::map<uint64_t, uint64_t>> AlienStore::fie
   });
 }
 
-seastar::future<FuturizedStore::OmapIteratorRef> AlienStore::get_omap_iterator(
-  CollectionRef ch,
-  const ghobject_t& oid)
-{
-  assert(tp);
-  return tp->submit(ch->get_cid().hash_to_shard(tp->size()),
-    [this, ch, oid] {
-    auto c = static_cast<AlienCollection*>(ch.get());
-    auto iter = store->get_omap_iterator(c->collection, oid);
-    return FuturizedStore::OmapIteratorRef(
-	      new AlienStore::AlienOmapIterator(iter,
-						this,
-						ch));
-  });
-}
-
-//TODO: each iterator op needs one submit, this is not efficient,
-//      needs further optimization.
-seastar::future<> AlienStore::AlienOmapIterator::seek_to_first()
-{
-  assert(store->tp);
-  return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()),
-    [this] {
-    return iter->seek_to_first();
-  }).then([] (int r) {
-    assert(r == 0);
-    return seastar::now();
-  });
-}
-
-seastar::future<> AlienStore::AlienOmapIterator::upper_bound(
-  const std::string& after)
-{
-  assert(store->tp);
-  return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()),
-    [this, after] {
-    return iter->upper_bound(after);
-  }).then([] (int r) {
-    assert(r == 0);
-    return seastar::now();
-  });
-}
-
-seastar::future<> AlienStore::AlienOmapIterator::lower_bound(
-  const std::string& to)
-{
-  assert(store->tp);
-  return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()),
-    [this, to] {
-    return iter->lower_bound(to);
-  }).then([] (int r) {
-    assert(r == 0);
-    return seastar::now();
-  });
-}
-
-seastar::future<> AlienStore::AlienOmapIterator::next()
-{
-  assert(store->tp);
-  return store->tp->submit(ch->get_cid().hash_to_shard(store->tp->size()),
-    [this] {
-    return iter->next();
-  }).then([] (int r) {
-    assert(r == 0);
-    return seastar::now();
-  });
-}
-
-bool AlienStore::AlienOmapIterator::valid() const
-{
-  return iter->valid();
-}
-
-std::string AlienStore::AlienOmapIterator::key()
-{
-  return iter->key();
-}
-
-ceph::buffer::list AlienStore::AlienOmapIterator::value()
-{
-  return iter->value();
-}
-
-int AlienStore::AlienOmapIterator::status() const
-{
-  return iter->status();
-}
-
 std::vector<uint64_t> AlienStore::_parse_cpu_cores()
 {
   std::vector<uint64_t> cpu_cores;

src/crimson/os/alienstore/alien_store.h

@@ -21,24 +21,6 @@ class Transaction;
 namespace crimson::os {
 class AlienStore final : public FuturizedStore {
 public:
-  class AlienOmapIterator final : public OmapIterator {
-  public:
-    AlienOmapIterator(ObjectMap::ObjectMapIterator& it,
-        AlienStore* store, const CollectionRef& ch)
-      : iter(it), store(store), ch(ch) {}
-    seastar::future<> seek_to_first();
-    seastar::future<> upper_bound(const std::string& after);
-    seastar::future<> lower_bound(const std::string& to);
-    bool valid() const;
-    seastar::future<> next();
-    std::string key();
-    ceph::buffer::list value();
-    int status() const;
-  private:
-    ObjectMap::ObjectMapIterator iter;
-    AlienStore* store;
-    CollectionRef ch;
-  };
   AlienStore(const std::string& type,
              const std::string& path,
              const ConfigValues& values);
@@ -115,9 +97,6 @@ public:
     const ghobject_t&,
     uint64_t off,
     uint64_t len) final;
-  seastar::future<FuturizedStore::OmapIteratorRef> get_omap_iterator(
-    CollectionRef ch,
-    const ghobject_t& oid) final;
 
 private:
   template <class... Args>

src/crimson/os/cyanstore/cyan_store.cc

@@ -844,19 +844,6 @@ unsigned CyanStore::get_max_attr_name_length() const
   return 256;
 }
 
-seastar::future<FuturizedStore::OmapIteratorRef> CyanStore::get_omap_iterator(
-    CollectionRef ch,
-    const ghobject_t& oid)
-{
-  auto c = static_cast<Collection*>(ch.get());
-  auto o = c->get_object(oid);
-  if (!o) {
-    throw std::runtime_error(fmt::format("object does not exist: {}", oid));
-  }
-  return seastar::make_ready_future<FuturizedStore::OmapIteratorRef>(
-	    new CyanStore::CyanOmapIterator(o));
-}
-
 CyanStore::read_errorator::future<std::map<uint64_t, uint64_t>>
 CyanStore::fiemap(
     CollectionRef ch,
@@ -889,33 +876,4 @@ CyanStore::stat(
   return seastar::make_ready_future<struct stat>(std::move(st));
 }
 
-seastar::future<> CyanStore::CyanOmapIterator::seek_to_first()
-{
-  iter = obj->omap.begin();
-  return seastar::make_ready_future<>();
-}
-
-seastar::future<> CyanStore::CyanOmapIterator::upper_bound(const std::string& after)
-{
-  iter = obj->omap.upper_bound(after);
-  return seastar::make_ready_future<>();
-}
-
-seastar::future<> CyanStore::CyanOmapIterator::lower_bound(const std::string &to)
-{
-  iter = obj->omap.lower_bound(to);
-  return seastar::make_ready_future<>();
-}
-
-bool CyanStore::CyanOmapIterator::valid() const
-{
-  return iter != obj->omap.end();
-}
-
-seastar::future<> CyanStore::CyanOmapIterator::next()
-{
-  ++iter;
-  return seastar::make_ready_future<>();
-}
-
 }

src/crimson/os/cyanstore/cyan_store.h

@@ -34,32 +34,6 @@ class CyanStore final : public FuturizedStore {
   uuid_d osd_fsid;
 
 public:
-  class CyanOmapIterator final : public OmapIterator {
-  public:
-    CyanOmapIterator() {}
-    CyanOmapIterator(ObjectRef obj) : obj(obj) {
-      iter = obj->omap.begin();
-    }
-    seastar::future<> seek_to_first() final;
-    seastar::future<> upper_bound(const std::string &after) final;
-    seastar::future<> lower_bound(const std::string &to) final;
-    bool valid() const final;
-    seastar::future<> next() final;
-    std::string key() final {
-      return iter->first;
-    }
-    virtual ceph::buffer::list value() {
-      return iter->second;
-    }
-    virtual int status() const {
-      return iter != obj->omap.end() ? 0 : -1;
-    }
-    virtual ~CyanOmapIterator() {}
-  private:
-    std::map<std::string, bufferlist>::const_iterator iter;
-    ObjectRef obj;
-  };
-
   CyanStore(const std::string& path);
   ~CyanStore() final;
 
@@ -132,10 +106,6 @@ public:
   uuid_d get_fsid() const final;
   unsigned get_max_attr_name_length() const final;
 
-  seastar::future<OmapIteratorRef> get_omap_iterator(
-    CollectionRef c,
-    const ghobject_t& oid);
-
   read_errorator::future<std::map<uint64_t, uint64_t>> fiemap(CollectionRef c,
 						       const ghobject_t& oid,
 						       uint64_t off,

src/crimson/os/futurized_store.h

@@ -27,32 +27,6 @@ class FuturizedCollection;
 class FuturizedStore {
 
 public:
-  class OmapIterator {
-  public:
-    virtual seastar::future<> seek_to_first() = 0;
-    virtual seastar::future<> upper_bound(const std::string &after) = 0;
-    virtual seastar::future<> lower_bound(const std::string &to) = 0;
-    virtual bool valid() const {
-      return false;
-    }
-    virtual seastar::future<> next() = 0;
-    virtual std::string key() {
-      return {};
-    }
-    virtual ceph::buffer::list value() {
-      return {};
-    }
-    virtual int status() const {
-      return 0;
-    }
-    virtual ~OmapIterator() {}
-  private:
-    unsigned count = 0;
-    friend void intrusive_ptr_add_ref(FuturizedStore::OmapIterator* iter);
-    friend void intrusive_ptr_release(FuturizedStore::OmapIterator* iter);
-  };
-  using OmapIteratorRef = boost::intrusive_ptr<OmapIterator>;
-
   static seastar::future<std::unique_ptr<FuturizedStore>> create(const std::string& type,
                                                 const std::string& data,
                                                 const ConfigValues& values);
@@ -175,9 +149,6 @@ public:
     return seastar::now();
   }
 
-  virtual seastar::future<OmapIteratorRef> get_omap_iterator(
-    CollectionRef ch,
-    const ghobject_t& oid) = 0;
   virtual read_errorator::future<std::map<uint64_t, uint64_t>> fiemap(
     CollectionRef ch,
     const ghobject_t& oid,
@@ -192,19 +163,6 @@ public:
   virtual unsigned get_max_attr_name_length() const = 0;
 };
 
-inline void intrusive_ptr_add_ref(FuturizedStore::OmapIterator* iter) {
-  assert(iter);
-  iter->count++;
-}
-
-inline void intrusive_ptr_release(FuturizedStore::OmapIterator* iter) {
-  assert(iter);
-  assert(iter->count > 0);
-  if ((--iter->count) == 0) {
-    delete iter;
-  }
-}
-
 /**
  * ShardedStoreProxy
  *
@@ -231,60 +189,6 @@ class ShardedStoreProxy : public FuturizedStore {
       core, *impl, method, std::forward<Args>(args)...);
   }
 
-  /**
-   * _OmapIterator
-   *
-   * Proxies OmapIterator operations to store's core.  Assumes that
-   * syncronous methods are safe to call directly from calling core
-   * since remote store should only be touching that memory during
-   * a method invocation.
-   *
-   * TODO: We don't really need OmapIterator at all, replace it with
-   * an appropriately paged omap_get_values variant.
-   */
-  class _OmapIterator : public OmapIterator {
-    using fref_t = seastar::foreign_ptr<OmapIteratorRef>;
-    const core_id_t core;
-    fref_t impl;
-
-    template <typename Method, typename... Args>
-    decltype(auto) proxy(Method method, Args&&... args) {
-      return proxy_method_on_core(
-	core, *impl, method, std::forward<Args>(args)...);
-    }
-
-  public:
-    _OmapIterator(core_id_t core, fref_t &&impl)
-      : core(core), impl(std::move(impl)) {}
-
-    seastar::future<> seek_to_first() final {
-      return proxy(&OmapIterator::seek_to_first);
-    }
-    seastar::future<> upper_bound(const std::string &after) final {
-      return proxy(&OmapIterator::upper_bound, after);
-    }
-    seastar::future<> lower_bound(const std::string &to) final {
-      return proxy(&OmapIterator::lower_bound, to);
-    }
-    bool valid() const final {
-      return impl->valid();
-    }
-    seastar::future<> next() final {
-      return proxy(&OmapIterator::next);
-    }
-    std::string key() final {
-      return impl->key();
-    }
-    ceph::buffer::list value() final {
-      return impl->value();
-    }
-    int status() const final {
-      return impl->status();
-    }
-    ~_OmapIterator() = default;
-  };
-
-
 public:
   ShardedStoreProxy(T *t)
     : core(seastar::this_shard_id()),
@@ -412,21 +316,6 @@ public:
     return proxy(&T::inject_mdata_error, o);
   }
 
-  seastar::future<OmapIteratorRef> get_omap_iterator(
-    CollectionRef ch,
-    const ghobject_t &oid) final {
-    return crimson::submit_to(
-      core,
-      [this, ch=std::move(ch), oid]() mutable {
-	return impl->get_omap_iterator(
-	  std::move(ch), oid
-	).then([](auto iref) {
-	  return seastar::foreign_ptr(iref);
-	});
-      }).then([this](auto iref) {
-	return OmapIteratorRef(new _OmapIterator(core, std::move(iref)));
-      });
-  }
   read_errorator::future<std::map<uint64_t, uint64_t>> fiemap(
     CollectionRef ch,
     const ghobject_t &oid,

src/crimson/os/seastore/seastore.cc

@@ -940,110 +940,6 @@ SeaStore::omap_get_values_ret_t SeaStore::omap_get_values(
   });
 }
 
-class SeaStoreOmapIterator : public FuturizedStore::OmapIterator {
-  using omap_values_t = FuturizedStore::omap_values_t;
-
-  SeaStore &seastore;
-  CollectionRef ch;
-  const ghobject_t oid;
-
-  omap_values_t current;
-  omap_values_t::iterator iter;
-
-  seastar::future<> repopulate_from(
-    std::optional<std::string> from,
-    bool inclusive) {
-    return seastar::do_with(
-      from,
-      [this, inclusive](auto &from) {
-	return seastore.omap_list(
-	  ch,
-	  oid,
-	  from,
-	  OMapManager::omap_list_config_t::with_inclusive(inclusive)
-	).safe_then([this](auto p) {
-	  auto &[complete, values] = p;
-	  current.swap(values);
-	  if (current.empty()) {
-	    assert(complete);
-	  }
-	  iter = current.begin();
-	});
-      }).handle_error(
-	crimson::ct_error::assert_all{
-	  "Invalid error in SeaStoreOmapIterator::repopulate_from"
-        }
-      );
-  }
-public:
-  SeaStoreOmapIterator(
-    SeaStore &seastore,
-    CollectionRef ch,
-    const ghobject_t &oid) :
-    seastore(seastore),
-    ch(ch),
-    oid(oid),
-    iter(current.begin())
-  {}
-
-  seastar::future<> seek_to_first() final {
-    return repopulate_from(
-      std::nullopt,
-      false);
-  }
-  seastar::future<> upper_bound(const std::string &after) final {
-    return repopulate_from(
-      after,
-      false);
-  }
-  seastar::future<> lower_bound(const std::string &from) final {
-    return repopulate_from(
-      from,
-      true);
-  }
-  bool valid() const {
-    return iter != current.end();
-  }
-  seastar::future<> next() final {
-    assert(valid());
-    auto prev = iter++;
-    if (iter == current.end()) {
-      return repopulate_from(
-	prev->first,
-	false);
-    } else {
-      return seastar::now();
-    }
-  }
-  std::string key() {
-    return iter->first;
-  }
-  ceph::buffer::list value() {
-    return iter->second;
-  }
-  int status() const {
-    return 0;
-  }
-  ~SeaStoreOmapIterator() {}
-};
-
-seastar::future<FuturizedStore::OmapIteratorRef> SeaStore::get_omap_iterator(
-  CollectionRef ch,
-  const ghobject_t& oid)
-{
-  LOG_PREFIX(SeaStore::get_omap_iterator);
-  DEBUG("oid: {}", oid);
-  auto ret = FuturizedStore::OmapIteratorRef(
-    new SeaStoreOmapIterator(
-      *this,
-      ch,
-      oid));
-  return ret->seek_to_first(
-  ).then([ret]() mutable {
-    return std::move(ret);
-  });
-}
-
 SeaStore::_fiemap_ret SeaStore::_fiemap(
   Transaction &t,
   Onode &onode,

src/crimson/os/seastore/seastore.h

@@ -155,9 +155,6 @@ public:
    * stages and locks as do_transaction. */
   seastar::future<> flush(CollectionRef ch) final;
 
-  seastar::future<OmapIteratorRef> get_omap_iterator(
-    CollectionRef ch,
-    const ghobject_t& oid) final;
   read_errorator::future<std::map<uint64_t, uint64_t>> fiemap(
     CollectionRef ch,
     const ghobject_t& oid,