Merge pull request #60517 from xxhdx1985126/wip-68737

crimson/osd/osd_operations/client_request: no need to hold head's obc lock throughout the recovery of clones

Reviewed-by: Matan Breizman <mbreizma@redhat.com>

2 files changed, 31 insertions, 29 deletions

diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc
index a89fb2c84bc..7c79795f224 100644
--- a/src/crimson/osd/osd_operations/client_request.cc
+++ b/src/crimson/osd/osd_operations/client_request.cc
@@ -290,29 +290,41 @@ ClientRequest::process_pg_op(
 ClientRequest::interruptible_future<>
 ClientRequest::recover_missing_snaps(
   Ref<PG> pg,
-  instance_handle_t &ihref,
-  ObjectContextRef head,
   std::set<snapid_t> &snaps)
 {
   LOG_PREFIX(ClientRequest::recover_missing_snaps);
-  for (auto &snap : snaps) {
-    auto coid = head->obs.oi.soid;
-    coid.snap = snap;
-    auto oid = resolve_oid(head->get_head_ss(), coid);
-    /* Rollback targets may legitimately not exist if, for instance,
-     * the object is an rbd block which happened to be sparse and
-     * therefore non-existent at the time of the specified snapshot.
-     * In such a case, rollback will simply delete the object.  Here,
-     * we skip the oid as there is no corresponding clone to recover.
-     * See https://tracker.ceph.com/issues/63821 */
-    if (oid) {
-      auto unfound = co_await do_recover_missing(pg, *oid, m->get_reqid());
-      if (unfound) {
-        DEBUGDPP("{} unfound, hang it for now", *pg, *oid);
-        co_await interruptor::make_interruptible(
-          pg->get_recovery_backend()->add_unfound(*oid));
+
+  std::vector<hobject_t> ret;
+  auto resolve_oids = pg->obc_loader.with_obc<RWState::RWREAD>(
+    m->get_hobj().get_head(),
+    [&snaps, &ret](auto head, auto) {
+    for (auto &snap : snaps) {
+      auto coid = head->obs.oi.soid;
+      coid.snap = snap;
+      auto oid = resolve_oid(head->get_head_ss(), coid);
+      /* Rollback targets may legitimately not exist if, for instance,
+       * the object is an rbd block which happened to be sparse and
+       * therefore non-existent at the time of the specified snapshot.
+       * In such a case, rollback will simply delete the object.  Here,
+       * we skip the oid as there is no corresponding clone to recover.
+       * See https://tracker.ceph.com/issues/63821 */
+      if (oid) {
+        ret.emplace_back(std::move(*oid));
       }
     }
+    return seastar::now();
+  }).handle_error_interruptible(
+    crimson::ct_error::assert_all("unexpected error")
+  );
+  co_await std::move(resolve_oids);
+
+  for (auto &oid : ret) {
+    auto unfound = co_await do_recover_missing(pg, oid, m->get_reqid());
+    if (unfound) {
+      DEBUGDPP("{} unfound, hang it for now", *pg, oid);
+      co_await interruptor::make_interruptible(
+        pg->get_recovery_backend()->add_unfound(oid));
+    }
   }
 }
 
@@ -337,15 +349,7 @@ ClientRequest::process_op(
 
     std::set<snapid_t> snaps = snaps_need_to_recover();
     if (!snaps.empty()) {
-      auto with_obc = pg->obc_loader.with_obc<RWState::RWREAD>(
-        m->get_hobj().get_head(),
-        [&snaps, &ihref, pg, this](auto head, auto) {
-        return recover_missing_snaps(pg, ihref, head, snaps);
-      }).handle_error_interruptible(
-        crimson::ct_error::assert_all("unexpected error")
-      );
-      // see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98401
-      co_await std::move(with_obc);
+      co_await recover_missing_snaps(pg, snaps);
     }
   }
 
diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h
index 6ee57e9874c..9df33127fb0 100644
--- a/src/crimson/osd/osd_operations/client_request.h
+++ b/src/crimson/osd/osd_operations/client_request.h
@@ -285,8 +285,6 @@ private:
   interruptible_future<>
   recover_missing_snaps(
     Ref<PG> pg,
-    instance_handle_t &ihref,
-    ObjectContextRef head,
     std::set<snapid_t> &snaps);
   ::crimson::interruptible::interruptible_future<
     ::crimson::osd::IOInterruptCondition> process_op(