osd/scrub: remove requested_scrub_t::deep_scrub_on_error

This flag was used to indicate that a deep scrub should
be performed if a shallow scrub finds an error. It was
always set true for shallow, regular, scrubs - if
can_autorepair flag was set. Thus, the ephemeral flag in
the requested_scrub_t object is not really needed.

Signed-off-by: Ronen Friedman <rfriedma@redhat.com>

diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc
index 7a37d8f23a397772db9188289d04d1e0a151e2e8..7b58c2cfdfce5a2238e5f2957fc340ab22f88392 100644 (file)
--- a/src/osd/scrubber/pg_scrubber.cc
+++ b/src/osd/scrubber/pg_scrubber.cc
@@ -67,8 +67,6 @@ ostream& operator<<(ostream& out, const requested_scrub_t& sf)
     out << " MUST_REPAIR";
   if (sf.auto_repair)
     out << " planned AUTO_REPAIR";
-  if (sf.deep_scrub_on_error)
-    out << " planned DEEP_SCRUB_ON_ERROR";
   if (sf.must_deep_scrub)
     out << " MUST_DEEP_SCRUB";
   if (sf.must_scrub)
@@ -1698,7 +1696,9 @@ void PgScrubber::replica_scrub_op(OpRequestRef op)
                              cost);
 }
 
-void PgScrubber::set_op_parameters(const requested_scrub_t& request)
+void PgScrubber::set_op_parameters(
+    ScrubPGPreconds pg_cond,
+    const requested_scrub_t& request)
 {
   dout(10) << fmt::format("{}: @ input: {}", __func__, request) << dendl;
 
@@ -1716,6 +1716,17 @@ void PgScrubber::set_op_parameters(const requested_scrub_t& request)
                       ? get_pg_cct()->_conf->osd_requested_scrub_priority
                       : m_pg->get_scrub_priority();
 
+  // 'deep-on-error' is set for periodic shallow scrubs, if allowed
+  // by the environment
+  if (m_active_target->is_shallow() && pg_cond.can_autorepair &&
+      m_active_target->urgency() == urgency_t::periodic_regular) {
+    m_flags.deep_scrub_on_error = true;
+    dout(10) << fmt::format(
+                   "{}: auto repair with scrubbing, rescrub if errors found",
+                   __func__)
+            << dendl;
+  }
+
   state_set(PG_STATE_SCRUBBING);
 
   // will we be deep-scrubbing?
@@ -1742,7 +1753,6 @@ void PgScrubber::set_op_parameters(const requested_scrub_t& request)
   // The publishing here is required for tests synchronization.
   // The PG state flags were modified.
   m_pg->publish_stats_to_osd();
-  m_flags.deep_scrub_on_error = request.deep_scrub_on_error;
 }
 
 
@@ -2185,8 +2195,6 @@ void PgScrubber::on_mid_scrub_abort(Scrub::delay_cause_t issue)
                               m_planned_scrub.must_scrub || m_flags.required;
   m_planned_scrub.must_repair = m_planned_scrub.must_repair || m_is_repair;
   m_planned_scrub.need_auto = m_planned_scrub.need_auto || m_flags.auto_repair;
-  m_planned_scrub.deep_scrub_on_error =
-      m_planned_scrub.deep_scrub_on_error || m_flags.deep_scrub_on_error;
 
   // copy the aborted target
   const auto aborted_target = *m_active_target;
@@ -2276,7 +2284,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
                    "{}: cannot scrub (not clean). Registered?{:c}", __func__,
                    m_scrub_job->is_registered() ? 'Y' : 'n')
             << dendl;
-    requeue_penalized(trgt.level(), delay_cause_t::pg_state, clock_now);
+    requeue_penalized(s_or_d, delay_cause_t::pg_state, clock_now);
     return schedule_result_t::target_specific_failure;
   }
 
@@ -2285,7 +2293,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
     // (on the transition from NotTrimming to Trimming/WaitReservation),
     // i.e. some time before setting 'snaptrim'.
     dout(10) << __func__ << ": cannot scrub while snap-trimming" << dendl;
-    requeue_penalized(trgt.level(), delay_cause_t::pg_state, clock_now);
+    requeue_penalized(s_or_d, delay_cause_t::pg_state, clock_now);
     return schedule_result_t::target_specific_failure;
   }
 
@@ -2296,13 +2304,13 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
       if (!pg_cond.allow_shallow) {
        // can't scrub at all
        dout(10) << __func__ << ": shallow not allowed" << dendl;
-       requeue_penalized(trgt.level(), delay_cause_t::flags, clock_now);
+       requeue_penalized(s_or_d, delay_cause_t::flags, clock_now);
        return schedule_result_t::target_specific_failure;
       }
     } else if (!pg_cond.allow_deep) {
       // can't scrub at all
       dout(10) << __func__ << ": deep not allowed" << dendl;
-      requeue_penalized(trgt.level(), delay_cause_t::flags, clock_now);
+      requeue_penalized(s_or_d, delay_cause_t::flags, clock_now);
       return schedule_result_t::target_specific_failure;
     }
   }
@@ -2316,7 +2324,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
     // scrub until this is solved.
       dout(10) << __func__ << ": Regular scrub skipped due to deep-scrub errors"
               << dendl;
-      requeue_penalized(trgt.level(), delay_cause_t::pg_state, clock_now);
+      requeue_penalized(s_or_d, delay_cause_t::pg_state, clock_now);
       return schedule_result_t::target_specific_failure;
     } else {
       // we will honor the request anyway, but will report the issue
@@ -2334,8 +2342,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
             << ": skipping this PG as repairing was not explicitly "
                "requested for it"
             << dendl;
-    requeue_penalized(
-       trgt.level(), delay_cause_t::scrub_params, clock_now);
+    requeue_penalized(s_or_d, delay_cause_t::scrub_params, clock_now);
     return schedule_result_t::target_specific_failure;
   }
 
@@ -2343,8 +2350,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
   // be retried by the OSD later on.
   if (!reserve_local(trgt)) {
     dout(10) << __func__ << ": failed to reserve locally" << dendl;
-    requeue_penalized(
-       trgt.level(), delay_cause_t::local_resources, clock_now);
+    requeue_penalized(s_or_d, delay_cause_t::local_resources, clock_now);
     return schedule_result_t::osd_wide_failure;
   }
 
@@ -2355,7 +2361,7 @@ Scrub::schedule_result_t PgScrubber::start_scrub_session(
   state_clear(PG_STATE_REPAIR);
 
   m_active_target = trgt;
-  set_op_parameters(m_planned_scrub);
+  set_op_parameters(pg_cond, m_planned_scrub);
   // dequeue the PG's "other" target
   m_osds->get_scrub_services().remove_from_osd_queue(m_pg_id);
   m_scrub_job->clear_both_targets_queued();

diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h
index 90b76970023656cc21c34043dee5a738deeb8090..4a4176b0a583232783c7e09d967f3f12096957a8 100644 (file)
--- a/src/osd/scrubber/pg_scrubber.h
+++ b/src/osd/scrubber/pg_scrubber.h
@@ -380,7 +380,9 @@ class PgScrubber : public ScrubPgIF,
    * flag-set; PG_STATE_SCRUBBING, and possibly PG_STATE_DEEP_SCRUB &
    * PG_STATE_REPAIR are set.
    */
-  void set_op_parameters(const requested_scrub_t& request) final;
+  void set_op_parameters(
+      Scrub::ScrubPGPreconds pg_cond,
+      const requested_scrub_t& request) final;
 
   void cleanup_store(ObjectStore::Transaction* t) final;
 

diff --git a/src/osd/scrubber_common.h b/src/osd/scrubber_common.h
index 12808f71e7f3ef50191d27f6a334274630fd74a5..c14834db8aae61fd9248da74fbc7b052ba76d2e9 100644 (file)
--- a/src/osd/scrubber_common.h
+++ b/src/osd/scrubber_common.h
@@ -310,7 +310,7 @@ struct requested_scrub_t {
   /**
    * Set from:
    *  - scrub_requested() with need_auto param set, which only happens in
-   *  - scrub_finish() - if deep_scrub_on_error is set, and we have errors
+   *  - scrub_finish() - if can_autorepair is set, and we have errors
    *
    * If set, will prevent the OSD from casually postponing our scrub. When
    * scrubbing starts, will cause must_scrub, must_deep_scrub and auto_repair to
@@ -325,8 +325,6 @@ struct requested_scrub_t {
    */
   bool must_deep_scrub{false};
 
-  bool deep_scrub_on_error{false};
-
   /**
    * If set, we should see must_deep_scrub & must_scrub, too
    *
@@ -354,10 +352,9 @@ struct fmt::formatter<requested_scrub_t> {
   auto format(const requested_scrub_t& rs, FormatContext& ctx)
   {
     return fmt::format_to(ctx.out(),
-                          "(plnd:{}{}{}{}{}{}{})",
+                          "(plnd:{}{}{}{}{}{})",
                           rs.must_repair ? " must_repair" : "",
                           rs.auto_repair ? " auto_repair" : "",
-                          rs.deep_scrub_on_error ? " deep_scrub_on_error" : "",
                           rs.must_deep_scrub ? " must_deep_scrub" : "",
                           rs.must_scrub ? " must_scrub" : "",
                           rs.need_auto ? " need_auto" : "",
@@ -478,7 +475,9 @@ struct ScrubPgIF {
       Scrub::ScrubPGPreconds pg_cond,
       const requested_scrub_t& requested_flags) = 0;
 
-  virtual void set_op_parameters(const requested_scrub_t&) = 0;
+  virtual void set_op_parameters(
+      Scrub::ScrubPGPreconds pg_cond,
+      const requested_scrub_t&) = 0;
 
   /// stop any active scrubbing (on interval end) and unregister from
   /// the OSD scrub queue