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 <rfriedma@redhat.com>

diff --git a/src/osd/PG.cc b/src/osd/PG.cc
index 25278eb7585dee434ebdbab1fcdc6b3ea869e711..af667a091302f7d6ad54bf182aebd4a864851e60 100644 (file)
--- 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<pg_shard_t> &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<requested_scrub_t> 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<requested_scrub_t> 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 b68f1bfda60445cf4226ef2f7f8c22035cfb62bf..9d4872d926e9ac7b5e3dc1cfa0025aec4e8af569 100644 (file)
--- 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 fae6281ca1d9a75cfe3f852cf1e67e689614a3ca..f607c188ff8e460bbe70cef98c9f370e95cbc9ef 100644 (file)
--- 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 db18749b91509be3c95fce6ce3b90f0a2f88bfa4..fb58633a679a27999e4fc8d8b9f28cb95f47016a 100644 (file)
--- 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<MOSDRepScrub>();
-  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 d64eb994c53ecfb513c3b766903a5b688213fdaf..1835310298f275bcd83b10c31f6d2f5e53ad7214 100644 (file)
--- 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 8f88a77b52e06e7204effddc3a28855a2b0cfe11..2610bf351a15029e0d2f79375197178d06072701 100644 (file)
--- 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 75a7e02207ba0c882c0f8aa833aada92cc08b0ee..d9132eedca8581ada060f917cc491160c6667ba1 100644 (file)
--- 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.