From eb22a6ff352571474cb8012782dfa6067cde07e6 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Fri, 28 Mar 2025 11:16:52 -0500 Subject: [PATCH] osd/scrub: decoupling the FSM from the scrubber The scrubber FSM has always been accessing the PgScrubber object via an abstract interface (ScrubMachineListener). But the reverse was not true: the PgScrubber owned a unique_ptr directly to an instance of Scrubber::ScrubMachine. This PR introduces a new interface, ScrubFsmIf, that is implemented by the ScrubMachine. This simplifies unit-testing the PgScrubber, as we can now inject a mock implementation of the FSM interface. Signed-off-by: Ronen Friedman --- src/osd/scrubber/pg_scrubber.h | 3 +- src/osd/scrubber/scrub_machine.h | 24 ++++++++---- src/osd/scrubber/scrub_machine_if.h | 59 +++++++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 src/osd/scrubber/scrub_machine_if.h diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index d6c686e3ac4f5..f148101559f55 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -84,6 +84,7 @@ Main Scrubber interfaces: #include "osd_scrub_sched.h" #include "scrub_backend.h" #include "scrub_machine_lstnr.h" +#include "scrub_machine_if.h" #include "scrub_reservations.h" namespace Scrub { @@ -693,7 +694,7 @@ class PgScrubber : public ScrubPgIF, hobject_t end, bool deep); - std::unique_ptr m_fsm; + std::unique_ptr m_fsm; /// the FSM state, as a string for logging const char* m_fsm_state_name{nullptr}; const spg_t m_pg_id; ///< a local copy of m_pg->pg_id diff --git a/src/osd/scrubber/scrub_machine.h b/src/osd/scrubber/scrub_machine.h index 5ae895d86df89..60a3eea5ab50a 100644 --- a/src/osd/scrubber/scrub_machine.h +++ b/src/osd/scrubber/scrub_machine.h @@ -2,6 +2,7 @@ // vim: ts=8 sw=2 smarttab #pragma once +#include #include #include @@ -23,6 +24,7 @@ #include "messages/MOSDRepScrubMap.h" #include "osd/scrubber_common.h" +#include "scrub_machine_if.h" #include "scrub_machine_lstnr.h" #include "scrub_reservations.h" @@ -278,28 +280,34 @@ struct ReplicaWaitUpdates; struct ReplicaBuildingMap; -class ScrubMachine : public sc::state_machine { +class ScrubMachine : public ScrubFsmIf, public sc::state_machine { public: friend class PgScrubber; public: explicit ScrubMachine(PG* pg, ScrubMachineListener* pg_scrub); - ~ScrubMachine(); + virtual ~ScrubMachine(); spg_t m_pg_id; ScrubMachineListener* m_scrbr; std::ostream& gen_prefix(std::ostream& out) const; - void assert_not_in_session() const; - [[nodiscard]] bool is_reserving() const; - [[nodiscard]] bool is_accepting_updates() const; - [[nodiscard]] bool is_primary_idle() const; + void assert_not_in_session() const final; + [[nodiscard]] bool is_reserving() const final; + [[nodiscard]] bool is_accepting_updates() const final; + [[nodiscard]] bool is_primary_idle() const final; /// elapsed time for the currently active scrub.session - ceph::timespan get_time_scrubbing() const; + ceph::timespan get_time_scrubbing() const final; /// replica reservation process status - std::optional get_reservation_status() const; + std::optional get_reservation_status() const final; + + void initiate() final { sc::state_machine::initiate(); } + + void process_event(const boost::statechart::event_base& evt) final { + sc::state_machine::process_event(evt); + } // ///////////////// aux declarations & functions //////////////////////// // diff --git a/src/osd/scrubber/scrub_machine_if.h b/src/osd/scrubber/scrub_machine_if.h new file mode 100644 index 0000000000000..c49ac3cf9b5f0 --- /dev/null +++ b/src/osd/scrubber/scrub_machine_if.h @@ -0,0 +1,59 @@ +// -*- mode:C++; tab-width:8; c-basic-offset:2; indent-tabs-mode:t -*- +// vim: ts=8 sw=2 smarttab + +#pragma once +/** + * \file the FSM API used by the scrubber + */ +#include +#include +#include +#include +#include + +#include "osd/scrubber_common.h" + +namespace Scrub { + +namespace sc = ::boost::statechart; + + +/// the FSM API used by the scrubber +class ScrubFsmIf { + public: + virtual ~ScrubFsmIf() = default; + + virtual void process_event(const sc::event_base& evt) = 0; + + /// 'true' if the FSM state is PrimaryActive/PrimaryIdle + [[nodiscard]] virtual bool is_primary_idle() const = 0; + + /// 'true' if the FSM state is PrimaryActive/Session/ReservingReplicas + [[nodiscard]] virtual bool is_reserving() const = 0; + + /// 'true' if the FSM state is PrimaryActive/Session/Act/WaitLastUpdate + [[nodiscard]] virtual bool is_accepting_updates() const = 0; + + /// verify state is not any substate of PrimaryActive/Session + virtual void assert_not_in_session() const = 0; + + /** + * time passed since entering the current scrubbing session. + * Specifically - since the Session ctor has completed. + */ + virtual ceph::timespan get_time_scrubbing() const = 0; + + /** + * if we are in the ReservingReplicas state - fetch the reservation status. + * The returned data names the last request sent to the replicas, and + * how many replicas responded / are yet expected to respond. + */ + virtual std::optional get_reservation_status() + const = 0; + + /// "initiate" the state machine (an internal state_chart function) + virtual void initiate() = 0; +}; + +} // namespace Scrub + -- 2.39.5