From 8b5351f434fe7095922a6815f5187635cd5b21b1 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Tue, 4 Jun 2024 04:02:55 -0500 Subject: [PATCH] osd/scrub: do not track reserving state at OSD level As we no longer block the initiation of new scrub sessions for an OSD for which any of its PGs is in the process of reserving scrub resources, there is no need to track the reserving state at the OSD level. Signed-off-by: Ronen Friedman (cherry picked from commit 1deac158036fffa8c2745f27feffcceccb889c27) --- src/osd/scrubber/osd_scrub.cc | 10 ------- src/osd/scrubber/osd_scrub.h | 9 ------- src/osd/scrubber/osd_scrub_sched.cc | 25 ------------------ src/osd/scrubber/osd_scrub_sched.h | 36 -------------------------- src/osd/scrubber/pg_scrubber.cc | 11 -------- src/osd/scrubber/pg_scrubber.h | 3 --- src/osd/scrubber/scrub_machine.cc | 17 ------------ src/osd/scrubber/scrub_machine.h | 2 +- src/osd/scrubber/scrub_machine_lstnr.h | 12 --------- 9 files changed, 1 insertion(+), 124 deletions(-) diff --git a/src/osd/scrubber/osd_scrub.cc b/src/osd/scrubber/osd_scrub.cc index 2b67d145a2ae3..13cc4e80fb188 100644 --- a/src/osd/scrubber/osd_scrub.cc +++ b/src/osd/scrubber/osd_scrub.cc @@ -476,13 +476,3 @@ int OsdScrub::get_blocked_pgs_count() const { return m_queue.get_blocked_pgs_count(); } - -bool OsdScrub::set_reserving_now(spg_t reserving_id, utime_t now_is) -{ - return m_queue.set_reserving_now(reserving_id, now_is); -} - -void OsdScrub::clear_reserving_now(spg_t reserving_id) -{ - m_queue.clear_reserving_now(reserving_id); -} diff --git a/src/osd/scrubber/osd_scrub.h b/src/osd/scrubber/osd_scrub.h index cd1158d472366..41f5122681c23 100644 --- a/src/osd/scrubber/osd_scrub.h +++ b/src/osd/scrubber/osd_scrub.h @@ -134,15 +134,6 @@ class OsdScrub { utime_t t, bool high_priority_scrub) const; - /** - * No new scrub session will start while a scrub was initiated on a PG, - * and that PG is trying to acquire replica resources. - * \retval false if the flag was already set (due to a race) - */ - bool set_reserving_now(spg_t reserving_id, utime_t now_is); - - void clear_reserving_now(spg_t reserving_id); - /** * push the 'not_before' time out by 'delay' seconds, so that this scrub target * would not be retried before 'delay' seconds have passed. diff --git a/src/osd/scrubber/osd_scrub_sched.cc b/src/osd/scrubber/osd_scrub_sched.cc index da355ec692dac..079e2a7e7aed6 100644 --- a/src/osd/scrubber/osd_scrub_sched.cc +++ b/src/osd/scrubber/osd_scrub_sched.cc @@ -361,28 +361,3 @@ int ScrubQueue::get_blocked_pgs_count() const { return blocked_scrubs_cnt; } - -// ////////////////////////////////////////////////////////////////////////// // -// ScrubQueue - maintaining the 'some PG is reserving' flag - -bool ScrubQueue::set_reserving_now(spg_t reserving_id, utime_t now_is) -{ - std::unique_lock l{reserving_lock}; - - if (!reserving_pg.has_value()) { - reserving_pg = reserving_id; - reserving_since = now_is; - return true; - } - ceph_assert(reserving_id != *reserving_pg); - return false; -} - -void ScrubQueue::clear_reserving_now(spg_t was_reserving_id) -{ - std::unique_lock l{reserving_lock}; - if (reserving_pg && (*reserving_pg == was_reserving_id)) { - reserving_pg.reset(); - } - // otherwise - ignore silently -} diff --git a/src/osd/scrubber/osd_scrub_sched.h b/src/osd/scrubber/osd_scrub_sched.h index 140c1428889ce..49b451e62d26d 100644 --- a/src/osd/scrubber/osd_scrub_sched.h +++ b/src/osd/scrubber/osd_scrub_sched.h @@ -84,7 +84,6 @@ ScrubQueue interfaces (main functions): - can_inc_scrubs() - {inc/dec}_scrubs_{local/remote}() - dump_scrub_reservations() - - {set/clear/is}_reserving_now() <2> - environment conditions: @@ -238,30 +237,6 @@ class ScrubQueue { public: void dump_scrubs(ceph::Formatter* f) const; - /** - * No new scrub session will start while a scrub was initiated on a PG, - * and that PG is trying to acquire replica resources. - * - * \todo replace the atomic bool with a regular bool protected by a - * common OSD-service lock. Or better still - once PR#53263 is merged, - * remove this flag altogether. - */ - - /** - * set_reserving_now() - * \returns 'false' if the flag was already set - * (which is a possible result of a race between the check in OsdScrub and - * the initiation of a scrub by some other PG) - */ - bool set_reserving_now(spg_t reserving_id, utime_t now_is); - - /** - * silently ignore attempts to clear the flag if it was not set by - * the named pg. - */ - void clear_reserving_now(spg_t reserving_id); - bool is_reserving_now() const; - /// counting the number of PGs stuck while scrubbing, waiting for objects void mark_pg_scrub_blocked(spg_t blocked_pg); void clear_pg_scrub_blocked(spg_t blocked_pg); @@ -331,17 +306,6 @@ class ScrubQueue { */ std::atomic_int_fast16_t blocked_scrubs_cnt{0}; - /** - * One of the OSD's primary PGs is in the initial phase of a scrub, - * trying to secure its replicas' resources. We will refrain from initiating - * any other scrub sessions until this one is done. - * - * \todo replace the local lock with regular osd-service locking - */ - ceph::mutex reserving_lock = ceph::make_mutex("ScrubQueue::reserving_lock"); - std::optional reserving_pg; - utime_t reserving_since; - /** * If the scrub job was not explicitly requested, we postpone it by some * random length of time. diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index 9fe7295201d69..65999456d5ba9 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -1743,17 +1743,6 @@ void PgScrubber::handle_scrub_reserve_msgs(OpRequestRef op) } } - -bool PgScrubber::set_reserving_now() { - return m_osds->get_scrub_services().set_reserving_now(m_pg_id, - ceph_clock_now()); -} - -void PgScrubber::clear_reserving_now() -{ - m_osds->get_scrub_services().clear_reserving_now(m_pg_id); -} - void PgScrubber::set_queued_or_active() { m_queued_or_active = true; diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index 78e8ba90d449e..fa05af580782c 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -456,9 +456,6 @@ class PgScrubber : public ScrubPgIF, int build_replica_map_chunk() final; - bool set_reserving_now() final; - void clear_reserving_now() final; - [[nodiscard]] bool was_epoch_changed() const final; void set_queued_or_active() final; diff --git a/src/osd/scrubber/scrub_machine.cc b/src/osd/scrubber/scrub_machine.cc index e2998752522ea..9ad1424877843 100644 --- a/src/osd/scrubber/scrub_machine.cc +++ b/src/osd/scrubber/scrub_machine.cc @@ -183,15 +183,6 @@ Session::Session(my_context ctx) dout(10) << "-- state -->> PrimaryActive/Session" << dendl; DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases - // while we've checked the 'someone is reserving' flag before queueing - // the start-scrub event, it's possible that the flag was set in the meantime. - // Handling this case here requires adding a new sub-state, and the - // complication of reporting a failure to the caller in a new failure - // path. On the other hand - ignoring an ongoing reservation on rare - // occasions will cause no harm. - // We choose ignorance. - std::ignore = scrbr->set_reserving_now(); - m_perf_set = &scrbr->get_counters_set(); m_perf_set->inc(scrbcnt_started); } @@ -242,14 +233,6 @@ ReservingReplicas::ReservingReplicas(my_context ctx) } } -ReservingReplicas::~ReservingReplicas() -{ - DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases - // it's OK to try and clear the flag even if we don't hold it - // (the flag remembers the actual holder) - scrbr->clear_reserving_now(); -} - sc::result ReservingReplicas::react(const ReplicaGrant& ev) { DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases diff --git a/src/osd/scrubber/scrub_machine.h b/src/osd/scrubber/scrub_machine.h index 1bd5d69bc3ed9..cf8d28c765b04 100644 --- a/src/osd/scrubber/scrub_machine.h +++ b/src/osd/scrubber/scrub_machine.h @@ -564,7 +564,7 @@ struct Session : sc::state, struct ReservingReplicas : sc::state, NamedSimply { explicit ReservingReplicas(my_context ctx); - ~ReservingReplicas(); + ~ReservingReplicas() = default; using reactions = mpl::list< sc::custom_reaction, sc::custom_reaction, diff --git a/src/osd/scrubber/scrub_machine_lstnr.h b/src/osd/scrubber/scrub_machine_lstnr.h index ea893ba81f01c..85c518c402f82 100644 --- a/src/osd/scrubber/scrub_machine_lstnr.h +++ b/src/osd/scrubber/scrub_machine_lstnr.h @@ -205,18 +205,6 @@ struct ScrubMachineListener { virtual void set_scrub_duration(std::chrono::milliseconds duration) = 0; - /** - * No new scrub session will start while a scrub was initiate on a PG, - * and that PG is trying to acquire replica resources. - * set_reserving_now()/clear_reserving_now() let's the OSD scrub-queue know - * we are busy reserving. - * - * set_reserving_now() returns 'false' if there already is a PG in the - * reserving stage of the scrub session. - */ - virtual bool set_reserving_now() = 0; - virtual void clear_reserving_now() = 0; - /** * Manipulate the 'I am being scrubbed now' Scrubber's flag */ -- 2.39.5