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 <rfriedma@redhat.com>
(cherry picked from commit 38057ce2c14ca9a6b81617399480510fb0eae951)

diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc
index af8890c1788b84679d2940c2733a52c9f055318d..65e99ad387c4ea8470033b215f4d78e3465fa8e8 100644 (file)
--- 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<seconds>(m_fsm->get_time_scrubbing()).count();
+         ceil<seconds>(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 28cba5bd4cb5bb8fd53ad87e653e6c28c865cd52..d6cf2c03b9679a566753dff802a02f5c09664de6 100644 (file)
--- 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<const Session*>();
-  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<milliseconds>(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<pg_scrubbing_status_t> 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<milliseconds>(duration));
-  session.m_session_started_at = ScrubTimePoint{};
+  scrbr->set_scrub_duration(ceil<milliseconds>(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 48b78a7b38d11216bba65131ea7701eb85aad488..db3905ca478b563765ff5978000244e752164ff5 100644 (file)
--- 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<ScrubMachine, N
  public:
   friend class PgScrubber;
 
- public:
   explicit ScrubMachine(PG* pg, ScrubMachineListener* pg_scrub);
   virtual ~ScrubMachine();
 
@@ -309,10 +308,14 @@ class ScrubMachine : public ScrubFsmIf, public sc::state_machine<ScrubMachine, N
     sc::state_machine<ScrubMachine, NotActive>::process_event(evt);
   }
 
-// ///////////////// aux declarations & functions //////////////////////// //
+  /// the time when the session was initiated
+  std::optional<ScrubTimePoint> m_session_started_at;
+
 
+  // ///////////////// aux declarations & functions //////////////////////// //
 
 private:
+
   /**
    * scheduled_event_state_t
    *
@@ -573,9 +576,6 @@ struct Session : sc::state<Session, PrimaryActive, ReservingReplicas>,
   /// 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<Scrub::delay_cause_t> m_abort_reason{std::nullopt};