From da57f6edbc5593b8339d770d53b31f12bd6fb679 Mon Sep 17 00:00:00 2001 From: Ronen Friedman Date: Thu, 11 Nov 2021 10:14:23 +0000 Subject: [PATCH] osd/scrub: minor fixes/formatting changes Fixing a compiler warning. The formatting changes are mostly related to imposing the 80-cols limit on modified functions. Split from "osd/scrub: mark PG as being scrubbed, from scrub initiation to Inactive state" Signed-off-by: Ronen Friedman --- src/osd/PG.cc | 37 ++++++++++++++++++------------- src/osd/PG.h | 1 + src/osd/PrimaryLogPG.cc | 8 +++---- src/osd/scrubber/pg_scrubber.cc | 24 +++++++++++--------- src/osd/scrubber/pg_scrubber.h | 7 +++--- src/osd/scrubber/scrub_machine.cc | 1 - src/osd/scrubber/scrub_machine.h | 2 +- 7 files changed, 45 insertions(+), 35 deletions(-) diff --git a/src/osd/PG.cc b/src/osd/PG.cc index 25278eb7585..af667a09130 100644 --- a/src/osd/PG.cc +++ b/src/osd/PG.cc @@ -629,8 +629,8 @@ void PG::release_backoffs(const hobject_t& begin, const hobject_t& end) auto q = p->second.begin(); while (q != p->second.end()) { dout(20) << __func__ << " checking " << *q << dendl; - int r = cmp((*q)->begin, begin); - if (r == 0 || (r > 0 && (*q)->end < end)) { + int rr = cmp((*q)->begin, begin); + if (rr == 0 || (rr > 0 && (*q)->end < end)) { bv.push_back(*q); q = p->second.erase(q); } else { @@ -756,7 +756,7 @@ void PG::set_probe_targets(const set &probe_set) void PG::send_cluster_message( int target, MessageRef m, - epoch_t epoch, bool share_map_update=false) + epoch_t epoch, bool share_map_update) { ConnectionRef con = osd->get_con_osd_cluster( target, get_osdmap_epoch()); @@ -1477,14 +1477,16 @@ bool PG::verify_periodic_scrub_mode(bool allow_deep_scrub, std::optional PG::verify_scrub_mode() const { - bool allow_regular_scrub = !(get_osdmap()->test_flag(CEPH_OSDMAP_NOSCRUB) || - pool.info.has_flag(pg_pool_t::FLAG_NOSCRUB)); - bool allow_deep_scrub = allow_regular_scrub && - !(get_osdmap()->test_flag(CEPH_OSDMAP_NODEEP_SCRUB) || - pool.info.has_flag(pg_pool_t::FLAG_NODEEP_SCRUB)); - bool has_deep_errors = (info.stats.stats.sum.num_deep_scrub_errors > 0); - bool try_to_auto_repair = (cct->_conf->osd_scrub_auto_repair && - get_pgbackend()->auto_repair_supported()); + const bool allow_regular_scrub = + !(get_osdmap()->test_flag(CEPH_OSDMAP_NOSCRUB) || + pool.info.has_flag(pg_pool_t::FLAG_NOSCRUB)); + const bool allow_deep_scrub = + allow_regular_scrub && + !(get_osdmap()->test_flag(CEPH_OSDMAP_NODEEP_SCRUB) || + pool.info.has_flag(pg_pool_t::FLAG_NODEEP_SCRUB)); + const bool has_deep_errors = (info.stats.stats.sum.num_deep_scrub_errors > 0); + const bool try_to_auto_repair = (cct->_conf->osd_scrub_auto_repair && + get_pgbackend()->auto_repair_supported()); dout(10) << __func__ << " pg: " << info.pgid << " allow: " << allow_regular_scrub << "/" << allow_deep_scrub @@ -1501,18 +1503,21 @@ std::optional PG::verify_scrub_mode() const upd_flags.auto_repair = false; if (upd_flags.must_scrub && !upd_flags.must_deep_scrub && has_deep_errors) { - osd->clog->error() << "osd." << osd->whoami << " pg " << info.pgid - << " Regular scrub request, deep-scrub details will be lost"; + osd->clog->error() + << "osd." << osd->whoami << " pg " << info.pgid + << " Regular scrub request, deep-scrub details will be lost"; } if (!upd_flags.must_scrub) { // All periodic scrub handling goes here because must_scrub is // always set for must_deep_scrub and must_repair. - bool can_start_periodic = - verify_periodic_scrub_mode(allow_deep_scrub, try_to_auto_repair, - allow_regular_scrub, has_deep_errors, upd_flags); + const bool can_start_periodic = verify_periodic_scrub_mode( + allow_deep_scrub, try_to_auto_repair, allow_regular_scrub, + has_deep_errors, upd_flags); if (!can_start_periodic) { + // "I don't want no scrub" + dout(20) << __func__ << ": no periodic scrubs allowed" << dendl; return std::nullopt; } } diff --git a/src/osd/PG.h b/src/osd/PG.h index b68f1bfda60..9d4872d926e 100644 --- a/src/osd/PG.h +++ b/src/osd/PG.h @@ -185,6 +185,7 @@ public: /// flags detailing scheduling/operation characteristics of the next scrub requested_scrub_t m_planned_scrub; + /// scrubbing state for both Primary & replicas bool is_scrub_active() const { return m_scrubber->is_scrub_active(); } diff --git a/src/osd/PrimaryLogPG.cc b/src/osd/PrimaryLogPG.cc index fae6281ca1d..f607c188ff8 100644 --- a/src/osd/PrimaryLogPG.cc +++ b/src/osd/PrimaryLogPG.cc @@ -12435,7 +12435,7 @@ void PrimaryLogPG::_applied_recovered_object(ObjectContextRef obc) // requeue an active chunky scrub waiting on recovery ops if (!recovery_state.is_deleting() && active_pushes == 0 && - m_scrubber->is_scrub_active()) { + is_scrub_active()) { osd->queue_scrub_pushes_update(this, is_scrub_blocking_ops()); } @@ -12449,7 +12449,7 @@ void PrimaryLogPG::_applied_recovered_object_replica() // requeue an active scrub waiting on recovery ops if (!recovery_state.is_deleting() && active_pushes == 0 && - m_scrubber->is_scrub_active()) { + is_scrub_active()) { osd->queue_scrub_replica_pushes(this, m_scrubber->replica_op_priority()); } @@ -15407,10 +15407,10 @@ bool PrimaryLogPG::already_complete(eversion_t v) void PrimaryLogPG::do_replica_scrub_map(OpRequestRef op) { - dout(15) << __func__ << " is scrub active? " << m_scrubber->is_scrub_active() << dendl; + dout(15) << __func__ << " is scrub active? " << is_scrub_active() << dendl; op->mark_started(); - if (!m_scrubber->is_scrub_active()) { + if (!is_scrub_active()) { dout(10) << __func__ << " scrub isn't active" << dendl; return; } diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc index db18749b915..fb58633a679 100644 --- a/src/osd/scrubber/pg_scrubber.cc +++ b/src/osd/scrubber/pg_scrubber.cc @@ -1254,23 +1254,26 @@ Scrub::preemption_t& PgScrubber::get_preemptor() } /* - * Process note: called for the arriving "give me your map, replica!" request. Unlike - * the original implementation, we do not requeue the Op waiting for + * Process note: called for the arriving "give me your map, replica!" request. + * Unlike the original implementation, we do not requeue the Op waiting for * updates. Instead - we trigger the FSM. */ void PgScrubber::replica_scrub_op(OpRequestRef op) { op->mark_started(); auto msg = op->get_req(); - dout(10) << __func__ << " pg:" << m_pg->pg_id << " Msg: map_epoch:" << msg->map_epoch - << " min_epoch:" << msg->min_epoch << " deep?" << msg->deep << dendl; + dout(10) << __func__ << " pg:" << m_pg->pg_id + << " Msg: map_epoch:" << msg->map_epoch + << " min_epoch:" << msg->min_epoch << " deep?" << msg->deep << dendl; - // are we still processing a previous scrub-map request without noticing that the - // interval changed? won't see it here, but rather at the reservation stage. + // are we still processing a previous scrub-map request without noticing that + // the interval changed? won't see it here, but rather at the reservation + // stage. if (msg->map_epoch < m_pg->info.history.same_interval_since) { - dout(10) << "replica_scrub_op discarding old replica_scrub from " << msg->map_epoch - << " < " << m_pg->info.history.same_interval_since << dendl; + dout(10) << "replica_scrub_op discarding old replica_scrub from " + << msg->map_epoch << " < " + << m_pg->info.history.same_interval_since << dendl; // is there a general sync issue? are we holding a stale reservation? // not checking now - assuming we will actively react to interval change. @@ -1307,8 +1310,9 @@ void PgScrubber::replica_scrub_op(OpRequestRef op) m_max_end = msg->end; m_is_deep = msg->deep; m_interval_start = m_pg->info.history.same_interval_since; - m_replica_request_priority = msg->high_priority ? Scrub::scrub_prio_t::high_priority - : Scrub::scrub_prio_t::low_priority; + m_replica_request_priority = msg->high_priority + ? Scrub::scrub_prio_t::high_priority + : Scrub::scrub_prio_t::low_priority; m_flags.priority = msg->priority ? msg->priority : m_pg->get_scrub_priority(); preemption_data.reset(); diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h index d64eb994c53..1835310298f 100644 --- a/src/osd/scrubber/pg_scrubber.h +++ b/src/osd/scrubber/pg_scrubber.h @@ -504,7 +504,7 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener { // ----- methods used to verify the relevance of incoming events: /** - * is the incoming event still relevant, and should be processed? + * is the incoming event still relevant and should be forwarded to the FSM? * * It isn't if: * - (1) we are no longer 'actively scrubbing'; or @@ -513,7 +513,7 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener { * - (3) the message epoch is from a previous interval; or * - (4) the 'abort' configuration flags were set. * - * For (1) & (2) - teh incoming message is discarded, w/o further action. + * For (1) & (2) - the incoming message is discarded, w/o further action. * * For (3): (see check_interval() for a full description) if we have not reacted yet * to this specific new interval, we do now: @@ -596,6 +596,7 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener { const pg_shard_t m_pg_whoami; ///< a local copy of m_pg->pg_whoami; epoch_t m_interval_start{0}; ///< interval's 'from' of when scrubbing was first scheduled + /* * the exact epoch when the scrubbing actually started (started here - cleared checks * for no-scrub conf). Incoming events are verified against this, with stale events @@ -629,7 +630,7 @@ class PgScrubber : public ScrubPgIF, public ScrubMachineListener { * - all the time from scrub_finish() calling update_stats() till the * FSM handles the 'finished' event * - * Compared with 'm_active', this flag is asserted earlier and remains ON for longer. + * Compared with 'm_active', this flag is asserted earlier and remains ON for longer. */ bool m_queued_or_active{false}; diff --git a/src/osd/scrubber/scrub_machine.cc b/src/osd/scrubber/scrub_machine.cc index 8f88a77b52e..2610bf351a1 100644 --- a/src/osd/scrubber/scrub_machine.cc +++ b/src/osd/scrubber/scrub_machine.cc @@ -458,7 +458,6 @@ sc::result WaitDigestUpdate::react(const DigestUpdate&) // - finish the scrubbing of the current chunk, and: // - send NextChunk, or // - send ScrubFinished - scrbr->on_digest_updates(); return discard_event(); } diff --git a/src/osd/scrubber/scrub_machine.h b/src/osd/scrubber/scrub_machine.h index 75a7e02207b..d9132eedca8 100644 --- a/src/osd/scrubber/scrub_machine.h +++ b/src/osd/scrubber/scrub_machine.h @@ -90,7 +90,7 @@ MEV(IntLocalMapDone) MEV(DigestUpdate) ///< external. called upon success of a MODIFY op. See ///< scrub_snapshot_metadata() -MEV(MapsCompared) ///< (Crimson) maps_compare_n_cleanup() transactions are done +MEV(MapsCompared) ///< maps_compare_n_cleanup() transactions are done MEV(StartReplica) ///< initiating replica scrub. -- 2.39.5