From e104a0bf8c401b490294deb18b512e98840c39d6 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Wed, 4 Jun 2025 12:44:16 -0500 Subject: [PATCH] osd/scrub: make m_session_started_at at Session state ctor ScrubMachine::get_time_scrubbing() must access the Session object to compute the scrub duration. But the State data is not externally accessible before its ctor has completed. As we always happen to try to access that data inside the ctor, this always results in a warning log message. Here we move m_session_started_at into the outer state, simplifying the logic required to access it. Fixes: https://tracker.ceph.com/issues/64955 Signed-off-by: Ronen Friedman (cherry picked from commit 38057ce2c14ca9a6b81617399480510fb0eae951) --- src/osd/scrubber/pg_scrubber.cc | 2 +- src/osd/scrubber/scrub_machine.cc | 32 +++++++++++++++---------------- src/osd/scrubber/scrub_machine.h | 10 +++++----- 3 files changed, 21 insertions(+), 23 deletions(-) diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index af8890c1788b8..65e99ad387c4e 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -2377,7 +2377,7 @@ pg_scrubbing_status_t PgScrubber::get_schedule() const } else { int32_t dur_seconds = - duration_cast(m_fsm->get_time_scrubbing()).count(); + ceil(m_fsm->get_time_scrubbing()).count(); return pg_scrubbing_status_t{ utime_t{}, dur_seconds, diff --git a/src/osd/scrubber/scrub_machine.cc b/src/osd/scrubber/scrub_machine.cc index 28cba5bd4cb5b..d6cf2c03b9679 100644 --- a/src/osd/scrubber/scrub_machine.cc +++ b/src/osd/scrubber/scrub_machine.cc @@ -89,23 +89,18 @@ std::ostream& ScrubMachine::gen_prefix(std::ostream& out) const ceph::timespan ScrubMachine::get_time_scrubbing() const { - // note: the state_cast does not work in the Session ctor - auto session = state_cast(); - if (!session) { - dout(20) << fmt::format("{}: not in session", __func__) << dendl; + if (!m_session_started_at) { + dout(30) << fmt::format("{}: no session_start time", __func__) << dendl; return ceph::timespan{}; } - if (session && session->m_session_started_at != ScrubTimePoint{}) { - dout(20) << fmt::format( - "{}: session_started_at: {} d:{}", __func__, - session->m_session_started_at, - ScrubClock::now() - session->m_session_started_at) + const auto dur = ScrubClock::now() - *m_session_started_at; + dout(20) << fmt::format( + "{}: session_started_at: {} duration:{}ms", __func__, + *m_session_started_at, + ceil(dur).count()) << dendl; - return ScrubClock::now() - session->m_session_started_at; - } - dout(30) << fmt::format("{}: no session_start time", __func__) << dendl; - return ceph::timespan{}; + return dur; } std::optional ScrubMachine::get_reservation_status() @@ -196,6 +191,8 @@ Session::Session(my_context ctx) dout(10) << "-- state -->> PrimaryActive/Session" << dendl; DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases + machine.m_session_started_at = ScrubClock::now(); + m_perf_set = scrbr->get_labeled_counters(); m_osd_counters = scrbr->get_osd_perf_counters(); m_counters_idx = &scrbr->get_unlabeled_counters(); @@ -206,6 +203,7 @@ Session::~Session() { DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases m_reservations.reset(); + machine.m_session_started_at.reset(); // note the interaction between clearing the 'queued' flag and two // other states: the snap-mapper and the scrubber internal state. @@ -337,12 +335,12 @@ ActiveScrubbing::~ActiveScrubbing() // if the begin-time stamp was not set 'off' (as done if the scrubbing // completed successfully), we use it now to set the 'failed scrub' duration. - if (session.m_session_started_at != ScrubTimePoint{}) { + if (machine.m_session_started_at) { // delay the next invocation of the scrubber on this target scrbr->on_mid_scrub_abort( session.m_abort_reason.value_or(Scrub::delay_cause_t::aborted)); - auto logged_duration = ScrubClock::now() - session.m_session_started_at; + auto logged_duration = ScrubClock::now() - *machine.m_session_started_at; session.m_osd_counters->tinc(session.m_counters_idx->failed_elapsed, logged_duration); session.m_osd_counters->inc(session.m_counters_idx->failed_cnt); @@ -723,8 +721,8 @@ sc::result WaitDigestUpdate::react(const ScrubFinished&) // set the 'scrub duration' auto duration = machine.get_time_scrubbing(); - scrbr->set_scrub_duration(duration_cast(duration)); - session.m_session_started_at = ScrubTimePoint{}; + scrbr->set_scrub_duration(ceil(duration)); + machine.m_session_started_at.reset(); session.m_osd_counters->tinc( session.m_counters_idx->successful_elapsed, duration); diff --git a/src/osd/scrubber/scrub_machine.h b/src/osd/scrubber/scrub_machine.h index 48b78a7b38d11..db3905ca478b5 100644 --- a/src/osd/scrubber/scrub_machine.h +++ b/src/osd/scrubber/scrub_machine.h @@ -284,7 +284,6 @@ class ScrubMachine : public ScrubFsmIf, public sc::state_machine::process_event(evt); } -// ///////////////// aux declarations & functions //////////////////////// // + /// the time when the session was initiated + std::optional m_session_started_at; + + // ///////////////// aux declarations & functions //////////////////////// // private: + /** * scheduled_event_state_t * @@ -573,9 +576,6 @@ struct Session : sc::state, /// this pool type) const ScrubCounterSet* m_counters_idx{nullptr}; - /// the time when the session was initiated - ScrubTimePoint m_session_started_at{ScrubClock::now()}; - /// abort reason - if known. Determines the delay time imposed on the /// failed scrub target. std::optional m_abort_reason{std::nullopt}; -- 2.39.5