crimson/osd/osd_operations/client_request: check already complete in the

"check_already_complete_get_obc" phase

Otherwise, the replies can go out-of-order

Signed-off-by: Xuehan Xu <xuxuehan@qianxin.com>

4 files changed, 27 insertions, 6 deletions

diff --git a/src/crimson/osd/osd_operation_external_tracking.h b/src/crimson/osd/osd_operation_external_tracking.h
index 5d3bba58db0..530732ba710 100644
--- a/src/crimson/osd/osd_operation_external_tracking.h
+++ b/src/crimson/osd/osd_operation_external_tracking.h
@@ -35,6 +35,7 @@ struct LttngBackend
     ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
     ClientRequest::PGPipeline::RecoverMissing::
       BlockingEvent::ExitBarrierEvent::Backend,
+    ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent::Backend,
     ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
     ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend,
     ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend,
@@ -111,6 +112,11 @@ struct LttngBackend
 	      const Operation& op) override {
   }
 
+  void handle(ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent& ev,
+              const Operation& op,
+              const ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc& blocker) override {
+  }
+
   void handle(ClientRequest::PGPipeline::GetOBC::BlockingEvent& ev,
               const Operation& op,
               const ClientRequest::PGPipeline::GetOBC& blocker) override {
@@ -164,6 +170,7 @@ struct HistoricBackend
     ClientRequest::PGPipeline::RecoverMissing::BlockingEvent::Backend,
     ClientRequest::PGPipeline::RecoverMissing::
       BlockingEvent::ExitBarrierEvent::Backend,
+    ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent::Backend,
     ClientRequest::PGPipeline::GetOBC::BlockingEvent::Backend,
     ClientRequest::PGPipeline::LockOBC::BlockingEvent::Backend,
     ClientRequest::PGPipeline::LockOBC::BlockingEvent::ExitBarrierEvent::Backend,
@@ -240,6 +247,11 @@ struct HistoricBackend
               const Operation& op) override {
   }
 
+  void handle(ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent& ev,
+              const Operation& op,
+              const ClientRequest::PGPipeline::CheckAlreadyCompleteGetObc& 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 4c359410871..07570d39883 100644
--- a/src/crimson/osd/osd_operations/client_request.cc
+++ b/src/crimson/osd/osd_operations/client_request.cc
@@ -337,7 +337,6 @@ ClientRequest::process_op(
 
     std::set<snapid_t> snaps = snaps_need_to_recover();
     if (!snaps.empty()) {
-      // call with_obc() in order, but wait concurrently for loading.
       auto with_obc = pg->obc_loader.with_obc<RWState::RWREAD>(
         m->get_hobj().get_head(),
         [&snaps, &ihref, pg, this](auto head, auto) {
@@ -350,6 +349,16 @@ ClientRequest::process_op(
     }
   }
 
+  /**
+   * The previous stage of recover_missing is a concurrent phase.
+   * Checking for already_complete requests must done exclusively.
+   * Since get_obc is also an exclusive stage, we can merge both stages into
+   * a single stage and avoid stage switching overhead.
+   */
+  DEBUGDPP("{}.{}: entering check_already_complete_get_obc",
+	   *pg, *this, this_instance_id);
+  co_await ihref.enter_stage<interruptor>(
+    client_pp(*pg).check_already_complete_get_obc, *this);
   DEBUGDPP("{}.{}: checking already_complete",
 	   *pg, *this, this_instance_id);
   auto completed = co_await pg->already_complete(m->get_reqid());
@@ -368,11 +377,7 @@ ClientRequest::process_op(
     co_return;
   }
 
-  DEBUGDPP("{}.{}: not completed, entering get_obc stage",
-	   *pg, *this, this_instance_id);
-  co_await ihref.enter_stage<interruptor>(client_pp(*pg).get_obc, *this);
-
-  DEBUGDPP("{}.{}: entered get_obc stage, about to wait_scrub",
+  DEBUGDPP("{}.{}: not completed, about to wait_scrub",
 	   *pg, *this, this_instance_id);
   co_await ihref.enter_blocker(
     *this, pg->scrubber, &decltype(pg->scrubber)::wait_scrub,
diff --git a/src/crimson/osd/osd_operations/client_request.h b/src/crimson/osd/osd_operations/client_request.h
index c5ab670c78b..ea7aade22ac 100644
--- a/src/crimson/osd/osd_operations/client_request.h
+++ b/src/crimson/osd/osd_operations/client_request.h
@@ -103,6 +103,7 @@ public:
       PGActivationBlocker::BlockingEvent,
       PGPipeline::RecoverMissing::BlockingEvent,
       scrub::PGScrubber::BlockingEvent,
+      PGPipeline::CheckAlreadyCompleteGetObc::BlockingEvent,
       PGPipeline::GetOBC::BlockingEvent,
       PGPipeline::LockOBC::BlockingEvent,
       PGPipeline::Process::BlockingEvent,
diff --git a/src/crimson/osd/osd_operations/common/pg_pipeline.h b/src/crimson/osd/osd_operations/common/pg_pipeline.h
index 5984a1399ba..2b2d03ae4b3 100644
--- a/src/crimson/osd/osd_operations/common/pg_pipeline.h
+++ b/src/crimson/osd/osd_operations/common/pg_pipeline.h
@@ -20,6 +20,9 @@ protected:
   struct RecoverMissing : OrderedConcurrentPhaseT<RecoverMissing> {
     static constexpr auto type_name = "CommonPGPipeline::recover_missing";
   } recover_missing;
+  struct CheckAlreadyCompleteGetObc : OrderedExclusivePhaseT<CheckAlreadyCompleteGetObc> {
+    static constexpr auto type_name = "CommonPGPipeline::check_already_complete_get_obc";
+  } check_already_complete_get_obc;
   struct GetOBC : OrderedExclusivePhaseT<GetOBC> {
     static constexpr auto type_name = "CommonPGPipeline::get_obc";
   } get_obc;