Merge pull request ceph#59118 from xxhdx1985126/wip-crimson-backfill-cancellation

crimson/osd/backfill_state: support backfill cancellation

Reviewed-by: Matan Breizman <[email protected]>

Author: Matan-B

src/crimson/osd/backfill_state.cc

@@ -498,6 +498,14 @@ BackfillState::Crashed::Crashed()
   ceph_abort_msg("{}: this should not happen");
 }
 
+// -- Cancelled
+BackfillState::Cancelled::Cancelled()
+{
+  backfill_state().backfill_info.clear();
+  backfill_state().peer_backfill_info.clear();
+  backfill_state().progress_tracker.reset();
+}
+
 // ProgressTracker is an intermediary between the BackfillListener and
 // BackfillMachine + its states. All requests to push or drop an object
 // are directed through it. The same happens with notifications about

src/crimson/osd/backfill_state.h

@@ -58,6 +58,9 @@ struct BackfillState {
   struct RequestDone : sc::event<RequestDone> {
   };
 
+  struct CancelBackfill : sc::event<CancelBackfill> {
+  };
+
 private:
   // internal events
   struct RequestPrimaryScanning : sc::event<RequestPrimaryScanning> {
@@ -132,10 +135,16 @@ struct BackfillState {
     explicit Crashed();
   };
 
+  struct Cancelled : sc::simple_state<Cancelled, BackfillMachine>,
+		     StateHelper<Cancelled> {
+    explicit Cancelled();
+  };
+
   struct Initial : sc::state<Initial, BackfillMachine>,
                    StateHelper<Initial> {
     using reactions = boost::mpl::list<
       sc::custom_reaction<Triggered>,
+      sc::transition<CancelBackfill, Cancelled>,
       sc::transition<sc::event_base, Crashed>>;
     explicit Initial(my_context);
     // initialize after triggering backfill by on_activate_complete().
@@ -146,6 +155,7 @@ struct BackfillState {
   struct Enqueuing : sc::state<Enqueuing, BackfillMachine>,
                      StateHelper<Enqueuing> {
     using reactions = boost::mpl::list<
+      sc::transition<CancelBackfill, Cancelled>,
       sc::transition<RequestPrimaryScanning, PrimaryScanning>,
       sc::transition<RequestReplicasScanning, ReplicasScanning>,
       sc::transition<RequestWaiting, Waiting>,
@@ -206,6 +216,7 @@ struct BackfillState {
       sc::custom_reaction<ObjectPushed>,
       sc::custom_reaction<PrimaryScanned>,
       sc::transition<RequestDone, Done>,
+      sc::transition<CancelBackfill, Cancelled>,
       sc::transition<sc::event_base, Crashed>>;
     explicit PrimaryScanning(my_context);
     sc::result react(ObjectPushed);
@@ -219,6 +230,7 @@ struct BackfillState {
       sc::custom_reaction<ObjectPushed>,
       sc::custom_reaction<ReplicaScanned>,
       sc::transition<RequestDone, Done>,
+      sc::transition<CancelBackfill, Cancelled>,
       sc::transition<sc::event_base, Crashed>>;
     explicit ReplicasScanning(my_context);
     // collect scanning result; if all results are collected, transition
@@ -243,6 +255,7 @@ struct BackfillState {
     using reactions = boost::mpl::list<
       sc::custom_reaction<ObjectPushed>,
       sc::transition<RequestDone, Done>,
+      sc::transition<CancelBackfill, Cancelled>,
       sc::transition<sc::event_base, Crashed>>;
     explicit Waiting(my_context);
     sc::result react(ObjectPushed);

src/crimson/osd/pg.h

@@ -417,7 +417,7 @@ class PG : public boost::intrusive_ref_counter<
     recovery_handler->on_backfill_reserved();
   }
   void on_backfill_canceled() final {
-    ceph_assert(0 == "Not implemented");
+    recovery_handler->backfill_cancelled();
   }
 
   void on_recovery_reserved() final {

src/crimson/osd/pg_recovery.cc

@@ -605,6 +605,7 @@ bool PGRecovery::budget_available() const
 
 void PGRecovery::backfilled()
 {
+  backfill_state.reset();
   using LocalPeeringEvent = crimson::osd::LocalPeeringEvent;
   std::ignore = pg->get_shard_services().start_operation<LocalPeeringEvent>(
     static_cast<crimson::osd::PG*>(pg),
@@ -615,11 +616,30 @@ void PGRecovery::backfilled()
     PeeringState::Backfilled{});
 }
 
+void PGRecovery::backfill_cancelled()
+{
+  // We are not creating a new BackfillRecovery request here, as we
+  // need to cancel the backfill synchronously (before this method returns).
+  using BackfillState = crimson::osd::BackfillState;
+  backfill_state->process_event(
+    BackfillState::CancelBackfill{}.intrusive_from_this());
+  backfill_state.reset();
+}
+
 void PGRecovery::dispatch_backfill_event(
   boost::intrusive_ptr<const boost::statechart::event_base> evt)
 {
   logger().debug("{}", __func__);
-  backfill_state->process_event(evt);
+  if (backfill_state) {
+    backfill_state->process_event(evt);
+  } else {
+    // TODO: Do we need to worry about cases in which the pg has
+    // 	     been through both backfill cancellations and backfill
+    // 	     restarts between the sendings and replies of
+    // 	     ReplicaScan/ObjectPush requests? Seems classic OSDs
+    // 	     doesn't handle these cases.
+    logger().debug("{}, backfill cancelled, dropping evt");
+  }
 }
 
 void PGRecovery::on_backfill_reserved()

src/crimson/osd/pg_recovery.h

@@ -97,6 +97,7 @@ class PGRecovery : public crimson::osd::BackfillState::BackfillListener {
   template <class EventT>
   void start_backfill_recovery(
     const EventT& evt);
+  void backfill_cancelled();
   void request_replica_scan(
     const pg_shard_t& target,
     const hobject_t& begin,