From: Ronen Friedman Date: Tue, 19 Sep 2023 11:55:25 +0000 (-0500) Subject: osd/scrub: move initiate_a_scrub() to OsdScrub X-Git-Tag: v19.0.0~438^2~5 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=0803780d5ef1088cc4e52a8f3faee68fdfc20bed;p=ceph.git osd/scrub: move initiate_a_scrub() to OsdScrub Scrub initiation is now fully owned by OsdScrub. Signed-off-by: Ronen Friedman --- diff --git a/src/osd/OSD.h b/src/osd/OSD.h index efde588d939eb..4ba3d76811948 100644 --- a/src/osd/OSD.h +++ b/src/osd/OSD.h @@ -245,21 +245,6 @@ public: public: OsdScrub& get_scrub_services() { return m_osd_scrub; } - /** - * A callback used by the ScrubQueue object to initiate a scrub on a specific PG. - * - * The request might fail for multiple reasons, as ScrubQueue cannot by its own - * check some of the PG-specific preconditions and those are checked here. See - * attempt_t definition. - * - * @param pgid to scrub - * @param allow_requested_repair_only - * @return a Scrub::attempt_t detailing either a success, or the failure reason. - */ - Scrub::schedule_result_t initiate_a_scrub( - spg_t pgid, - bool allow_requested_repair_only) final; - /** * locks the named PG, returning an RAII wrapper that unlocks upon * destruction. diff --git a/src/osd/scrubber/osd_scrub.cc b/src/osd/scrubber/osd_scrub.cc index 6e1923b638f15..6c31dce802b4b 100644 --- a/src/osd/scrubber/osd_scrub.cc +++ b/src/osd/scrubber/osd_scrub.cc @@ -132,7 +132,7 @@ void OsdScrub::initiate_scrub(bool is_recovery_active) // we have a candidate to scrub. But we may fail when trying to initiate that // scrub. For some failures - we can continue with the next candidate. For // others - we should stop trying to scrub at this tick. - res = m_osd_svc.initiate_a_scrub( + res = initiate_a_scrub( candidate, env_restrictions->allow_requested_repair_only); switch (res) { case schedule_result_t::scrub_initiated: @@ -224,53 +224,53 @@ std::optional OsdScrub::restrictions_on_scrubbing( } -// ////////////////////////////////////////////////////////////////////////// // -// scrub initiation - OSD code temporarily moved here from OSD.cc - -// temporary dout() support for OSD members: -static ostream& _prefix(std::ostream* _dout, int whoami, epoch_t epoch) { - return *_dout << "osd." << whoami << " " << epoch << " "; -} -#undef dout_prefix -#define dout_prefix _prefix(_dout, whoami, get_osdmap_epoch()) - - -schedule_result_t OSDService::initiate_a_scrub(spg_t pgid, - bool allow_requested_repair_only) +Scrub::schedule_result_t OsdScrub::initiate_a_scrub( + spg_t pgid, + bool allow_requested_repair_only) { - dout(20) << __func__ << " trying " << pgid << dendl; + dout(20) << fmt::format("trying pg[{}]", pgid) << dendl; - // we have a candidate to scrub. We need some PG information to know if scrubbing is - // allowed + // we have a candidate to scrub. We need some PG information to + // know if scrubbing is allowed - PGRef pg = osd->lookup_lock_pg(pgid); - if (!pg) { - // the PG was dequeued in the short timespan between creating the candidates list - // (collect_ripe_jobs()) and here - dout(5) << __func__ << " pg " << pgid << " not found" << dendl; - return schedule_result_t::no_such_pg; + auto locked_pg = m_osd_svc.get_locked_pg(pgid); + if (!locked_pg) { + // the PG was dequeued in the short timespan between creating the + // candidates list (ready_to_scrub()) and here + dout(5) << fmt::format("pg[{}] not found", pgid) << dendl; + return Scrub::schedule_result_t::no_such_pg; } - // This has already started, so go on to the next scrub job - if (pg->is_scrub_queued_or_active()) { - pg->unlock(); - dout(20) << __func__ << ": already in progress pgid " << pgid << dendl; - return schedule_result_t::already_started; + // This one is already scrubbing, so go on to the next scrub job + if (locked_pg->pg()->is_scrub_queued_or_active()) { + dout(10) << fmt::format("pg[{}]: scrub already in progress", pgid) << dendl; + return Scrub::schedule_result_t::already_started; } // Skip other kinds of scrubbing if only explicitly requested repairing is allowed - if (allow_requested_repair_only && !pg->get_planned_scrub().must_repair) { - pg->unlock(); - dout(10) << __func__ << " skip " << pgid - << " because repairing is not explicitly requested on it" << dendl; - return schedule_result_t::preconditions; + if (allow_requested_repair_only && + !locked_pg->pg()->get_planned_scrub().must_repair) { + dout(10) << fmt::format( + "skipping pg[{}] as repairing was not explicitly " + "requested for that pg", + pgid) + << dendl; + return Scrub::schedule_result_t::preconditions; } - auto scrub_attempt = pg->sched_scrub(); - pg->unlock(); - return scrub_attempt; + return locked_pg->pg()->sched_scrub(); } +// ////////////////////////////////////////////////////////////////////////// // +// scrub initiation - OSD code temporarily moved here from OSD.cc + +// temporary dout() support for OSD members: +static ostream& _prefix(std::ostream* _dout, int whoami, epoch_t epoch) { + return *_dout << "osd." << whoami << " " << epoch << " "; +} +#undef dout_prefix +#define dout_prefix _prefix(_dout, whoami, get_osdmap_epoch()) + void OSD::resched_all_scrubs() { dout(10) << __func__ << ": start" << dendl; diff --git a/src/osd/scrubber/osd_scrub.h b/src/osd/scrubber/osd_scrub.h index 701a5d7f25f32..75ad02fd60714 100644 --- a/src/osd/scrubber/osd_scrub.h +++ b/src/osd/scrubber/osd_scrub.h @@ -178,6 +178,19 @@ class OsdScrub { bool is_recovery_active, utime_t scrub_clock_now) const; + /** + * initiate a scrub on a specific PG + * The PG is locked, enabling us to query its state. Specifically, we + * verify that the PG is not already scrubbing, and that + * a possible 'allow requested repair only' condition is not in conflict. + * + * \returns a schedule_result_t object, indicating whether the scrub was + * initiated, and if not - why. + */ + Scrub::schedule_result_t initiate_a_scrub( + spg_t pgid, + bool allow_requested_repair_only); + /// resource reservation management Scrub::ScrubResources m_resource_bookkeeper; diff --git a/src/osd/scrubber/osd_scrub_sched.h b/src/osd/scrubber/osd_scrub_sched.h index e0904a653e9f5..1c73fd6364a90 100644 --- a/src/osd/scrubber/osd_scrub_sched.h +++ b/src/osd/scrubber/osd_scrub_sched.h @@ -6,9 +6,13 @@ /* ┌───────────────────────┐ │ OSD │ -│ OSDService ─┼───┐ -│ │ │ -│ │ │ +│ OSDService │ +│ │ +│ ┌─────────────────────│ +│ │ │ +│ │ OsdScrub │ +│ │ ─┼───┐ +│ │ │ │ └───────────────────────┘ │ Ownes & uses the following │ ScrubQueue interfaces: │ @@ -21,9 +25,6 @@ │ │ │ - │ - │ - │ ScrubQueue │ ┌───────────────────────────▼────────────┐ │ │ @@ -139,21 +140,6 @@ class ScrubSchedListener { */ virtual std::optional get_locked_pg(spg_t pgid) = 0; - /** - * A callback used by the ScrubQueue object to initiate a scrub on a specific - * PG. - * - * The request might fail for multiple reasons, as ScrubQueue cannot by its - * own check some of the PG-specific preconditions and those are checked here. - * See attempt_t definition. - * - * @return a Scrub::attempt_t detailing either a success, or the failure - * reason. - */ - virtual schedule_result_t initiate_a_scrub( - spg_t pgid, - bool allow_requested_repair_only) = 0; - virtual ~ScrubSchedListener() {} }; @@ -177,7 +163,6 @@ class ScrubQueue { friend class TestOSDScrub; friend class ScrubSchedTestWrapper; ///< unit-tests structure - friend class OsdScrub; ///< transitory - fixed in followup commits using sched_params_t = Scrub::sched_params_t; /** diff --git a/src/test/osd/test_scrub_sched.cc b/src/test/osd/test_scrub_sched.cc index 13c72ee6cf1bb..da8fb3bb5e3ec 100644 --- a/src/test/osd/test_scrub_sched.cc +++ b/src/test/osd/test_scrub_sched.cc @@ -99,17 +99,6 @@ class FakeOsd : public Scrub::ScrubSchedListener { int get_nodeid() const final { return m_osd_num; } - schedule_result_t initiate_a_scrub(spg_t pgid, - bool allow_requested_repair_only) final - { - std::ignore = allow_requested_repair_only; - auto res = m_next_response.find(pgid); - if (res == m_next_response.end()) { - return schedule_result_t::no_such_pg; - } - return m_next_response[pgid]; - } - void set_initiation_response(spg_t pgid, schedule_result_t result) { m_next_response[pgid] = result;