Merge pull request #48345 from Matan-B/wip-matanb-c-load-obc

crimson/osd: Generalize loading obc for head/clone

Reviewed-by: Samuel Just <sjust@redhat.com>
Reviewed-by: Xuehan Xu <xxhdx1985126@gmail.com>

3 files changed, 41 insertions, 44 deletions

diff --git a/src/crimson/osd/object_context.h b/src/crimson/osd/object_context.h
index 35aae18cda4..a77fd0bc6a4 100644
--- a/src/crimson/osd/object_context.h
+++ b/src/crimson/osd/object_context.h
@@ -104,10 +104,9 @@ public:
     ssc = std::move(_ssc);
   }
 
-  void set_clone_state(ObjectState &&_obs, Ref &&_head) {
+  void set_clone_state(ObjectState &&_obs) {
     ceph_assert(!is_head());
     obs = std::move(_obs);
-    head = _head;
   }
 
   /// pass the provided exception to any waiting consumers of this ObjectContext
diff --git a/src/crimson/osd/pg.cc b/src/crimson/osd/pg.cc
index 692648cf302..a1808427ac3 100644
--- a/src/crimson/osd/pg.cc
+++ b/src/crimson/osd/pg.cc
@@ -1025,16 +1025,8 @@ PG::with_head_obc(ObjectContextRef obc, bool existed, with_obc_func_t&& func)
   obc->append_to(obc_set_accessing);
   return obc->with_lock<State, IOInterruptCondition>(
     [existed=existed, obc=obc, func=std::move(func), this] {
-    auto loaded = load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
-    if (existed) {
-      logger().debug("with_head_obc: found {} in cache", obc->get_oid());
-    } else {
-      logger().debug("with_head_obc: cache miss on {}", obc->get_oid());
-      loaded = obc->with_promoted_lock<State, IOInterruptCondition>([this, obc] {
-        return load_head_obc(obc);
-      });
-    }
-    return loaded.safe_then_interruptible([func = std::move(func)](auto obc) {
+    return get_or_load_obc<State>(obc, existed).safe_then_interruptible(
+      [func = std::move(func)](auto obc) {
       return std::move(func)(std::move(obc));
     });
   }).finally([this, pgref=boost::intrusive_ptr<PG>{this}, obc=std::move(obc)] {
@@ -1082,21 +1074,8 @@ PG::with_clone_obc(hobject_t oid, with_obc_func_t&& func)
     return clone->template with_lock<State, IOInterruptCondition>(
       [existed=existed, head=std::move(head), clone=std::move(clone),
        func=std::move(func), this]() -> load_obc_iertr::future<> {
-      auto loaded = load_obc_iertr::make_ready_future<ObjectContextRef>(clone);
-      if (existed) {
-        logger().debug("with_clone_obc: found {} in cache", clone->get_oid());
-      } else {
-        logger().debug("with_clone_obc: cache miss on {}", clone->get_oid());
-        //TODO: generalize load_head_obc -> load_obc (support head/clone obc)
-        loaded = clone->template with_promoted_lock<State, IOInterruptCondition>(
-          [clone, head, this] {
-          return backend->load_metadata(clone->get_oid()).safe_then_interruptible(
-            [clone=std::move(clone), head=std::move(head)](auto md) mutable {
-            clone->set_clone_state(std::move(md->os), std::move(head));
-            return clone;
-          });
-        });
-      }
+      auto loaded = get_or_load_obc<State>(clone, existed);
+      clone->head = head;
       return loaded.safe_then_interruptible([func = std::move(func)](auto clone) {
         return std::move(func)(std::move(clone));
       });
@@ -1126,29 +1105,51 @@ PG::with_existing_clone_obc(ObjectContextRef clone, with_obc_func_t&& func)
 }
 
 PG::load_obc_iertr::future<crimson::osd::ObjectContextRef>
-PG::load_head_obc(ObjectContextRef obc)
+PG::load_obc(ObjectContextRef obc)
 {
   return backend->load_metadata(obc->get_oid()).safe_then_interruptible(
     [obc=std::move(obc)](auto md)
     -> load_obc_ertr::future<crimson::osd::ObjectContextRef> {
     const hobject_t& oid = md->os.oi.soid;
     logger().debug(
-      "load_head_obc: loaded obs {} for {}", md->os.oi, oid);
-    if (!md->ssc) {
-      logger().error(
-        "load_head_obc: oid {} missing snapsetcontext", oid);
-      return crimson::ct_error::object_corrupted::make();
-
+      "load_obc: loaded obs {} for {}", md->os.oi, oid);
+    if (oid.is_head()) {
+      if (!md->ssc) {
+        logger().error(
+          "load_obc: oid {} missing snapsetcontext", oid);
+        return crimson::ct_error::object_corrupted::make();
+      }
+      obc->set_head_state(std::move(md->os), std::move(md->ssc));
+    } else {
+      obc->set_clone_state(std::move(md->os));
     }
-    obc->set_head_state(std::move(md->os), std::move(md->ssc));
     logger().debug(
-      "load_head_obc: returning obc {} for {}",
+      "load_obc: returning obc {} for {}",
       obc->obs.oi, obc->obs.oi.soid);
     return load_obc_ertr::make_ready_future<
       crimson::osd::ObjectContextRef>(obc);
   });
 }
 
+template<RWState::State State>
+PG::load_obc_iertr::future<crimson::osd::ObjectContextRef>
+PG::get_or_load_obc(
+    crimson::osd::ObjectContextRef obc,
+    bool existed)
+{
+  auto loaded = load_obc_iertr::make_ready_future<ObjectContextRef>(obc);
+  if (existed) {
+    logger().debug("{}: found {} in cache", __func__, obc->get_oid());
+  } else {
+    logger().debug("{}: cache miss on {}", __func__, obc->get_oid());
+    loaded = obc->template with_promoted_lock<State, IOInterruptCondition>(
+      [obc, this] {
+      return load_obc(obc);
+    });
+  }
+  return loaded;
+}
+
 PG::load_obc_iertr::future<>
 PG::reload_obc(crimson::osd::ObjectContext& obc) const
 {
diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
index db1b2ddfb11..565ac055983 100644
--- a/src/crimson/osd/pg.h
+++ b/src/crimson/osd/pg.h
@@ -518,17 +518,14 @@ public:
       load_obc_ertr>;
   using interruptor = ::crimson::interruptible::interruptor<
     ::crimson::osd::IOInterruptCondition>;
-  load_obc_iertr::future<
-    std::pair<crimson::osd::ObjectContextRef, bool>>
-  get_or_load_clone_obc(
-    hobject_t oid, crimson::osd::ObjectContextRef head_obc);
 
-  load_obc_iertr::future<
-    std::pair<crimson::osd::ObjectContextRef, bool>>
-  get_or_load_head_obc(hobject_t oid);
+  template<RWState::State State>
+  load_obc_iertr::future<crimson::osd::ObjectContextRef>
+  get_or_load_obc(
+    crimson::osd::ObjectContextRef head_obc, bool existed);
 
   load_obc_iertr::future<crimson::osd::ObjectContextRef>
-  load_head_obc(ObjectContextRef obc);
+  load_obc(ObjectContextRef obc);
 
   load_obc_iertr::future<>
   reload_obc(crimson::osd::ObjectContext& obc) const;