From 92bbd73c7cbf0fa4acf88fb5becac91271e8ec07 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Sun, 28 Jan 2024 05:02:31 -0600 Subject: [PATCH] osd/scrub: remove remote reservation code from ScrubResources Signed-off-by: Ronen Friedman --- src/osd/scrubber/osd_scrub.cc | 15 -------- src/osd/scrubber/osd_scrub.h | 3 -- src/osd/scrubber/scrub_resources.cc | 54 +++-------------------------- src/osd/scrubber/scrub_resources.h | 44 ++--------------------- 4 files changed, 6 insertions(+), 110 deletions(-) diff --git a/src/osd/scrubber/osd_scrub.cc b/src/osd/scrubber/osd_scrub.cc index 2ff70dee56d..b4e1ec8cef2 100644 --- a/src/osd/scrubber/osd_scrub.cc +++ b/src/osd/scrubber/osd_scrub.cc @@ -468,21 +468,6 @@ void OsdScrub::dec_scrubs_local() m_resource_bookkeeper.dec_scrubs_local(); } -bool OsdScrub::inc_scrubs_remote(pg_t pgid) -{ - return m_resource_bookkeeper.inc_scrubs_remote(pgid); -} - -void OsdScrub::enqueue_remote_reservation(pg_t pgid) -{ - m_resource_bookkeeper.enqueue_remote_reservation(pgid); -} - -void OsdScrub::dec_scrubs_remote(pg_t pgid) -{ - m_resource_bookkeeper.dec_scrubs_remote(pgid); -} - void OsdScrub::mark_pg_scrub_blocked(spg_t blocked_pg) { m_queue.mark_pg_scrub_blocked(blocked_pg); diff --git a/src/osd/scrubber/osd_scrub.h b/src/osd/scrubber/osd_scrub.h index 2701a762f79..9aca86ad798 100644 --- a/src/osd/scrubber/osd_scrub.h +++ b/src/osd/scrubber/osd_scrub.h @@ -69,9 +69,6 @@ class OsdScrub { std::unique_ptr inc_scrubs_local( bool is_high_priority); void dec_scrubs_local(); - bool inc_scrubs_remote(pg_t pgid); - void enqueue_remote_reservation(pg_t pgid); - void dec_scrubs_remote(pg_t pgid); // counting the number of PGs stuck while scrubbing, waiting for objects void mark_pg_scrub_blocked(spg_t blocked_pg); diff --git a/src/osd/scrubber/scrub_resources.cc b/src/osd/scrubber/scrub_resources.cc index a69c6f06aca..c47048c2a02 100644 --- a/src/osd/scrubber/scrub_resources.cc +++ b/src/osd/scrubber/scrub_resources.cc @@ -40,8 +40,8 @@ std::unique_ptr ScrubResources::inc_scrubs_local( if (is_high_priority || can_inc_local_scrubs_unlocked()) { ++scrubs_local; log_upwards(fmt::format( - "{}: {} -> {} (max {}, remote {})", __func__, (scrubs_local - 1), - scrubs_local, conf->osd_max_scrubs, granted_reservations.size())); + "{}: {} -> {} (max {})", __func__, (scrubs_local - 1), scrubs_local, + conf->osd_max_scrubs)); return std::make_unique(*this); } return nullptr; @@ -62,63 +62,17 @@ void ScrubResources::dec_scrubs_local() { std::lock_guard lck{resource_lock}; log_upwards(fmt::format( - "{}: {} -> {} (max {}, remote {})", - __func__, scrubs_local, (scrubs_local - 1), conf->osd_max_scrubs, - granted_reservations.size())); + "{}: {} -> {} (max {})", __func__, scrubs_local, (scrubs_local - 1), + conf->osd_max_scrubs)); --scrubs_local; ceph_assert(scrubs_local >= 0); } -// ------------------------- scrubbing on this OSD as replicas ---------------- - -bool ScrubResources::inc_scrubs_remote(pg_t pgid) -{ - std::lock_guard lck{resource_lock}; - - // if this PG is already reserved - it's probably a benign bug. - // report it, but do not fail the reservation. - if (granted_reservations.contains(pgid)) { - log_upwards(fmt::format("{}: pg[{}] already reserved", __func__, pgid)); - return true; - } - - auto pre_op_cnt = granted_reservations.size(); - if (std::cmp_less(pre_op_cnt, conf->osd_max_scrubs)) { - granted_reservations.insert(pgid); - log_upwards(fmt::format( - "{}: pg[{}] reserved. Remote scrubs count changed from {} -> {} (max " - "{}, local {})", - __func__, pgid, pre_op_cnt, granted_reservations.size(), - conf->osd_max_scrubs, scrubs_local)); - return true; - } - - log_upwards(fmt::format( - "{}: pg[{}] failed. Too many concurrent replica scrubs ({} >= max ({}))", - __func__, pgid, pre_op_cnt, conf->osd_max_scrubs)); - return false; -} - -void ScrubResources::dec_scrubs_remote(pg_t pgid) -{ - std::lock_guard lck{resource_lock}; - // we might not have this PG in the set (e.g. if we are concluding a - // high priority scrub, one that does not require reservations) - auto cnt = granted_reservations.erase(pgid); - if (cnt) { - log_upwards(fmt::format( - "{}: remote reservation for {} removed -> {} (max {}, local {})", - __func__, pgid, granted_reservations.size(), conf->osd_max_scrubs, - scrubs_local)); - } -} void ScrubResources::dump_scrub_reservations(ceph::Formatter* f) const { std::lock_guard lck{resource_lock}; f->dump_int("scrubs_local", scrubs_local); - f->dump_int("granted_reservations", granted_reservations.size()); - f->dump_string("PGs being served", fmt::format("{}", granted_reservations)); f->dump_int("osd_max_scrubs", conf->osd_max_scrubs); } diff --git a/src/osd/scrubber/scrub_resources.h b/src/osd/scrubber/scrub_resources.h index afb154e3e16..0aaa2f4bd0f 100644 --- a/src/osd/scrubber/scrub_resources.h +++ b/src/osd/scrubber/scrub_resources.h @@ -10,33 +10,6 @@ #include "common/Formatter.h" #include "osd/osd_types.h" -/* - * AsyncReserver for scrub 'remote' reservations - * ----------------------------------------------- - * - * On the replica side, all reservations are treated as having the same priority. - * Note that 'high priority' scrubs, e.g. user-initiated scrubs, are not required - * to perform any reservations, and are never handled by the replicas' OSD. - * - * A queued scrub reservation request is cancelled by any of the following events: - * - * - a new interval: in this case, we do not expect to see a cancellation request - * from the primary, and we can simply remove the request from the queue; - * - * - a cancellation request from the primary: probably a result of timing out on - * the reservation process. Here, we can simply remove the request from the queue. - * - * - a new reservation request for the same PG: which means we had missed the - * previous cancellation request. We cancel the previous request, and replace - * it with the new one. We would also issue an error log message. - * - * Primary/Replica with differing versions: - * - * The updated version of MOSDScrubReserve contains a new 'OK to queue' field. - * For legacy Primary OSDs, this field is decoded as 'false', and the replica - * responds immediately, with grant/rejection. -*/ - namespace Scrub { /** @@ -50,8 +23,8 @@ class LocalResourceWrapper; /** * The number of concurrent scrub operations performed on an OSD is limited * by a configuration parameter. The 'ScrubResources' class is responsible for - * maintaining a count of the number of scrubs currently performed, both - * acting as primary and acting as a replica, and for enforcing the limit. + * maintaining a count of the number of scrubs currently performed by primary + * PGs on this OSD, and for enforcing the limit. */ class ScrubResources { friend class LocalResourceWrapper; @@ -65,10 +38,6 @@ class ScrubResources { */ int scrubs_local{0}; - /// the set of PGs that have active scrub reservations as replicas - /// \todo come C++23 - consider std::flat_set - std::set granted_reservations; - mutable ceph::mutex resource_lock = ceph::make_mutex("ScrubQueue::resource_lock"); @@ -97,15 +66,6 @@ class ScrubResources { /// decrements the number of scrubs acting as a Primary void dec_scrubs_local(); - /// increments the number of scrubs acting as a Replica - bool inc_scrubs_remote(pg_t pgid); - - /// queue a request with the scrub reserver - void enqueue_remote_reservation(pg_t pgid) {} - - /// decrements the number of scrubs acting as a Replica - void dec_scrubs_remote(pg_t pgid); - void dump_scrub_reservations(ceph::Formatter* f) const; }; -- 2.39.5