From 0385fc4faf9ad984699feafca1265388671989f7 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Thu, 8 Aug 2024 08:49:57 -0500 Subject: [PATCH] osd/scrub: remove requested_scrub_t::deep_scrub_on_error This flag was used to indicate that a deep scrub should be performed if a shallow scrub finds an error. It was always set true for shallow, regular, scrubs - if can_autorepair flag was set. Thus, the ephemeral flag in the requested_scrub_t object is not really needed. Signed-off-by: Ronen Friedman --- src/osd/scrubber/pg_scrubber.cc | 38 +++++++++++++++++++-------------- src/osd/scrubber/pg_scrubber.h | 4 +++- src/osd/scrubber_common.h | 11 +++++----- 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index 7a37d8f23a397..7b58c2cfdfce5 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -67,8 +67,6 @@ ostream& operator<<(ostream& out, const requested_scrub_t& sf) out << " MUST_REPAIR"; if (sf.auto_repair) out << " planned AUTO_REPAIR"; - if (sf.deep_scrub_on_error) - out << " planned DEEP_SCRUB_ON_ERROR"; if (sf.must_deep_scrub) out << " MUST_DEEP_SCRUB"; if (sf.must_scrub) @@ -1698,7 +1696,9 @@ void PgScrubber::replica_scrub_op(OpRequestRef op) cost); } -void PgScrubber::set_op_parameters(const requested_scrub_t& request) +void PgScrubber::set_op_parameters( + ScrubPGPreconds pg_cond, + const requested_scrub_t& request) { dout(10) << fmt::format("{}: @ input: {}", __func__, request) << dendl; @@ -1716,6 +1716,17 @@ void PgScrubber::set_op_parameters(const requested_scrub_t& request) ? get_pg_cct()->_conf->osd_requested_scrub_priority : m_pg->get_scrub_priority(); + // 'deep-on-error' is set for periodic shallow scrubs, if allowed + // by the environment + if (m_active_target->is_shallow() && pg_cond.can_autorepair && + m_active_target->urgency() == urgency_t::periodic_regular) { + m_flags.deep_scrub_on_error = true; + dout(10) << fmt::format( + "{}: auto repair with scrubbing, rescrub if errors found", + __func__) + << dendl; + } + state_set(PG_STATE_SCRUBBING); // will we be deep-scrubbing? @@ -1742,7 +1753,6 @@ void PgScrubber::set_op_parameters(const requested_scrub_t& request) // The publishing here is required for tests synchronization. // The PG state flags were modified. m_pg->publish_stats_to_osd(); - m_flags.deep_scrub_on_error = request.deep_scrub_on_error; } @@ -2185,8 +2195,6 @@ void PgScrubber::on_mid_scrub_abort(Scrub::delay_cause_t issue) m_planned_scrub.must_scrub || m_flags.required; m_planned_scrub.must_repair = m_planned_scrub.must_repair || m_is_repair; m_planned_scrub.need_auto = m_planned_scrub.need_auto || m_flags.auto_repair; - m_planned_scrub.deep_scrub_on_error = - m_planned_scrub.deep_scrub_on_error || m_flags.deep_scrub_on_error; // copy the aborted target const auto aborted_target = *m_active_target; @@ -2276,7 +2284,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session( "{}: cannot scrub (not clean). Registered?{:c}", __func__, m_scrub_job->is_registered() ? 'Y' : 'n') << dendl; - requeue_penalized(trgt.level(), delay_cause_t::pg_state, clock_now); + requeue_penalized(s_or_d, delay_cause_t::pg_state, clock_now); return schedule_result_t::target_specific_failure; } @@ -2285,7 +2293,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session( // (on the transition from NotTrimming to Trimming/WaitReservation), // i.e. some time before setting 'snaptrim'. dout(10) << __func__ << ": cannot scrub while snap-trimming" << dendl; - requeue_penalized(trgt.level(), delay_cause_t::pg_state, clock_now); + requeue_penalized(s_or_d, delay_cause_t::pg_state, clock_now); return schedule_result_t::target_specific_failure; } @@ -2296,13 +2304,13 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session( if (!pg_cond.allow_shallow) { // can't scrub at all dout(10) << __func__ << ": shallow not allowed" << dendl; - requeue_penalized(trgt.level(), delay_cause_t::flags, clock_now); + requeue_penalized(s_or_d, delay_cause_t::flags, clock_now); return schedule_result_t::target_specific_failure; } } else if (!pg_cond.allow_deep) { // can't scrub at all dout(10) << __func__ << ": deep not allowed" << dendl; - requeue_penalized(trgt.level(), delay_cause_t::flags, clock_now); + requeue_penalized(s_or_d, delay_cause_t::flags, clock_now); return schedule_result_t::target_specific_failure; } } @@ -2316,7 +2324,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session( // scrub until this is solved. dout(10) << __func__ << ": Regular scrub skipped due to deep-scrub errors" << dendl; - requeue_penalized(trgt.level(), delay_cause_t::pg_state, clock_now); + requeue_penalized(s_or_d, delay_cause_t::pg_state, clock_now); return schedule_result_t::target_specific_failure; } else { // we will honor the request anyway, but will report the issue @@ -2334,8 +2342,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session( << ": skipping this PG as repairing was not explicitly " "requested for it" << dendl; - requeue_penalized( - trgt.level(), delay_cause_t::scrub_params, clock_now); + requeue_penalized(s_or_d, delay_cause_t::scrub_params, clock_now); return schedule_result_t::target_specific_failure; } @@ -2343,8 +2350,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session( // be retried by the OSD later on. if (!reserve_local(trgt)) { dout(10) << __func__ << ": failed to reserve locally" << dendl; - requeue_penalized( - trgt.level(), delay_cause_t::local_resources, clock_now); + requeue_penalized(s_or_d, delay_cause_t::local_resources, clock_now); return schedule_result_t::osd_wide_failure; } @@ -2355,7 +2361,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session( state_clear(PG_STATE_REPAIR); m_active_target = trgt; - set_op_parameters(m_planned_scrub); + set_op_parameters(pg_cond, m_planned_scrub); // dequeue the PG's "other" target m_osds->get_scrub_services().remove_from_osd_queue(m_pg_id); m_scrub_job->clear_both_targets_queued(); diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index 90b7697002365..4a4176b0a5832 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -380,7 +380,9 @@ class PgScrubber : public ScrubPgIF, * flag-set; PG_STATE_SCRUBBING, and possibly PG_STATE_DEEP_SCRUB & * PG_STATE_REPAIR are set. */ - void set_op_parameters(const requested_scrub_t& request) final; + void set_op_parameters( + Scrub::ScrubPGPreconds pg_cond, + const requested_scrub_t& request) final; void cleanup_store(ObjectStore::Transaction* t) final; diff --git a/src/osd/scrubber_common.h b/src/osd/scrubber_common.h index 12808f71e7f3e..c14834db8aae6 100644 --- a/src/osd/scrubber_common.h +++ b/src/osd/scrubber_common.h @@ -310,7 +310,7 @@ struct requested_scrub_t { /** * Set from: * - scrub_requested() with need_auto param set, which only happens in - * - scrub_finish() - if deep_scrub_on_error is set, and we have errors + * - scrub_finish() - if can_autorepair is set, and we have errors * * If set, will prevent the OSD from casually postponing our scrub. When * scrubbing starts, will cause must_scrub, must_deep_scrub and auto_repair to @@ -325,8 +325,6 @@ struct requested_scrub_t { */ bool must_deep_scrub{false}; - bool deep_scrub_on_error{false}; - /** * If set, we should see must_deep_scrub & must_scrub, too * @@ -354,10 +352,9 @@ struct fmt::formatter { auto format(const requested_scrub_t& rs, FormatContext& ctx) { return fmt::format_to(ctx.out(), - "(plnd:{}{}{}{}{}{}{})", + "(plnd:{}{}{}{}{}{})", rs.must_repair ? " must_repair" : "", rs.auto_repair ? " auto_repair" : "", - rs.deep_scrub_on_error ? " deep_scrub_on_error" : "", rs.must_deep_scrub ? " must_deep_scrub" : "", rs.must_scrub ? " must_scrub" : "", rs.need_auto ? " need_auto" : "", @@ -478,7 +475,9 @@ struct ScrubPgIF { Scrub::ScrubPGPreconds pg_cond, const requested_scrub_t& requested_flags) = 0; - virtual void set_op_parameters(const requested_scrub_t&) = 0; + virtual void set_op_parameters( + Scrub::ScrubPGPreconds pg_cond, + const requested_scrub_t&) = 0; /// stop any active scrubbing (on interval end) and unregister from /// the OSD scrub queue -- 2.39.5