From 9060dd7d0ea2e8cd94bbe90535aa3dc19857a3ac Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Sat, 11 Feb 2023 21:16:32 +0200 Subject: [PATCH] osd/scrub: only call publish_stats_to_osd() where allowed and needed This patch removes several unnecessary calls to publish_stats_to_osd() in the clear_pgscrub_state pathway used during interval changes. We didn't actually have anything to publish and we can't validly call publish_stats_to_osd() when we aren't an active primary anyway (which we can't be during an interval change). Fixes: https://tracker.ceph.com/issues/58496 Signed-off-by: Ronen Friedman (cherry picked from commit aeff19db99ab53fbaec7eb530e35d578e6254e4c) --- src/osd/scrubber/pg_scrubber.cc | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index 42518e88cee..6faf065d20e 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -537,6 +537,16 @@ void PgScrubber::on_primary_change( << dendl; } +/* + * A note re the call to publish_stats_to_osd() below: + * - we are called from either request_rescrubbing() or scrub_requested(). + * - in both cases - the schedule was modified, and needs to be published; + * - we are a Primary. + * - in the 1st case - the call is made as part of scrub_finish(), which + * guarantees that the PG is locked and the interval is still the same. + * - in the 2nd case - we know the PG state and we know we are only called + * for a Primary. +*/ void PgScrubber::update_scrub_job(const requested_scrub_t& request_flags) { dout(10) << fmt::format("{}: flags:<{}>", __func__, request_flags) << dendl; @@ -1039,9 +1049,7 @@ void PgScrubber::on_init() ceph_assert(!is_scrub_active()); m_pg->reset_objects_scrubbed(); preemption_data.reset(); - m_pg->publish_stats_to_osd(); m_interval_start = m_pg->get_history().same_interval_since; - dout(10) << __func__ << " start same_interval:" << m_interval_start << dendl; m_be = std::make_unique( @@ -1064,6 +1072,7 @@ void PgScrubber::on_init() m_start = m_pg->info.pgid.pgid.get_hobj_start(); m_active = true; ++m_sessions_counter; + // publish the session counter and the fact the we are scrubbing. m_pg->publish_stats_to_osd(); } @@ -1401,6 +1410,8 @@ void PgScrubber::maps_compare_n_cleanup() m_start = m_end; run_callbacks(); + + // requeue the writes from the chunk that just finished requeue_waiting(); m_osds->queue_scrub_maps_compared(m_pg, Scrub::scrub_prio_t::low_priority); } @@ -1529,7 +1540,8 @@ void PgScrubber::set_op_parameters(const requested_scrub_t& request) update_op_mode_text(); } - // the publishing here is required for tests synchronization + // 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; } @@ -2311,19 +2323,17 @@ void PgScrubber::cleanup_on_finish() state_clear(PG_STATE_SCRUBBING); state_clear(PG_STATE_DEEP_SCRUB); - m_pg->publish_stats_to_osd(); clear_scrub_reservations(); - m_pg->publish_stats_to_osd(); - requeue_waiting(); reset_internal_state(); - m_pg->publish_stats_to_osd(); m_flags = scrub_flags_t{}; // type-specific state clear _scrub_clear_state(); + // PG state flags changed: + m_pg->publish_stats_to_osd(); } // uses process_event(), so must be invoked externally @@ -2349,8 +2359,6 @@ void PgScrubber::clear_pgscrub_state() state_clear(PG_STATE_REPAIR); clear_scrub_reservations(); - m_pg->publish_stats_to_osd(); - requeue_waiting(); reset_internal_state(); @@ -2358,7 +2366,6 @@ void PgScrubber::clear_pgscrub_state() // type-specific state clear _scrub_clear_state(); - m_pg->publish_stats_to_osd(); } void PgScrubber::replica_handling_done() -- 2.39.5