Skip to content

Commit 38057ce

Browse files
committed
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 <[email protected]>
1 parent 24bc646 commit 38057ce

File tree

3 files changed

+21
-23
lines changed

3 files changed

+21
-23
lines changed

src/osd/scrubber/pg_scrubber.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2377,7 +2377,7 @@ pg_scrubbing_status_t PgScrubber::get_schedule() const
23772377

23782378
} else {
23792379
int32_t dur_seconds =
2380-
duration_cast<seconds>(m_fsm->get_time_scrubbing()).count();
2380+
ceil<seconds>(m_fsm->get_time_scrubbing()).count();
23812381
return pg_scrubbing_status_t{
23822382
utime_t{},
23832383
dur_seconds,

src/osd/scrubber/scrub_machine.cc

Lines changed: 15 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -89,23 +89,18 @@ std::ostream& ScrubMachine::gen_prefix(std::ostream& out) const
8989

9090
ceph::timespan ScrubMachine::get_time_scrubbing() const
9191
{
92-
// note: the state_cast does not work in the Session ctor
93-
auto session = state_cast<const Session*>();
94-
if (!session) {
95-
dout(20) << fmt::format("{}: not in session", __func__) << dendl;
92+
if (!m_session_started_at) {
93+
dout(30) << fmt::format("{}: no session_start time", __func__) << dendl;
9694
return ceph::timespan{};
9795
}
9896

99-
if (session && session->m_session_started_at != ScrubTimePoint{}) {
100-
dout(20) << fmt::format(
101-
"{}: session_started_at: {} d:{}", __func__,
102-
session->m_session_started_at,
103-
ScrubClock::now() - session->m_session_started_at)
97+
const auto dur = ScrubClock::now() - *m_session_started_at;
98+
dout(20) << fmt::format(
99+
"{}: session_started_at: {} duration:{}ms", __func__,
100+
*m_session_started_at,
101+
ceil<milliseconds>(dur).count())
104102
<< dendl;
105-
return ScrubClock::now() - session->m_session_started_at;
106-
}
107-
dout(30) << fmt::format("{}: no session_start time", __func__) << dendl;
108-
return ceph::timespan{};
103+
return dur;
109104
}
110105

111106
std::optional<pg_scrubbing_status_t> ScrubMachine::get_reservation_status()
@@ -196,6 +191,8 @@ Session::Session(my_context ctx)
196191
dout(10) << "-- state -->> PrimaryActive/Session" << dendl;
197192
DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases
198193

194+
machine.m_session_started_at = ScrubClock::now();
195+
199196
m_perf_set = scrbr->get_labeled_counters();
200197
m_osd_counters = scrbr->get_osd_perf_counters();
201198
m_counters_idx = &scrbr->get_unlabeled_counters();
@@ -206,6 +203,7 @@ Session::~Session()
206203
{
207204
DECLARE_LOCALS; // 'scrbr' & 'pg_id' aliases
208205
m_reservations.reset();
206+
machine.m_session_started_at.reset();
209207

210208
// note the interaction between clearing the 'queued' flag and two
211209
// other states: the snap-mapper and the scrubber internal state.
@@ -337,12 +335,12 @@ ActiveScrubbing::~ActiveScrubbing()
337335

338336
// if the begin-time stamp was not set 'off' (as done if the scrubbing
339337
// completed successfully), we use it now to set the 'failed scrub' duration.
340-
if (session.m_session_started_at != ScrubTimePoint{}) {
338+
if (machine.m_session_started_at) {
341339
// delay the next invocation of the scrubber on this target
342340
scrbr->on_mid_scrub_abort(
343341
session.m_abort_reason.value_or(Scrub::delay_cause_t::aborted));
344342

345-
auto logged_duration = ScrubClock::now() - session.m_session_started_at;
343+
auto logged_duration = ScrubClock::now() - *machine.m_session_started_at;
346344
session.m_osd_counters->tinc(session.m_counters_idx->failed_elapsed,
347345
logged_duration);
348346
session.m_osd_counters->inc(session.m_counters_idx->failed_cnt);
@@ -723,8 +721,8 @@ sc::result WaitDigestUpdate::react(const ScrubFinished&)
723721

724722
// set the 'scrub duration'
725723
auto duration = machine.get_time_scrubbing();
726-
scrbr->set_scrub_duration(duration_cast<milliseconds>(duration));
727-
session.m_session_started_at = ScrubTimePoint{};
724+
scrbr->set_scrub_duration(ceil<milliseconds>(duration));
725+
machine.m_session_started_at.reset();
728726
session.m_osd_counters->tinc(
729727
session.m_counters_idx->successful_elapsed, duration);
730728

src/osd/scrubber/scrub_machine.h

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,6 @@ class ScrubMachine : public ScrubFsmIf, public sc::state_machine<ScrubMachine, N
284284
public:
285285
friend class PgScrubber;
286286

287-
public:
288287
explicit ScrubMachine(PG* pg, ScrubMachineListener* pg_scrub);
289288
virtual ~ScrubMachine();
290289

@@ -309,10 +308,14 @@ class ScrubMachine : public ScrubFsmIf, public sc::state_machine<ScrubMachine, N
309308
sc::state_machine<ScrubMachine, NotActive>::process_event(evt);
310309
}
311310

312-
// ///////////////// aux declarations & functions //////////////////////// //
311+
/// the time when the session was initiated
312+
std::optional<ScrubTimePoint> m_session_started_at;
313+
313314

315+
// ///////////////// aux declarations & functions //////////////////////// //
314316

315317
private:
318+
316319
/**
317320
* scheduled_event_state_t
318321
*
@@ -573,9 +576,6 @@ struct Session : sc::state<Session, PrimaryActive, ReservingReplicas>,
573576
/// this pool type)
574577
const ScrubCounterSet* m_counters_idx{nullptr};
575578

576-
/// the time when the session was initiated
577-
ScrubTimePoint m_session_started_at{ScrubClock::now()};
578-
579579
/// abort reason - if known. Determines the delay time imposed on the
580580
/// failed scrub target.
581581
std::optional<Scrub::delay_cause_t> m_abort_reason{std::nullopt};

0 commit comments

Comments
 (0)