Merge pull request #55453 from ronen-fr/wip-rf-0741-logs

osd/scrub: improve scheduling decision logs

Reviewed-by: Radoslaw Zarzynski <rzarzyns@redhat.com>

7 files changed, 98 insertions, 64 deletions

diff --git a/src/osd/scrubber/osd_scrub.cc b/src/osd/scrubber/osd_scrub.cc
index a74e1ae5c30..d2d2db3ff72 100644
--- a/src/osd/scrubber/osd_scrub.cc
+++ b/src/osd/scrubber/osd_scrub.cc
@@ -427,14 +427,6 @@ PerfCounters* OsdScrub::get_perf_counters(int pool_type, scrub_level_t level)
 // ////////////////////////////////////////////////////////////////////////// //
 // forwarders to the queue
 
-Scrub::sched_params_t OsdScrub::determine_scrub_time(
-    const requested_scrub_t& request_flags,
-    const pg_info_t& pg_info,
-    const pool_opts_t& pool_conf) const
-{
-  return m_queue.determine_scrub_time(request_flags, pg_info, pool_conf);
-}
-
 void OsdScrub::update_job(
     Scrub::ScrubJobRef sjob,
     const Scrub::sched_params_t& suggested,
diff --git a/src/osd/scrubber/osd_scrub.h b/src/osd/scrubber/osd_scrub.h
index 64709cc7aab..cd1158d4723 100644
--- a/src/osd/scrubber/osd_scrub.h
+++ b/src/osd/scrubber/osd_scrub.h
@@ -76,12 +76,6 @@ class OsdScrub {
   void mark_pg_scrub_blocked(spg_t blocked_pg);
   void clear_pg_scrub_blocked(spg_t blocked_pg);
 
-  // updating scheduling information for a specific PG
-  Scrub::sched_params_t determine_scrub_time(
-      const requested_scrub_t& request_flags,
-      const pg_info_t& pg_info,
-      const pool_opts_t& pool_conf) const;
-
   /**
    * modify a scrub-job's scheduled time and deadline
    *
diff --git a/src/osd/scrubber/osd_scrub_sched.cc b/src/osd/scrubber/osd_scrub_sched.cc
index 36644211ec3..1d0bf614c9b 100644
--- a/src/osd/scrubber/osd_scrub_sched.cc
+++ b/src/osd/scrubber/osd_scrub_sched.cc
@@ -168,42 +168,6 @@ void ScrubQueue::delay_on_failure(
 }
 
 
-sched_params_t ScrubQueue::determine_scrub_time(
-  const requested_scrub_t& request_flags,
-  const pg_info_t& pg_info,
-  const pool_opts_t& pool_conf) const
-{
-  sched_params_t res;
-
-  if (request_flags.must_scrub || request_flags.need_auto) {
-
-    // Set the smallest time that isn't utime_t()
-    res.proposed_time = PgScrubber::scrub_must_stamp();
-    res.is_must = Scrub::must_scrub_t::mandatory;
-    // we do not need the interval data in this case
-
-  } else if (pg_info.stats.stats_invalid && conf()->osd_scrub_invalid_stats) {
-    res.proposed_time = time_now();
-    res.is_must = Scrub::must_scrub_t::mandatory;
-
-  } else {
-    res.proposed_time = pg_info.history.last_scrub_stamp;
-    res.min_interval = pool_conf.value_or(pool_opts_t::SCRUB_MIN_INTERVAL, 0.0);
-    res.max_interval = pool_conf.value_or(pool_opts_t::SCRUB_MAX_INTERVAL, 0.0);
-  }
-
-  dout(15) << fmt::format(
-		"suggested: {:s} hist: {:s} v:{}/{} must:{} pool-min:{} {}",
-		res.proposed_time, pg_info.history.last_scrub_stamp,
-		(bool)pg_info.stats.stats_invalid,
-		conf()->osd_scrub_invalid_stats,
-		(res.is_must == must_scrub_t::mandatory ? "y" : "n"),
-		res.min_interval, request_flags)
-	   << dendl;
-  return res;
-}
-
-
 std::vector<ScrubTargetId> ScrubQueue::ready_to_scrub(
     OSDRestrictions restrictions,  // note: 4B in size! (copy)
     utime_t scrub_tick)
diff --git a/src/osd/scrubber/osd_scrub_sched.h b/src/osd/scrubber/osd_scrub_sched.h
index 95f1680d403..140c1428889 100644
--- a/src/osd/scrubber/osd_scrub_sched.h
+++ b/src/osd/scrubber/osd_scrub_sched.h
@@ -233,10 +233,6 @@ class ScrubQueue {
       Scrub::delay_cause_t delay_cause,
       utime_t now_is);
 
-  sched_params_t determine_scrub_time(const requested_scrub_t& request_flags,
-				      const pg_info_t& pg_info,
-				      const pool_opts_t& pool_conf) const;
-
   std::ostream& gen_prefix(std::ostream& out, std::string_view fn) const;
 
  public:
diff --git a/src/osd/scrubber/pg_scrubber.cc b/src/osd/scrubber/pg_scrubber.cc
index 444815fdf84..9fe7295201d 100644
--- a/src/osd/scrubber/pg_scrubber.cc
+++ b/src/osd/scrubber/pg_scrubber.cc
@@ -501,6 +501,43 @@ void PgScrubber::rm_from_osd_scrubbing()
   }
 }
 
+sched_params_t PgScrubber::determine_scrub_time(
+    const pool_opts_t& pool_conf) const
+{
+  sched_params_t res;
+
+  if (m_planned_scrub.must_scrub || m_planned_scrub.need_auto) {
+
+    // Set the smallest time that isn't utime_t()
+    res.proposed_time = PgScrubber::scrub_must_stamp();
+    res.is_must = Scrub::must_scrub_t::mandatory;
+    // we do not need the interval data in this case
+
+  } else if (
+      m_pg->info.stats.stats_invalid &&
+      get_pg_cct()->_conf->osd_scrub_invalid_stats) {
+    res.proposed_time = ceph_clock_now();
+    res.is_must = Scrub::must_scrub_t::mandatory;
+
+  } else {
+    res.proposed_time = m_pg->info.history.last_scrub_stamp;
+    res.min_interval = pool_conf.value_or(pool_opts_t::SCRUB_MIN_INTERVAL, 0.0);
+    res.max_interval = pool_conf.value_or(pool_opts_t::SCRUB_MAX_INTERVAL, 0.0);
+  }
+
+  dout(15)
+      << fmt::format(
+	     "{}: suggested: {:s} hist: {:s} v:{}/{} must:{} pool-min:{} {}",
+	     __func__, res.proposed_time, m_pg->info.history.last_scrub_stamp,
+	     (bool)m_pg->info.stats.stats_invalid,
+	     get_pg_cct()->_conf->osd_scrub_invalid_stats,
+	     (res.is_must == must_scrub_t::mandatory ? "y" : "n"),
+	     res.min_interval, m_planned_scrub)
+      << dendl;
+  return res;
+}
+
+
 /*
  * Note: referring to m_planned_scrub here is temporary, as this set of
  * scheduling flags will be removed in a followup PR.
@@ -513,8 +550,7 @@ void PgScrubber::schedule_scrub_with_osd()
   auto pre_state = m_scrub_job->state_desc();
   auto pre_reg = registration_state();
 
-  auto suggested = m_osds->get_scrub_services().determine_scrub_time(
-      m_planned_scrub, m_pg->info, m_pg->get_pgpool().info.opts);
+  auto suggested = determine_scrub_time(m_pg->get_pgpool().info.opts);
   m_osds->get_scrub_services().register_with_osd(m_scrub_job, suggested);
 
   dout(10) << fmt::format(
@@ -554,8 +590,7 @@ void PgScrubber::update_scrub_job(const requested_scrub_t& request_flags)
 
   if (is_primary() && m_scrub_job) {
     ceph_assert(m_pg->is_locked());
-    auto suggested = m_osds->get_scrub_services().determine_scrub_time(
-	request_flags, m_pg->info, m_pg->get_pgpool().info.opts);
+    auto suggested = determine_scrub_time(m_pg->get_pgpool().info.opts);
     m_osds->get_scrub_services().update_job(m_scrub_job, suggested, true);
     m_pg->publish_stats_to_osd();
   }
@@ -571,11 +606,12 @@ scrub_level_t PgScrubber::scrub_requested(
   const bool deep_requested = (scrub_level == scrub_level_t::deep) ||
 			      (scrub_type == scrub_type_t::do_repair);
   dout(10) << fmt::format(
-		  "{}: {} {} scrub requested. Prev stamp: {}. Registered? {}",
+		  "{}: {}{} scrub requested. "
+		  "@entry:{},last-stamp:{:s},Registered?{}",
 		  __func__,
-		  (scrub_type == scrub_type_t::do_repair ? " repair + "
-							 : " not-repair + "),
-		  (deep_requested ? "deep" : "shallow"),
+		  (scrub_type == scrub_type_t::do_repair ? "repair + "
+							 : "not-repair + "),
+		  (deep_requested ? "deep" : "shallow"), req_flags,
 		  m_scrub_job->get_sched_time(), registration_state())
 	   << dendl;
 
@@ -585,7 +621,6 @@ scrub_level_t PgScrubber::scrub_requested(
   // User might intervene, so clear this
   req_flags.need_auto = false;
   req_flags.req_scrub = true;
-
   dout(20) << fmt::format("{}: planned scrub:{}", __func__, req_flags) << dendl;
 
   update_scrub_job(req_flags);
diff --git a/src/osd/scrubber/pg_scrubber.h b/src/osd/scrubber/pg_scrubber.h
index bcab24cddfa..78e8ba90d44 100644
--- a/src/osd/scrubber/pg_scrubber.h
+++ b/src/osd/scrubber/pg_scrubber.h
@@ -784,6 +784,15 @@ class PgScrubber : public ScrubPgIF,
    */
   Scrub::sched_conf_t populate_config_params() const;
 
+  /**
+   * determine the time when the next scrub should be scheduled
+   *
+   * based on the planned scrub's flags, time of last scrub, and
+   * the pool's scrub configuration.
+   */
+  Scrub::sched_params_t determine_scrub_time(
+      const pool_opts_t& pool_conf) const;
+
   /*
    * Select a range of objects to scrub.
    *
diff --git a/src/test/osd/test_scrub_sched.cc b/src/test/osd/test_scrub_sched.cc
index b6c069c4b5f..0fec55bf062 100644
--- a/src/test/osd/test_scrub_sched.cc
+++ b/src/test/osd/test_scrub_sched.cc
@@ -20,8 +20,9 @@
 #include "osd/PG.h"
 #include "osd/osd_types.h"
 #include "osd/osd_types_fmt.h"
-#include "osd/scrubber/osd_scrub_sched.h"
 #include "osd/scrubber_common.h"
+#include "osd/scrubber/pg_scrubber.h"
+#include "osd/scrubber/osd_scrub_sched.h"
 
 int main(int argc, char** argv)
 {
@@ -47,6 +48,9 @@ using ScrubJobRef = Scrub::ScrubJobRef;
 using qu_state_t = Scrub::qu_state_t;
 using scrub_schedule_t = Scrub::scrub_schedule_t;
 using ScrubQContainer = Scrub::ScrubQContainer;
+using sched_params_t = Scrub::sched_params_t;
+
+
 
 /// enabling access into ScrubQueue internals
 class ScrubSchedTestWrapper : public ScrubQueue {
@@ -85,6 +89,46 @@ class ScrubSchedTestWrapper : public ScrubQueue {
     return m_time_for_testing.value_or(ceph_clock_now());
   }
 
+  /**
+  * a temporary implementation of PgScrubber::determine_scrub_time(). That
+  * function is to be removed in the near future, and modifying the
+  * test to use the actual PgScrubber::determine_scrub_time() would create
+  * an unneeded coupling between objects that are due for separation.
+  */
+  sched_params_t determine_scrub_time(
+      const requested_scrub_t& request_flags,
+      const pg_info_t& pg_info,
+      const pool_opts_t& pool_conf)
+  {
+    sched_params_t res;
+
+    if (request_flags.must_scrub || request_flags.need_auto) {
+
+      // Set the smallest time that isn't utime_t()
+      res.proposed_time = PgScrubber::scrub_must_stamp();
+      res.is_must = Scrub::must_scrub_t::mandatory;
+      // we do not need the interval data in this case
+
+    } else if (pg_info.stats.stats_invalid && conf()->osd_scrub_invalid_stats) {
+      res.proposed_time = time_now();
+      res.is_must = Scrub::must_scrub_t::mandatory;
+
+    } else {
+      res.proposed_time = pg_info.history.last_scrub_stamp;
+      res.min_interval =
+	  pool_conf.value_or(pool_opts_t::SCRUB_MIN_INTERVAL, 0.0);
+      res.max_interval =
+	  pool_conf.value_or(pool_opts_t::SCRUB_MAX_INTERVAL, 0.0);
+    }
+
+    std::cout << fmt::format(
+	"suggested: {:s} hist: {:s} v:{}/{} must:{} pool-min:{} {}\n",
+	res.proposed_time, pg_info.history.last_scrub_stamp,
+	(bool)pg_info.stats.stats_invalid, conf()->osd_scrub_invalid_stats,
+	(res.is_must == Scrub::must_scrub_t::mandatory ? "y" : "n"),
+	res.min_interval, request_flags);
+    return res;
+  }
   ~ScrubSchedTestWrapper() override = default;
 };