From 48f2c21d4b4ed8e94885af0d9fd99d627faca11d Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Tue, 30 Jul 2024 07:12:54 -0500 Subject: [PATCH] osd/scrub: remove non-display usage of target's is_high_priority() Signed-off-by: Ronen Friedman --- src/osd/scrubber/osd_scrub.cc | 35 +++++++++++++++++++++++++++- src/osd/scrubber/osd_scrub.h | 5 ++++ src/osd/scrubber/osd_scrub_sched.cc | 12 ++++------ src/osd/scrubber/osd_scrub_sched.h | 5 ++++ src/osd/scrubber/pg_scrubber.cc | 35 +++++++++++----------------- src/osd/scrubber/pg_scrubber.h | 2 +- src/osd/scrubber/scrub_job.cc | 29 +++++++++++++++++++---- src/osd/scrubber/scrub_job.h | 25 ++++++++++++++++++-- src/osd/scrubber/scrub_queue_entry.h | 7 ------ 9 files changed, 111 insertions(+), 44 deletions(-) diff --git a/src/osd/scrubber/osd_scrub.cc b/src/osd/scrubber/osd_scrub.cc index 783973f78f9f6..dfba0ee56f0b5 100644 --- a/src/osd/scrubber/osd_scrub.cc +++ b/src/osd/scrubber/osd_scrub.cc @@ -125,7 +125,8 @@ void OsdScrub::initiate_scrub(bool is_recovery_active) debug_log_all_jobs(); } - auto candidate = m_queue.pop_ready_entry(env_restrictions, scrub_time); + auto candidate = m_queue.pop_ready_entry( + is_sched_target_eligible, env_restrictions, scrub_time); if (!candidate) { dout(20) << "no PGs are ready for scrubbing" << dendl; return; @@ -146,6 +147,38 @@ void OsdScrub::initiate_scrub(bool is_recovery_active) } +/** + * + * Note: only checking those conditions that are frequent, and should not cause + * a queue reshuffle. + */ +bool OsdScrub::is_sched_target_eligible( + const Scrub::SchedEntry& e, + const Scrub::OSDRestrictions& r, + utime_t time_now) +{ + using ScrubJob = Scrub::ScrubJob; + if (e.schedule.not_before > time_now) { + return false; + } + if (r.max_concurrency_reached && + ScrubJob::observes_max_concurrency(e.urgency)) { + return false; + } + if (!r.load_is_low && ScrubJob::observes_random_backoff(e.urgency)) { + return false; + } + if (!r.time_permit && ScrubJob::observes_allowed_hours(e.urgency)) { + return false; + } + if (!r.load_is_low && ScrubJob::observes_load_limit(e.urgency)) { + return false; + } + // recovery? + return true; +} + + Scrub::OSDRestrictions OsdScrub::restrictions_on_scrubbing( bool is_recovery_active, utime_t scrub_clock_now) const diff --git a/src/osd/scrubber/osd_scrub.h b/src/osd/scrubber/osd_scrub.h index 481f53581be23..a63f4ac505a40 100644 --- a/src/osd/scrubber/osd_scrub.h +++ b/src/osd/scrubber/osd_scrub.h @@ -153,6 +153,11 @@ class OsdScrub { bool is_recovery_active, utime_t scrub_clock_now) const; + static bool is_sched_target_eligible( + const Scrub::SchedEntry& e, + const Scrub::OSDRestrictions& r, + utime_t time_now); + /** * initiate a scrub on a specific PG * The PG is locked, enabling us to query its state. Specifically, we diff --git a/src/osd/scrubber/osd_scrub_sched.cc b/src/osd/scrubber/osd_scrub_sched.cc index 777843c137094..8ff0d1ff7d8cf 100644 --- a/src/osd/scrubber/osd_scrub_sched.cc +++ b/src/osd/scrubber/osd_scrub_sched.cc @@ -82,17 +82,15 @@ void ScrubQueue::dequeue_target(spg_t pgid, scrub_level_t s_or_d) std::optional ScrubQueue::pop_ready_entry( + EligibilityPred eligibility_pred, OSDRestrictions restrictions, utime_t time_now) { - auto eligible_filtr = [time_now, rst = restrictions]( + /// \todo must handle 'only_deadlined'! + + auto eligible_filtr = [&, rst = restrictions]( const SchedEntry& e) -> bool { - return (e.schedule.not_before <= time_now) && - (e.is_high_priority() || - (!rst.high_priority_only && - (!rst.only_deadlined || - (!e.schedule.deadline.is_zero() && - e.schedule.deadline <= time_now)))); + return eligibility_pred(e, rst, time_now); }; std::unique_lock lck{jobs_lock}; diff --git a/src/osd/scrubber/osd_scrub_sched.h b/src/osd/scrubber/osd_scrub_sched.h index fa0fa542dfcfe..c30532ce0d934 100644 --- a/src/osd/scrubber/osd_scrub_sched.h +++ b/src/osd/scrubber/osd_scrub_sched.h @@ -171,6 +171,10 @@ class ScrubQueue { using EntryPred = std::function; + /// a predicate to check entries against some common temporary restrictions + using EligibilityPred = std::function< + bool(const Scrub::SchedEntry&, const Scrub::OSDRestrictions&, utime_t)>; + /** * the set of all PGs named by the entries in the queue (but only those * entries that satisfy the predicate) @@ -212,6 +216,7 @@ class ScrubQueue { * nullopt is returned if no such entry exists. */ std::optional pop_ready_entry( + EligibilityPred eligibility_pred, Scrub::OSDRestrictions restrictions, utime_t time_now); diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index 9530cff7d7a78..7a37d8f23a397 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -2382,45 +2382,38 @@ void PgScrubber::dump_scrubber(ceph::Formatter* f, if (m_active_target) { f->dump_bool("active", true); - dump_active_scrubber(f, state_test(PG_STATE_DEEP_SCRUB)); + dump_active_scrubber(f); } else { f->dump_bool("active", false); - f->dump_bool("must_scrub", - (m_planned_scrub.must_scrub || m_flags.required)); - f->dump_bool("must_deep_scrub", request_flags.must_deep_scrub); + const auto now_is = ceph_clock_now(); + const auto& earliest = m_scrub_job->earliest_target(now_is); + f->dump_bool("must_scrub", earliest.is_high_priority()); + f->dump_bool("must_deep_scrub", request_flags.must_deep_scrub); // RRR f->dump_bool("must_repair", request_flags.must_repair); f->dump_bool("need_auto", request_flags.need_auto); - f->dump_stream("scrub_reg_stamp") << m_scrub_job->get_sched_time(); - - // note that we are repeating logic that is coded elsewhere (currently - // PG.cc). This is not optimal. - bool deep_expected = - (ceph_clock_now() >= m_pg->next_deepscrub_interval()) || - request_flags.must_deep_scrub || request_flags.need_auto; - auto sched_state = - m_scrub_job->scheduling_state(ceph_clock_now(), deep_expected); + f->dump_stream("scrub_reg_stamp") << earliest.sched_info.schedule.not_before; + auto sched_state = m_scrub_job->scheduling_state(now_is); f->dump_string("schedule", sched_state); } if (m_publish_sessions) { - f->dump_int("test_sequence", - m_sessions_counter); // an ever-increasing number used by tests + // this is a test-only feature. It is not expected to be used in production. + // The 'test_sequence' is an ever-increasing number used by tests. + f->dump_int("test_sequence", m_sessions_counter); } f->close_section(); } -void PgScrubber::dump_active_scrubber(ceph::Formatter* f, bool is_deep) const +void PgScrubber::dump_active_scrubber(ceph::Formatter* f) const { f->dump_stream("epoch_start") << m_interval_start; f->dump_stream("start") << m_start; f->dump_stream("end") << m_end; f->dump_stream("max_end") << m_max_end; f->dump_stream("subset_last_update") << m_subset_last_update; - // note that m_is_deep will be set some time after PG_STATE_DEEP_SCRUB is - // asserted. Thus, using the latter. - f->dump_bool("deep", is_deep); + f->dump_bool("deep", m_active_target->is_deep()); // dump the scrub-type flags f->dump_bool("req_scrub", m_flags.required); @@ -2506,7 +2499,7 @@ pg_scrubbing_status_t PgScrubber::get_schedule() const pg_scrub_sched_status_t::queued, false, (targ.is_deep() ? scrub_level_t::deep : scrub_level_t::shallow), - !targ.sched_info.is_high_priority()}; + !targ.is_high_priority()}; } // both targets are not ready yet @@ -2517,7 +2510,7 @@ pg_scrubbing_status_t PgScrubber::get_schedule() const pg_scrub_sched_status_t::scheduled, false, (targ.is_deep() ? scrub_level_t::deep : scrub_level_t::shallow), - !targ.sched_info.is_high_priority()}; + !targ.is_high_priority()}; } diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index f657dcc690cf9..90b7697002365 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -606,7 +606,7 @@ class PgScrubber : public ScrubPgIF, void run_callbacks(); // 'query' command data for an active scrub - void dump_active_scrubber(ceph::Formatter* f, bool is_deep) const; + void dump_active_scrubber(ceph::Formatter* f) const; /** * move the 'not before' to a later time (with a delay amount that is diff --git a/src/osd/scrubber/scrub_job.cc b/src/osd/scrubber/scrub_job.cc index 8a51ea0fbc84e..45a300aaafe68 100644 --- a/src/osd/scrubber/scrub_job.cc +++ b/src/osd/scrubber/scrub_job.cc @@ -219,6 +219,22 @@ const SchedTarget& ScrubJob::earliest_target() const return (compr == std::weak_ordering::less) ? shallow_target : deep_target; } + +SchedTarget& ScrubJob::earliest_target(utime_t scrub_clock_now) +{ + std::weak_ordering compr = cmp_entries(scrub_clock_now, + shallow_target.queued_element(), deep_target.queued_element()); + return (compr == std::weak_ordering::less) ? shallow_target : deep_target; +} + +const SchedTarget& ScrubJob::earliest_target(utime_t scrub_clock_now) const +{ + std::weak_ordering compr = cmp_entries(scrub_clock_now, + shallow_target.queued_element(), deep_target.queued_element()); + return (compr == std::weak_ordering::less) ? shallow_target : deep_target; +} + + utime_t ScrubJob::get_sched_time() const { return earliest_target().sched_info.schedule.not_before; @@ -298,8 +314,7 @@ SchedTarget& ScrubJob::delay_on_failure( } -std::string ScrubJob::scheduling_state(utime_t now_is, bool is_deep_expected) - const +std::string ScrubJob::scheduling_state(utime_t now_is) const { // if not registered, not a candidate for scrubbing on this OSD (or at all) if (!registered) { @@ -314,10 +329,9 @@ std::string ScrubJob::scheduling_state(utime_t now_is, bool is_deep_expected) if (first_ready) { // the target is ready to be scrubbed return fmt::format( - "queued for {}scrub at {:s} (debug RRR: {})", + "queued for {}scrub at {:s}", (first_ready->get().is_deep() ? "deep " : ""), - first_ready->get().sched_info.schedule.scheduled_at, - (is_deep_expected ? "deep " : "")); + first_ready->get().sched_info.schedule.scheduled_at); } else { // both targets are in the future const auto& nearest = earliest_target(); @@ -378,3 +392,8 @@ bool ScrubJob::observes_max_concurrency(urgency_t urgency) { return urgency < urgency_t::operator_requested; } + +bool ScrubJob::observes_random_backoff(urgency_t urgency) +{ + return urgency < urgency_t::after_repair; +} diff --git a/src/osd/scrubber/scrub_job.h b/src/osd/scrubber/scrub_job.h index 014cb0951d937..256e0d2caa178 100644 --- a/src/osd/scrubber/scrub_job.h +++ b/src/osd/scrubber/scrub_job.h @@ -116,6 +116,16 @@ struct SchedTarget { urgency_t urgency() const { return sched_info.urgency; } + /** + * a loose definition of 'high priority' scrubs. Can only be used for + * logs and user messages. Actual scheduling decisions should be based + * on the 'urgency' attribute and its fine-grained characteristics. + */ + bool is_high_priority() const + { + return urgency() != urgency_t::periodic_regular; + } + bool was_delayed() const { return sched_info.last_issue != delay_cause_t::none; } /// provides r/w access to the scheduling sub-object @@ -181,6 +191,16 @@ class ScrubJob { const SchedTarget& earliest_target() const; SchedTarget& earliest_target(); + /** + * the target that will be scrubbed first. Basically - used + * cmp_entries() to determine the order of the two targets. + * Which means: if only one of the targets is eligible, it will be returned. + * If both - the one with the highest priority -> level -> target time. + * Otherwise - the one with the earliest not-before. + */ + const SchedTarget& earliest_target(utime_t scrub_clock_now) const; + SchedTarget& earliest_target(utime_t scrub_clock_now); + /// the not-before of our earliest target (either shallow or deep) utime_t get_sched_time() const; @@ -266,7 +286,7 @@ class ScrubJob { * a text description of the "scheduling intentions" of this PG: * are we already scheduled for a scrub/deep scrub? when? */ - std::string scheduling_state(utime_t now_is, bool is_deep_expected) const; + std::string scheduling_state(utime_t now_is) const; std::ostream& gen_prefix(std::ostream& out, std::string_view fn) const; std::string log_msg_prefix; @@ -341,7 +361,8 @@ class ScrubJob { static bool requires_randomization(urgency_t urgency); static bool observes_max_concurrency(urgency_t urgency); -}; + + static bool observes_random_backoff(urgency_t urgency);}; } // namespace Scrub namespace std { diff --git a/src/osd/scrubber/scrub_queue_entry.h b/src/osd/scrubber/scrub_queue_entry.h index 9ab8affc601af..f6d3d51a8b97f 100644 --- a/src/osd/scrubber/scrub_queue_entry.h +++ b/src/osd/scrubber/scrub_queue_entry.h @@ -85,13 +85,6 @@ struct SchedEntry { /// either 'none', or the reason for the latest failure/delay (for /// logging/reporting purposes) delay_cause_t last_issue{delay_cause_t::none}; - - // note: is_high_priority() is temporary. Will be removed - // in a followup commit. - bool is_high_priority() const - { - return urgency != urgency_t::periodic_regular; - } }; -- 2.39.5