crimson/osd/osd_operations/client_request: make loading-obc concurrent during recover...

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/osd/osd_operation_external_tracking.h b/src/crimson/osd/osd_operation_external_tracking.h
index 5bc457c7809616c763d81dba8fe2d06855dbde3c..4fffd8e82b00bb7468197e5139b5bdfe95a95982 100644 (file)
--- a/src/crimson/osd/osd_operation_external_tracking.h
+++ b/src/crimson/osd/osd_operation_external_tracking.h
@@ -33,6 +33,10 @@ struct LttngBackend
     PGActivationBlocker::BlockingEvent::Backend,
     scrub::PGScrubber::BlockingEvent::Backend,
     ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
+    ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent::Backend,
+    ClientRequest::PGPipeline::RecoverMissingLockOBC::
+      BlockingEvent::ExitBarrierEvent::Backend,
+    ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent::Backend,
     ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
     ClientRequest::PGPipeline::Process::BlockingEvent::Backend,
     ClientRequest::PGPipeline::WaitRepop::BlockingEvent::Backend,
@@ -103,6 +107,21 @@ struct LttngBackend
               const ClientRequest::PGPipeline::RecoverMissing& blocker) override {
   }
 
+  void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent& ev,
+              const Operation& op,
+              const ClientRequest::PGPipeline::RecoverMissingLockOBC& blocker) override {
+  }
+
+  void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::
+                BlockingEvent::ExitBarrierEvent& ev,
+              const Operation& op) override {
+  }
+
+  void handle(ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent& ev,
+              const Operation& op,
+              const ClientRequest::PGPipeline::RecoverMissingSnaps& blocker) override {
+  }
+
   void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev,
               const Operation& op,
               const ClientRequest::PGPipeline::GetOBC& blocker) override {
@@ -145,6 +164,10 @@ struct HistoricBackend
     PGActivationBlocker::BlockingEvent::Backend,
     scrub::PGScrubber::BlockingEvent::Backend,
     ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
+    ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent::Backend,
+    ClientRequest::PGPipeline::RecoverMissingLockOBC::
+      BlockingEvent::ExitBarrierEvent::Backend,
+    ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent::Backend,
     ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
     ClientRequest::PGPipeline::Process::BlockingEvent::Backend,
     ClientRequest::PGPipeline::WaitRepop::BlockingEvent::Backend,
@@ -215,6 +238,21 @@ struct HistoricBackend
               const ClientRequest::PGPipeline::RecoverMissing& blocker) override {
   }
 
+  void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::BlockingEvent& ev,
+              const Operation& op,
+              const ClientRequest::PGPipeline::RecoverMissingLockOBC& blocker) override {
+  }
+
+  void handle(ClientRequest::PGPipeline::RecoverMissingLockOBC::
+                BlockingEvent::ExitBarrierEvent& ev,
+              const Operation& op) override {
+  }
+
+  void handle(ClientRequest::PGPipeline::RecoverMissingSnaps::BlockingEvent& ev,
+              const Operation& op,
+              const ClientRequest::PGPipeline::RecoverMissingSnaps& blocker) override {
+  }
+
   void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev,
               const Operation& op,
               const ClientRequest::PGPipeline::GetOBC& blocker) override {

diff --git a/src/crimson/osd/osd_operations/client_request.cc b/src/crimson/osd/osd_operations/client_request.cc
index 8f31284dab4b60b469fbf7bc394f93c894040db6..3f2399684a321d39660ec5d16e305d15a2773674 100644 (file)
--- a/src/crimson/osd/osd_operations/client_request.cc
+++ b/src/crimson/osd/osd_operations/client_request.cc
@@ -252,6 +252,31 @@ ClientRequest::process_pg_op(
     get_foreign_connection().send_with_throttling(std::move(reply)));
 }
 
+ClientRequest::interruptible_future<>
+ClientRequest::recover_missing_snaps(
+  Ref<PG> pg,
+  instance_handle_t &ihref,
+  ObjectContextRef head,
+  std::set<snapid_t> &snaps)
+{
+  co_await ihref.enter_stage<interruptor>(
+    client_pp(*pg).recover_missing_snaps, *this);
+  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) {
+      co_await do_recover_missing(pg, *oid, m->get_reqid());
+    }
+  }
+}
+
 ClientRequest::interruptible_future<>
 ClientRequest::process_op(
   instance_handle_t &ihref, Ref<PG> pg, unsigned this_instance_id)
@@ -268,28 +293,13 @@ ClientRequest::process_op(
     co_await do_recover_missing(pg, m->get_hobj().get_head(), m->get_reqid());
     std::set<snapid_t> snaps = snaps_need_to_recover();
     if (!snaps.empty()) {
+      // call with_obc() in order, but wait concurrently for loading.
+      ihref.enter_stage_sync(
+          client_pp(*pg).recover_missing_lock_obc, *this);
       auto with_obc = pg->obc_loader.with_obc<RWState::RWREAD>(
         m->get_hobj().get_head(),
-        [&snaps, pg, this](auto head, auto) {
-        return InterruptibleOperation::interruptor::do_for_each(
-          snaps,
-          [head, pg, this](auto &snap)
-          -> InterruptibleOperation::template interruptible_future<> {
-          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) {
-            return do_recover_missing(pg, *oid, m->get_reqid());
-          } else {
-            return seastar::now();
-          }
-        });
+        [&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")
       );

diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h
index c6ee364db59963750f5ee68ffee327da58b3b113..d5cbc94c1c40b142d5e4ef8a199ef22dfada2eef 100644 (file)
--- a/src/crimson/osd/osd_operations/client_request.h
+++ b/src/crimson/osd/osd_operations/client_request.h
@@ -44,6 +44,12 @@ class ClientRequest final : public PhasedOperationT<ClientRequest>,
 public:
   class PGPipeline : public CommonPGPipeline {
     public:
+    struct RecoverMissingLockOBC : OrderedConcurrentPhaseT<RecoverMissingLockOBC> {
+      static constexpr auto type_name = "ClientRequest::PGPipeline::recover_missing_lock_obc";
+    } recover_missing_lock_obc;
+    struct RecoverMissingSnaps : OrderedExclusivePhaseT<RecoverMissingSnaps> {
+      static constexpr auto type_name = "ClientRequest::PGPipeline::recover_missing_snaps";
+    } recover_missing_snaps;
     struct AwaitMap : OrderedExclusivePhaseT<AwaitMap> {
       static constexpr auto type_name = "ClientRequest::PGPipeline::await_map";
     } await_map;
@@ -105,6 +111,8 @@ public:
       PGPipeline::WaitForActive::BlockingEvent,
       PGActivationBlocker::BlockingEvent,
       PGPipeline::RecoverMissing::BlockingEvent,
+      PGPipeline::RecoverMissingLockOBC::BlockingEvent,
+      PGPipeline::RecoverMissingSnaps::BlockingEvent,
       scrub::PGScrubber::BlockingEvent,
       PGPipeline::GetOBC::BlockingEvent,
       PGPipeline::Process::BlockingEvent,
@@ -289,6 +297,12 @@ private:
   ::crimson::interruptible::interruptible_future<
     ::crimson::osd::IOInterruptCondition> process_pg_op(
     Ref<PG> pg);
+  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(
       instance_handle_t &ihref,

diff --git a/src/crimson/osd/osd_operations/common/pg_pipeline.h b/src/crimson/osd/osd_operations/common/pg_pipeline.h
index 1e5d0e511f6745189dae692868b52f7c1fae57d9..58fa07b8b4d2537c5c978d0a0d2f71799664ce32 100644 (file)
--- a/src/crimson/osd/osd_operations/common/pg_pipeline.h
+++ b/src/crimson/osd/osd_operations/common/pg_pipeline.h
@@ -19,7 +19,7 @@ protected:
   } wait_for_active;
   struct RecoverMissing : OrderedExclusivePhaseT<RecoverMissing> {
     static constexpr auto type_name = "CommonPGPipeline::recover_missing";
-  } recover_missing, recover_missing2;
+  } recover_missing;
   struct GetOBC : OrderedExclusivePhaseT<GetOBC> {
     static constexpr auto type_name = "CommonPGPipeline::get_obc";
   } get_obc;