crimson: refactor RecoverySubRequest to use start_pg_operation

A side effect of this PR is that RecoverySubRequest will now use the
peering pipeline for ordering.  Not using before was, I think, actually
a bug which could result in pull operations ordering ahead of the
activation message when pulling objects from non-acting osds.

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/osd/osd.cc b/src/crimson/osd/osd.cc
index 6e1d1bf5191e2307dd72440002b1fa8e6c279a18..2bd2fe8408cb3556920fe0f9f4a0afe4802f03e2 100644 (file)
--- a/src/crimson/osd/osd.cc
+++ b/src/crimson/osd/osd.cc
@@ -1288,10 +1288,7 @@ seastar::future<> OSD::handle_mark_me_down(crimson::net::ConnectionRef conn,
 seastar::future<> OSD::handle_recovery_subreq(crimson::net::ConnectionRef conn,
                                   Ref<MOSDFastDispatchOp> m)
 {
-  (void) shard_services.start_operation<RecoverySubRequest>(
-    *this,
-    conn,
-    std::move(m));
+  std::ignore = start_pg_operation<RecoverySubRequest>(conn, std::move(m));
   return seastar::now();
 }
 

diff --git a/src/crimson/osd/osd_operations/recovery_subrequest.cc b/src/crimson/osd/osd_operations/recovery_subrequest.cc
index b501b7bbb006f70fc7004aba3476b3dde420ea98..7e5dd76e0a8a7ac784d5fbb013ea6b193d4a4115 100644 (file)
--- a/src/crimson/osd/osd_operations/recovery_subrequest.cc
+++ b/src/crimson/osd/osd_operations/recovery_subrequest.cc
@@ -3,6 +3,7 @@
 
 #include "crimson/osd/osd_operations/recovery_subrequest.h"
 #include "crimson/osd/pg.h"
+#include "crimson/osd/osd_connection_priv.h"
 
 namespace {
   seastar::logger& logger() {
@@ -21,31 +22,25 @@ namespace crimson {
 
 namespace crimson::osd {
 
-seastar::future<> RecoverySubRequest::start() {
-  logger().debug("{}: start", *this);
+seastar::future<> RecoverySubRequest::with_pg(
+  ShardServices &shard_services, Ref<PG> pgref)
+{
+  logger().debug("{}: {}", "RecoverySubRequest::with_pg", *this);
 
   track_event<StartEvent>();
   IRef opref = this;
-  using OSDMapBlockingEvent =
-    OSD_OSDMapGate::OSDMapBlocker::BlockingEvent;
-  return with_blocking_event<OSDMapBlockingEvent>(
-    [this] (auto&& trigger) {
-    return osd.osdmap_gate.wait_for_map(std::move(trigger), m->get_min_epoch());
-  }).then([this] (epoch_t epoch) {
-    return with_blocking_event<PGMap::PGCreationBlockingEvent>(
-    [this] (auto&& trigger) {
-      return osd.wait_for_pg(std::move(trigger), m->get_spg());
-    });
-  }).then([this, opref=std::move(opref)] (Ref<PG> pgref) {
-    return interruptor::with_interruption([this, opref, pgref] {
-      return seastar::do_with(std::move(pgref), std::move(opref),
-       [this](auto& pgref, auto& opref) {
-       return pgref->get_recovery_backend()->handle_recovery_op(m);
-      });
-    }, [](std::exception_ptr) { return seastar::now(); }, pgref);
-  }).then([this] {
+  return interruptor::with_interruption([this, pgref] {
+    return pgref->get_recovery_backend()->handle_recovery_op(m);
+  }, [](std::exception_ptr) {
+    return seastar::now();
+  }, pgref).finally([this, opref, pgref] {
     track_event<CompletionEvent>();
   });
 }
 
+ConnectionPipeline &RecoverySubRequest::get_connection_pipeline()
+{
+  return get_osd_priv(conn.get()).peering_request_conn_pipeline;
+}
+
 }

diff --git a/src/crimson/osd/osd_operations/recovery_subrequest.h b/src/crimson/osd/osd_operations/recovery_subrequest.h
index 1c50b5dd15cb553881152e5879e412bf4389cd05..7c4d106712d16367238cd7889e9e2ab0909ebaee 100644 (file)
--- a/src/crimson/osd/osd_operations/recovery_subrequest.h
+++ b/src/crimson/osd/osd_operations/recovery_subrequest.h
@@ -6,21 +6,23 @@
 #include "osd/osd_op_util.h"
 #include "crimson/net/Connection.h"
 #include "crimson/osd/osd_operation.h"
-#include "crimson/osd/osd.h"
+#include "crimson/osd/pg.h"
 #include "crimson/common/type_helpers.h"
 #include "messages/MOSDFastDispatchOp.h"
 
 namespace crimson::osd {
 
-class OSD;
 class PG;
 
-class RecoverySubRequest final : public TrackableOperationT<RecoverySubRequest> {
+class RecoverySubRequest final : public PhasedOperationT<RecoverySubRequest> {
 public:
-  static constexpr OperationTypeCode type = OperationTypeCode::background_recovery_sub;
+  static constexpr OperationTypeCode type =
+    OperationTypeCode::background_recovery_sub;
 
-  RecoverySubRequest(OSD &osd, crimson::net::ConnectionRef conn, Ref<MOSDFastDispatchOp>&& m)
-    : osd(osd), conn(conn), m(m) {}
+  RecoverySubRequest(
+    crimson::net::ConnectionRef conn,
+    Ref<MOSDFastDispatchOp>&& m)
+    : conn(conn), m(m) {}
 
   void print(std::ostream& out) const final
   {
@@ -31,19 +33,30 @@ public:
   {
   }
 
-  seastar::future<> start();
-private:
-  OSD& osd;
-  crimson::net::ConnectionRef conn;
-  Ref<MOSDFastDispatchOp> m;
+  static constexpr bool can_create() { return false; }
+  spg_t get_pgid() const {
+    return m->get_spg();
+  }
+  ConnectionPipeline &get_connection_pipeline();
+  PipelineHandle &get_handle() { return handle; }
+  epoch_t get_epoch() const { return m->get_min_epoch(); }
+
+  seastar::future<> with_pg(
+    ShardServices &shard_services, Ref<PG> pg);
 
-public:
   std::tuple<
     StartEvent,
-    OSD_OSDMapGate::OSDMapBlocker::BlockingEvent,
+    ConnectionPipeline::AwaitActive::BlockingEvent,
+    ConnectionPipeline::AwaitMap::BlockingEvent,
+    ConnectionPipeline::GetPG::BlockingEvent,
     PGMap::PGCreationBlockingEvent,
+    OSD_OSDMapGate::OSDMapBlocker::BlockingEvent,
     CompletionEvent
   > tracking_events;
+
+private:
+  crimson::net::ConnectionRef conn;
+  Ref<MOSDFastDispatchOp> m;
 };
 
 }