Merge pull request ceph#61232 from xxhdx1985126/wip-67888-followup

yuriw · web-flow · commit 2fe3f079c742 · 2025-02-03T07:59:32.000-08:00
osd/PeeringState: rename "cancel_backfill" to "suspend_backfill"

Reviewed-by: Samuel Just &lt;sjust@redhat.com&gt;
diff --git a/src/crimson/osd/backfill_state.cc b/src/crimson/osd/backfill_state.cc
@@ -427,7 +427,7 @@ BackfillState::PrimaryScanning::react(PrimaryScanned evt)
 }
 
 boost::statechart::result
-BackfillState::PrimaryScanning::react(CancelBackfill evt)
+BackfillState::PrimaryScanning::react(SuspendBackfill evt)
 {
   LOG_PREFIX(BackfillState::PrimaryScanning::react::SuspendBackfill);
   DEBUGDPP("suspended within PrimaryScanning", pg());
@@ -523,7 +523,7 @@ BackfillState::ReplicasScanning::react(ReplicaScanned evt)
 }
 
 boost::statechart::result
-BackfillState::ReplicasScanning::react(CancelBackfill evt)
+BackfillState::ReplicasScanning::react(SuspendBackfill evt)
 {
   LOG_PREFIX(BackfillState::ReplicasScanning::react::SuspendBackfill);
   DEBUGDPP("suspended within ReplicasScanning", pg());
@@ -576,7 +576,7 @@ BackfillState::Waiting::react(ObjectPushed evt)
 }
 
 boost::statechart::result
-BackfillState::Waiting::react(CancelBackfill evt)
+BackfillState::Waiting::react(SuspendBackfill evt)
 {
   LOG_PREFIX(BackfillState::Waiting::react::SuspendBackfill);
   DEBUGDPP("suspended within Waiting", pg());
diff --git a/src/crimson/osd/backfill_state.h b/src/crimson/osd/backfill_state.h
@@ -59,7 +59,7 @@ struct BackfillState {
   struct RequestDone : sc::event<RequestDone> {
   };
 
-  struct CancelBackfill : sc::event<CancelBackfill> {
+  struct SuspendBackfill : sc::event<SuspendBackfill> {
   };
 
   struct ThrottleAcquired : sc::event<ThrottleAcquired> {
@@ -212,14 +212,14 @@ struct BackfillState {
       sc::custom_reaction<ObjectPushed>,
       sc::custom_reaction<PrimaryScanned>,
       sc::transition<RequestDone, Done>,
-      sc::custom_reaction<CancelBackfill>,
+      sc::custom_reaction<SuspendBackfill>,
       sc::custom_reaction<Triggered>,
       sc::transition<sc::event_base, Crashed>>;
     explicit PrimaryScanning(my_context);
     sc::result react(ObjectPushed);
     // collect scanning result and transit to Enqueuing.
     sc::result react(PrimaryScanned);
-    sc::result react(CancelBackfill);
+    sc::result react(SuspendBackfill);
     sc::result react(Triggered);
   };
 
@@ -228,7 +228,7 @@ struct BackfillState {
     using reactions = boost::mpl::list<
       sc::custom_reaction<ObjectPushed>,
       sc::custom_reaction<ReplicaScanned>,
-      sc::custom_reaction<CancelBackfill>,
+      sc::custom_reaction<SuspendBackfill>,
       sc::custom_reaction<Triggered>,
       sc::transition<RequestDone, Done>,
       sc::transition<sc::event_base, Crashed>>;
@@ -237,7 +237,7 @@ struct BackfillState {
     // to Enqueuing will happen.
     sc::result react(ObjectPushed);
     sc::result react(ReplicaScanned);
-    sc::result react(CancelBackfill);
+    sc::result react(SuspendBackfill);
     sc::result react(Triggered);
 
     // indicate whether a particular peer should be scanned to retrieve
@@ -257,23 +257,23 @@ struct BackfillState {
     using reactions = boost::mpl::list<
       sc::custom_reaction<ObjectPushed>,
       sc::transition<RequestDone, Done>,
-      sc::custom_reaction<CancelBackfill>,
+      sc::custom_reaction<SuspendBackfill>,
       sc::custom_reaction<Triggered>,
       sc::transition<ThrottleAcquired, Enqueuing>,
       sc::transition<sc::event_base, Crashed>>;
     explicit Waiting(my_context);
     sc::result react(ObjectPushed);
-    sc::result react(CancelBackfill);
+    sc::result react(SuspendBackfill);
     sc::result react(Triggered);
   };
 
   struct Done : sc::state<Done, BackfillMachine>,
                 StateHelper<Done> {
     using reactions = boost::mpl::list<
-      sc::custom_reaction<CancelBackfill>,
+      sc::custom_reaction<SuspendBackfill>,
       sc::transition<sc::event_base, Crashed>>;
     explicit Done(my_context);
-    sc::result react(CancelBackfill) {
+    sc::result react(SuspendBackfill) {
       return discard_event();
     }
   };
diff --git a/src/crimson/osd/pg.h b/src/crimson/osd/pg.h
@@ -434,8 +434,8 @@ class PG : public boost::intrusive_ref_counter<
   void on_backfill_reserved() final {
     recovery_handler->on_backfill_reserved();
   }
-  void on_backfill_canceled() final {
-    recovery_handler->backfill_cancelled();
+  void on_backfill_suspended() final {
+    recovery_handler->backfill_suspended();
   }
 
   void on_recovery_cancelled() final {
diff --git a/src/crimson/osd/pg_recovery.cc b/src/crimson/osd/pg_recovery.cc
@@ -641,13 +641,13 @@ void PGRecovery::backfilled()
     PeeringState::Backfilled{});
 }
 
-void PGRecovery::backfill_cancelled()
+void PGRecovery::backfill_suspended()
 {
   // 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());
+    BackfillState::SuspendBackfill{}.intrusive_from_this());
 }
 
 void PGRecovery::dispatch_backfill_event(
diff --git a/src/crimson/osd/pg_recovery.h b/src/crimson/osd/pg_recovery.h
@@ -107,7 +107,7 @@ class PGRecovery : public crimson::osd::BackfillState::BackfillListener {
   template <class EventT>
   void start_backfill_recovery(
     const EventT& evt);
-  void backfill_cancelled();
+  void backfill_suspended();
   void request_replica_scan(
     const pg_shard_t& target,
     const hobject_t& begin,
diff --git a/src/osd/PG.cc b/src/osd/PG.cc
@@ -1563,8 +1563,12 @@ void PG::on_backfill_reserved()
   queue_recovery();
 }
 
-void PG::on_backfill_canceled()
+void PG::on_backfill_suspended()
 {
+  // Scan replies asked before suspending this backfill should be ignored.
+  // See PrimaryLogPG::do_scan -  case MOSDPGScan::OP_SCAN_DIGEST.
+  // `waiting_on_backfill` will be re-refilled after the suspended backfill
+  // is resumed/restarted.
   if (!waiting_on_backfill.empty()) {
     waiting_on_backfill.clear();
     finish_recovery_op(hobject_t::get_max());
diff --git a/src/osd/PG.h b/src/osd/PG.h
@@ -604,7 +604,7 @@ class PG : public DoutPrefixProvider,
   void queue_snap_retrim(snapid_t snap);
 
   void on_backfill_reserved() override;
-  void on_backfill_canceled() override;
+  void on_backfill_suspended() override;
   void on_recovery_cancelled() override {}
   void on_recovery_reserved() override;
 
diff --git a/src/osd/PeeringState.cc b/src/osd/PeeringState.cc
@@ -5106,11 +5106,11 @@ void PeeringState::Backfilling::backfill_release_reservations()
   }
 }
 
-void PeeringState::Backfilling::cancel_backfill()
+void PeeringState::Backfilling::suspend_backfill()
 {
   DECLARE_LOCALS;
   backfill_release_reservations();
-  pl->on_backfill_canceled();
+  pl->on_backfill_suspended();
 }
 
 boost::statechart::result
@@ -5128,7 +5128,7 @@ PeeringState::Backfilling::react(const DeferBackfill &c)
   psdout(10) << "defer backfill, retry delay " << c.delay << dendl;
   ps->state_set(PG_STATE_BACKFILL_WAIT);
   ps->state_clear(PG_STATE_BACKFILLING);
-  cancel_backfill();
+  suspend_backfill();
 
   pl->schedule_event_after(
     std::make_shared<PGPeeringEvent>(
@@ -5146,7 +5146,7 @@ PeeringState::Backfilling::react(const UnfoundBackfill &c)
   psdout(10) << "backfill has unfound, can't continue" << dendl;
   ps->state_set(PG_STATE_BACKFILL_UNFOUND);
   ps->state_clear(PG_STATE_BACKFILLING);
-  cancel_backfill();
+  suspend_backfill();
   return transit<NotBackfilling>();
 }
 
@@ -5157,7 +5157,7 @@ PeeringState::Backfilling::react(const RemoteReservationRevokedTooFull &)
 
   ps->state_set(PG_STATE_BACKFILL_TOOFULL);
   ps->state_clear(PG_STATE_BACKFILLING);
-  cancel_backfill();
+  suspend_backfill();
 
   pl->schedule_event_after(
     std::make_shared<PGPeeringEvent>(
@@ -5174,7 +5174,7 @@ PeeringState::Backfilling::react(const RemoteReservationRevoked &)
 {
   DECLARE_LOCALS;
   ps->state_set(PG_STATE_BACKFILL_WAIT);
-  cancel_backfill();
+  suspend_backfill();
   if (ps->needs_backfill()) {
     return transit<WaitLocalBackfillReserved>();
   } else {
diff --git a/src/osd/PeeringState.h b/src/osd/PeeringState.h
@@ -417,7 +417,7 @@ class PeeringState : public MissingLoc::MappingInfo {
 
     // ============ recovery reservation notifications ==========
     virtual void on_backfill_reserved() = 0;
-    virtual void on_backfill_canceled() = 0;
+    virtual void on_backfill_suspended() = 0;
     virtual void on_recovery_reserved() = 0;
     virtual void on_recovery_cancelled() = 0;
 
@@ -963,7 +963,7 @@ class PeeringState : public MissingLoc::MappingInfo {
     boost::statechart::result react(const RemoteReservationRevoked& evt);
     boost::statechart::result react(const DeferBackfill& evt);
     boost::statechart::result react(const UnfoundBackfill& evt);
-    void cancel_backfill();
+    void suspend_backfill();
     void exit();
   };
 
diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc
@@ -4495,7 +4495,7 @@ void PrimaryLogPG::do_scan(
     {
       auto dpp = get_dpp();
       if (osd->check_backfill_full(dpp)) {
-	dout(1) << __func__ << ": Canceling backfill: Full." << dendl;
+	dout(1) << __func__ << ": Suspending backfill: Full." << dendl;
 	queue_peering_event(
 	  PGPeeringEventRef(
 	    std::make_shared<PGPeeringEvent>(
@@ -4552,7 +4552,7 @@ void PrimaryLogPG::do_scan(
       } else {
 	// we canceled backfill for a while due to a too full, and this
 	// is an extra response from a non-too-full peer
-	dout(20) << __func__ << " canceled backfill (too full?)" << dendl;
+	dout(20) << __func__ << " suspended backfill (too full?)" << dendl;
       }
     }
     break;
diff --git a/src/test/crimson/test_backfill.cc b/src/test/crimson/test_backfill.cc
@@ -193,7 +193,7 @@ class BackfillFixture : public crimson::osd::BackfillState::BackfillListener {
   struct PGFacade;
 
   void cancel() {
-    schedule_event_immediate(crimson::osd::BackfillState::CancelBackfill{});
+    schedule_event_immediate(crimson::osd::BackfillState::SuspendBackfill{});
   }
 
   void resume() {
@@ -476,7 +476,7 @@ TEST(backfill, cancel_resume_middle_of_primaryscan)
 
   EXPECT_CALL(cluster_fixture, backfilled);
   cluster_fixture.cancel();
-  cluster_fixture.next_round2<crimson::osd::BackfillState::CancelBackfill>();
+  cluster_fixture.next_round2<crimson::osd::BackfillState::SuspendBackfill>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::PrimaryScanned>();
   cluster_fixture.resume();
   cluster_fixture.next_round2<crimson::osd::BackfillState::Triggered>();
@@ -508,7 +508,7 @@ TEST(backfill, cancel_resume_middle_of_replicascan1)
   EXPECT_CALL(cluster_fixture, backfilled);
   cluster_fixture.next_round2<crimson::osd::BackfillState::PrimaryScanned>();
   cluster_fixture.cancel();
-  cluster_fixture.next_round2<crimson::osd::BackfillState::CancelBackfill>();
+  cluster_fixture.next_round2<crimson::osd::BackfillState::SuspendBackfill>();
   cluster_fixture.resume();
   cluster_fixture.next_round2<crimson::osd::BackfillState::Triggered>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ReplicaScanned>();
@@ -540,7 +540,7 @@ TEST(backfill, cancel_resume_middle_of_replicascan2)
   cluster_fixture.next_round2<crimson::osd::BackfillState::PrimaryScanned>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ReplicaScanned>();
   cluster_fixture.cancel();
-  cluster_fixture.next_round2<crimson::osd::BackfillState::CancelBackfill>();
+  cluster_fixture.next_round2<crimson::osd::BackfillState::SuspendBackfill>();
   cluster_fixture.resume();
   cluster_fixture.next_round2<crimson::osd::BackfillState::Triggered>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ReplicaScanned>();
@@ -572,7 +572,7 @@ TEST(backfill, cancel_resume_middle_of_push1)
   cluster_fixture.next_round2<crimson::osd::BackfillState::ReplicaScanned>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ReplicaScanned>();
   cluster_fixture.cancel();
-  cluster_fixture.next_round2<crimson::osd::BackfillState::CancelBackfill>();
+  cluster_fixture.next_round2<crimson::osd::BackfillState::SuspendBackfill>();
   cluster_fixture.resume();
   cluster_fixture.next_round2<crimson::osd::BackfillState::Triggered>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ObjectPushed>();
@@ -604,7 +604,7 @@ TEST(backfill, cancel_resume_middle_of_push2)
   cluster_fixture.next_round2<crimson::osd::BackfillState::ReplicaScanned>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ObjectPushed>();
   cluster_fixture.cancel();
-  cluster_fixture.next_round2<crimson::osd::BackfillState::CancelBackfill>();
+  cluster_fixture.next_round2<crimson::osd::BackfillState::SuspendBackfill>();
   cluster_fixture.resume();
   cluster_fixture.next_round2<crimson::osd::BackfillState::Triggered>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ObjectPushed>();
@@ -635,7 +635,7 @@ TEST(backfill, cancel_resume_middle_of_push3)
   cluster_fixture.next_round2<crimson::osd::BackfillState::ReplicaScanned>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ObjectPushed>();
   cluster_fixture.cancel();
-  cluster_fixture.next_round2<crimson::osd::BackfillState::CancelBackfill>();
+  cluster_fixture.next_round2<crimson::osd::BackfillState::SuspendBackfill>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ObjectPushed>();
   cluster_fixture.next_round2<crimson::osd::BackfillState::ObjectPushed>();
   cluster_fixture.resume();

Original file line number	Diff line number	Diff line change
`@@ -427,7 +427,7 @@ BackfillState::PrimaryScanning::react(PrimaryScanned evt)`
`427`	`427`	`}`
`428`	`428`
`429`	`429`	`boost::statechart::result`
`430`		`-BackfillState::PrimaryScanning::react(CancelBackfill evt)`
	`430`	`+BackfillState::PrimaryScanning::react(SuspendBackfill evt)`
`431`	`431`	`{`
`432`	`432`	`LOG_PREFIX(BackfillState::PrimaryScanning::react::SuspendBackfill);`
`433`	`433`	`DEBUGDPP("suspended within PrimaryScanning", pg());`
`@@ -523,7 +523,7 @@ BackfillState::ReplicasScanning::react(ReplicaScanned evt)`
`523`	`523`	`}`
`524`	`524`
`525`	`525`	`boost::statechart::result`
`526`		`-BackfillState::ReplicasScanning::react(CancelBackfill evt)`
	`526`	`+BackfillState::ReplicasScanning::react(SuspendBackfill evt)`
`527`	`527`	`{`
`528`	`528`	`LOG_PREFIX(BackfillState::ReplicasScanning::react::SuspendBackfill);`
`529`	`529`	`DEBUGDPP("suspended within ReplicasScanning", pg());`
`@@ -576,7 +576,7 @@ BackfillState::Waiting::react(ObjectPushed evt)`
`576`	`576`	`}`
`577`	`577`
`578`	`578`	`boost::statechart::result`
`579`		`-BackfillState::Waiting::react(CancelBackfill evt)`
	`579`	`+BackfillState::Waiting::react(SuspendBackfill evt)`
`580`	`580`	`{`
`581`	`581`	`LOG_PREFIX(BackfillState::Waiting::react::SuspendBackfill);`
`582`	`582`	`DEBUGDPP("suspended within Waiting", pg());`
Original file line number	Diff line number	Diff line change
`@@ -434,8 +434,8 @@ class PG : public boost::intrusive_ref_counter<`
`434`	`434`	`void on_backfill_reserved() final {`
`435`	`435`	`recovery_handler->on_backfill_reserved();`
`436`	`436`	`}`
`437`		`- void on_backfill_canceled() final {`
`438`		`- recovery_handler->backfill_cancelled();`
	`437`	`+ void on_backfill_suspended() final {`
	`438`	`+ recovery_handler->backfill_suspended();`
`439`	`439`	`}`
`440`	`440`
`441`	`441`	`void on_recovery_cancelled() final {`
Original file line number	Diff line number	Diff line change
`@@ -641,13 +641,13 @@ void PGRecovery::backfilled()`
`641`	`641`	`PeeringState::Backfilled{});`
`642`	`642`	`}`
`643`	`643`
`644`		`-void PGRecovery::backfill_cancelled()`
	`644`	`+void PGRecovery::backfill_suspended()`
`645`	`645`	`{`
`646`	`646`	`// We are not creating a new BackfillRecovery request here, as we`
`647`	`647`	`// need to cancel the backfill synchronously (before this method returns).`
`648`	`648`	`using BackfillState = crimson::osd::BackfillState;`
`649`	`649`	`backfill_state->process_event(`
`650`		`- BackfillState::CancelBackfill{}.intrusive_from_this());`
	`650`	`+ BackfillState::SuspendBackfill{}.intrusive_from_this());`
`651`	`651`	`}`
`652`	`652`
`653`	`653`	`void PGRecovery::dispatch_backfill_event(`
Original file line number	Diff line number	Diff line change
`@@ -1563,8 +1563,12 @@ void PG::on_backfill_reserved()`
`1563`	`1563`	`queue_recovery();`
`1564`	`1564`	`}`
`1565`	`1565`
`1566`		`-void PG::on_backfill_canceled()`
	`1566`	`+void PG::on_backfill_suspended()`
`1567`	`1567`	`{`
	`1568`	`+ // Scan replies asked before suspending this backfill should be ignored.`
	`1569`	`+ // See PrimaryLogPG::do_scan - case MOSDPGScan::OP_SCAN_DIGEST.`
	`1570`	+ // `waiting_on_backfill` will be re-refilled after the suspended backfill
	`1571`	`+ // is resumed/restarted.`
`1568`	`1572`	`if (!waiting_on_backfill.empty()) {`
`1569`	`1573`	`waiting_on_backfill.clear();`
`1570`	`1574`	`finish_recovery_op(hobject_t::get_max());`
Original file line number	Diff line number	Diff line change
`@@ -4495,7 +4495,7 @@ void PrimaryLogPG::do_scan(`
`4495`	`4495`	`{`
`4496`	`4496`	`auto dpp = get_dpp();`
`4497`	`4497`	`if (osd->check_backfill_full(dpp)) {`
`4498`		`- dout(1) << __func__ << ": Canceling backfill: Full." << dendl;`
	`4498`	`+ dout(1) << __func__ << ": Suspending backfill: Full." << dendl;`
`4499`	`4499`	`queue_peering_event(`
`4500`	`4500`	`PGPeeringEventRef(`
`4501`	`4501`	`std::make_shared<PGPeeringEvent>(`
`@@ -4552,7 +4552,7 @@ void PrimaryLogPG::do_scan(`
`4552`	`4552`	`} else {`
`4553`	`4553`	`// we canceled backfill for a while due to a too full, and this`
`4554`	`4554`	`// is an extra response from a non-too-full peer`
`4555`		`- dout(20) << __func__ << " canceled backfill (too full?)" << dendl;`
	`4555`	`+ dout(20) << __func__ << " suspended backfill (too full?)" << dendl;`
`4556`	`4556`	`}`
`4557`	`4557`	`}`
`4558`	`4558`	`break;`