osd/scrub: add a "clean primary" base state

... to the scrubber state machine.

Similar to ReplicaActive, this state is entered after the peering
is concluded and the PG is set to be Primary, active & clean.

Signed-off-by: Ronen Friedman <[email protected]>

Author: ronen-fr

src/osd/PG.cc

@@ -1861,6 +1861,16 @@ void PG::on_active_exit()
   agent_stop();
 }
 
+Context* PG::on_clean()
+{
+  if (is_active()) {
+    kick_snap_trim();
+  }
+  m_scrubber->on_primary_active_clean();
+  requeue_ops(waiting_for_clean_to_primary_repair);
+  return finish_recovery();
+}
+
 void PG::on_active_advmap(const OSDMapRef &osdmap)
 {
   const auto& new_removed_snaps = osdmap->get_new_removed_snaps();

src/osd/PG.h

@@ -608,13 +608,7 @@ class PG : public DoutPrefixProvider,
 
   void on_active_exit() override;
 
-  Context *on_clean() override {
-    if (is_active()) {
-      kick_snap_trim();
-    }
-    requeue_ops(waiting_for_clean_to_primary_repair);
-    return finish_recovery();
-  }
+  Context *on_clean() override;
 
   void on_activate(interval_set<snapid_t> snaps) override;
 

src/osd/scrubber/pg_scrubber.cc

@@ -152,7 +152,7 @@ bool PgScrubber::verify_against_abort(epoch_t epoch_to_verify)
 
   // if we were not aware of the abort before - kill the scrub.
   if (epoch_to_verify >= m_last_aborted) {
-    scrub_clear_state();
+    m_fsm->process_event(FullReset{});
     m_last_aborted = std::max(epoch_to_verify, m_epoch_start);
   }
   return false;
@@ -407,7 +407,7 @@ void PgScrubber::reset_epoch(epoch_t epoch_queued)
 {
   dout(10) << __func__ << " state deep? " << state_test(PG_STATE_DEEP_SCRUB)
 	   << dendl;
-  m_fsm->assert_not_active();
+  m_fsm->assert_not_in_session();
 
   m_epoch_start = epoch_queued;
   ceph_assert(m_is_deep == state_test(PG_STATE_DEEP_SCRUB));
@@ -462,6 +462,11 @@ void PgScrubber::on_new_interval()
 	   << dendl;
 
   m_fsm->process_event(IntervalChanged{});
+  // the following asserts were added due to a past bug, where PG flags were
+  // left set in some scenarios.
+  ceph_assert(!is_queued_or_active());
+  ceph_assert(!state_test(PG_STATE_SCRUBBING));
+  ceph_assert(!state_test(PG_STATE_DEEP_SCRUB));
   rm_from_osd_scrubbing();
 }
 
@@ -517,6 +522,14 @@ void PgScrubber::on_pg_activate(const requested_scrub_t& request_flags)
 	   << dendl;
 }
 
+void PgScrubber::on_primary_active_clean()
+{
+  dout(10) << fmt::format(
+		  "{}: reg state: {}", __func__, m_scrub_job->state_desc())
+	   << dendl;
+  m_fsm->process_event(PrimaryActivate{});
+}
+
 /*
  * A note re the call to publish_stats_to_osd() below:
  * - we are called from either request_rescrubbing() or scrub_requested().

src/osd/scrubber/pg_scrubber.h

@@ -314,6 +314,8 @@ class PgScrubber : public ScrubPgIF,
 
   void on_new_interval() final;
 
+  void on_primary_active_clean() final;
+
   void on_replica_activate() final;
 
   void scrub_clear_state() final;

src/osd/scrubber/scrub_machine.cc

@@ -47,9 +47,9 @@ void on_event_discard(std::string_view nm)
   dout(20) << " event: --^^^^---- " << nm << dendl;
 }
 
-void ScrubMachine::assert_not_active() const
+void ScrubMachine::assert_not_in_session() const
 {
-  ceph_assert(state_cast<const NotActive*>());
+  ceph_assert(!state_cast<const Session*>());
 }
 
 bool ScrubMachine::is_reserving() const
@@ -114,27 +114,64 @@ NotActive::NotActive(my_context ctx)
   scrbr->clear_queued_or_active();
 }
 
-sc::result NotActive::react(const StartScrub&)
+
+// ----------------------- PrimaryActive --------------------------------
+
+PrimaryActive::PrimaryActive(my_context ctx)
+    : my_base(ctx)
+    , NamedSimply(context<ScrubMachine>().m_scrbr, "PrimaryActive")
+{
+  DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
+  dout(10) << "-- state -->> PrimaryActive" << dendl;
+}
+
+PrimaryActive::~PrimaryActive()
+{
+  DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
+  // we may have set some PG state flags without reaching Session.
+  // And we may be holding a 'local resource'.
+  scrbr->clear_pgscrub_state();
+  scrbr->rm_from_osd_scrubbing();
+}
+
+
+// ---------------- PrimaryActive/PrimaryIdle ---------------------------
+
+PrimaryIdle::PrimaryIdle(my_context ctx)
+    : my_base(ctx)
+    , NamedSimply(context<ScrubMachine>().m_scrbr, "PrimaryActive/PrimaryIdle")
+{
+  dout(10) << "-- state -->> PrimaryActive/PrimaryIdle" << dendl;
+}
+
+sc::result PrimaryIdle::react(const StartScrub&)
 {
-  dout(10) << "NotActive::react(const StartScrub&)" << dendl;
+  dout(10) << "PrimaryIdle::react(const StartScrub&)" << dendl;
   DECLARE_LOCALS;
   return transit<ReservingReplicas>();
 }
 
-sc::result NotActive::react(const AfterRepairScrub&)
+sc::result PrimaryIdle::react(const AfterRepairScrub&)
 {
-  dout(10) << "NotActive::react(const AfterRepairScrub&)" << dendl;
+  dout(10) << "PrimaryIdle::react(const AfterRepairScrub&)" << dendl;
   DECLARE_LOCALS;
   return transit<ReservingReplicas>();
 }
 
+void PrimaryIdle::clear_state(const FullReset&) {
+  dout(10) << "PrimaryIdle::react(const FullReset&): clearing state flags"
+           << dendl;
+  DECLARE_LOCALS;
+  scrbr->clear_pgscrub_state();
+}
+
 // ----------------------- Session -----------------------------------------
 
 Session::Session(my_context ctx)
     : my_base(ctx)
-    , NamedSimply(context<ScrubMachine>().m_scrbr, "Session")
+    , NamedSimply(context<ScrubMachine>().m_scrbr, "PrimaryActive/Session")
 {
-  dout(10) << "-- state -->> Session" << dendl;
+  dout(10) << "-- state -->> PrimaryActive/Session" << dendl;
   DECLARE_LOCALS;  // 'scrbr' & 'pg_id' aliases
 
   // while we've checked the 'someone is reserving' flag before queueing
@@ -242,7 +279,7 @@ sc::result ReservingReplicas::react(const ReplicaReject& ev)
   scrbr->flag_reservations_failure();
 
   // 'Session' state dtor stops the scrubber
-  return transit<NotActive>();
+  return transit<PrimaryIdle>();
 }
 
 sc::result ReservingReplicas::react(const ReservationTimeout&)
@@ -261,7 +298,7 @@ sc::result ReservingReplicas::react(const ReservationTimeout&)
   // cause the scrubber to stop the scrub session, marking 'reservation
   // failure' as the cause (affecting future scheduling)
   scrbr->flag_reservations_failure();
-  return transit<NotActive>();
+  return transit<PrimaryIdle>();
 }
 
 // ----------------------- ActiveScrubbing -----------------------------------
@@ -678,7 +715,7 @@ sc::result WaitDigestUpdate::react(const ScrubFinished&)
   session.m_session_started_at = ScrubTimePoint{};
 
   scrbr->scrub_finish();
-  return transit<NotActive>();
+  return transit<PrimaryIdle>();
 }
 
 ScrubMachine::ScrubMachine(PG* pg, ScrubMachineListener* pg_scrub)
@@ -813,6 +850,11 @@ ReplicaIdle::ReplicaIdle(my_context ctx)
   dout(10) << "-- state -->> ReplicaActive/ReplicaIdle" << dendl;
 }
 
+void ReplicaIdle::reset_ignored(const FullReset&)
+{
+  dout(10) << "ReplicaIdle::react(const FullReset&): FullReset ignored"
+	   << dendl;
+}
 
 // ------------- ReplicaActive/ReplicaActiveOp --------------------------