Merge pull request ceph#62769 from ronen-fr/wip-rf-delayready

osd/scrub: discard delay_ready_t

Reviewed-by: Matan Breizman <[email protected]>

Author: ronen-fr

src/osd/PG.cc

@@ -1298,16 +1298,12 @@ double PG::next_deepscrub_interval() const
   return info.history.last_deep_scrub_stamp + deep_scrub_interval;
 }
 
-void PG::on_scrub_schedule_input_change(Scrub::delay_ready_t delay_ready)
+void PG::on_scrub_schedule_input_change()
 {
   if (is_active() && is_primary() && !is_scrub_queued_or_active()) {
-    dout(10) << fmt::format(
-		    "{}: active/primary. delay_ready={:c}", __func__,
-		    (delay_ready == Scrub::delay_ready_t::delay_ready) ? 't'
-								       : 'f')
-	     << dendl;
+    dout(10) << fmt::format("{}: active/primary", __func__) << dendl;
     ceph_assert(m_scrubber);
-    m_scrubber->update_scrub_job(delay_ready);
+    m_scrubber->update_scrub_job();
   } else {
     dout(10) << fmt::format(
 		    "{}: inactive, non-primary - or already scrubbing",
@@ -2210,7 +2206,7 @@ void PG::handle_activate_map(PeeringCtx &rctx, epoch_t range_starts_at)
   // on_scrub_schedule_input_change() as pool.info contains scrub scheduling
   // parameters.
   if (pool.info.last_change >= range_starts_at) {
-    on_scrub_schedule_input_change(Scrub::delay_ready_t::delay_ready);
+    on_scrub_schedule_input_change();
   }
 }
 

src/osd/PG.h

@@ -262,7 +262,7 @@ class PG : public DoutPrefixProvider,
 	set_last_scrub_stamp(t, history, stats);
 	return true;
       });
-    on_scrub_schedule_input_change(Scrub::delay_ready_t::delay_ready);
+    on_scrub_schedule_input_change();
   }
 
   static void set_last_deep_scrub_stamp(
@@ -278,7 +278,7 @@ class PG : public DoutPrefixProvider,
 	set_last_scrub_stamp(t, history, stats);
 	return true;
       });
-    on_scrub_schedule_input_change(Scrub::delay_ready_t::delay_ready);
+    on_scrub_schedule_input_change();
   }
 
   static void add_objects_scrubbed_count(
@@ -511,7 +511,7 @@ class PG : public DoutPrefixProvider,
    * - pg stat scrub timestamps
    * - etc
    */
-  void on_scrub_schedule_input_change(Scrub::delay_ready_t delay_ready);
+  void on_scrub_schedule_input_change();
 
   void scrub_requested(scrub_level_t scrub_level, scrub_type_t scrub_type) override;
 

src/osd/scrubber/osd_scrub.cc

@@ -259,8 +259,7 @@ void OsdScrub::on_config_change()
 		    "updating scrub schedule on {}",
 		    (locked_pg->pg())->get_pgid())
 	     << dendl;
-    locked_pg->pg()->on_scrub_schedule_input_change(
-	Scrub::delay_ready_t::no_delay);
+    locked_pg->pg()->on_scrub_schedule_input_change();
   }
 }
 

src/osd/scrubber/pg_scrubber.cc

@@ -480,11 +480,10 @@ void PgScrubber::update_targets(utime_t scrub_clock_now)
 
   // the next periodic scrubs:
   m_scrub_job->adjust_shallow_schedule(
-      m_pg->info.history.last_scrub_stamp, applicable_conf, scrub_clock_now,
-      delay_ready_t::delay_ready);
+      m_pg->info.history.last_scrub_stamp, applicable_conf, scrub_clock_now);
   m_scrub_job->adjust_deep_schedule(
       m_pg->info.history.last_deep_scrub_stamp, applicable_conf,
-      scrub_clock_now, delay_ready_t::delay_ready);
+      scrub_clock_now);
 
   dout(10) << fmt::format("{}: adjusted:{}", __func__, *m_scrub_job) << dendl;
 }
@@ -499,7 +498,7 @@ void PgScrubber::schedule_scrub_with_osd()
 		  "{}: state at entry: {}", __func__, m_scrub_job->state_desc())
 	   << dendl;
   m_scrub_job->registered = true;
-  update_scrub_job(delay_ready_t::delay_ready);
+  update_scrub_job();
 }
 
 
@@ -523,7 +522,7 @@ void PgScrubber::on_primary_active_clean()
  * - in the 2nd case - we know the PG state and we know we are only called
  *   for a Primary.
  */
-void PgScrubber::update_scrub_job(Scrub::delay_ready_t delay_ready)
+void PgScrubber::update_scrub_job()
 {
   if (!is_primary() || !m_scrub_job) {
     dout(10) << fmt::format(
@@ -2036,7 +2035,7 @@ void PgScrubber::scrub_finish()
     request_rescrubbing();
   }
   // determine the next scrub time
-  update_scrub_job(delay_ready_t::delay_ready);
+  update_scrub_job();
 
   if (m_pg->is_active() && m_pg->is_primary()) {
     m_pg->recovery_state.share_pg_info();

src/osd/scrubber/pg_scrubber.h

@@ -326,7 +326,7 @@ class PgScrubber : public ScrubPgIF,
 
   // managing scrub op registration
 
-  void update_scrub_job(Scrub::delay_ready_t delay_ready) final;
+  void update_scrub_job() final;
 
   void rm_from_osd_scrubbing() final;
 

src/osd/scrubber/scrub_job.cc

@@ -13,7 +13,6 @@ using OSDRestrictions = Scrub::OSDRestrictions;
 using sched_conf_t = Scrub::sched_conf_t;
 using scrub_schedule_t = Scrub::scrub_schedule_t;
 using ScrubJob = Scrub::ScrubJob;
-using delay_ready_t = Scrub::delay_ready_t;
 using namespace std::chrono;
 
 using SchedEntry = Scrub::SchedEntry;
@@ -99,15 +98,11 @@ void ScrubJob::set_both_targets_queued()
 void ScrubJob::adjust_shallow_schedule(
     utime_t last_scrub,
     const Scrub::sched_conf_t& app_conf,
-    utime_t scrub_clock_now,
-    delay_ready_t modify_ready_targets)
+    utime_t scrub_clock_now)
 {
   dout(10) << fmt::format(
-		  "at entry: shallow target:{}, conf:{}, last-stamp:{:s} "
-		  "also-ready?{:c}",
-		  shallow_target, app_conf, last_scrub,
-		  (modify_ready_targets == delay_ready_t::delay_ready) ? 'y'
-								       : 'n')
+		  "at entry: shallow target:{}, conf:{}, last-stamp:{:s}",
+		  shallow_target, app_conf, last_scrub)
 	   << dendl;
 
   auto& sh_times = shallow_target.sched_info.schedule;	// shorthand
@@ -117,15 +112,11 @@ void ScrubJob::adjust_shallow_schedule(
     utime_t adj_target = last_scrub;
     sh_times.deadline = adj_target;
 
-    // add a random delay to the proposed scheduled time - but only for periodic
-    // scrubs that are not already eligible for scrubbing.
-    if ((modify_ready_targets == delay_ready_t::delay_ready) ||
-	adj_not_before > scrub_clock_now) {
-      adj_target += app_conf.shallow_interval;
-      double r = rand() / (double)RAND_MAX;
-      adj_target +=
+    // add a random delay to the proposed scheduled time
+    adj_target += app_conf.shallow_interval;
+    double r = rand() / (double)RAND_MAX;
+    adj_target +=
 	  app_conf.shallow_interval * app_conf.interval_randomize_ratio * r;
-    }
 
     // the deadline can be updated directly into the scrub-job
     if (app_conf.max_shallow) {
@@ -238,15 +229,11 @@ utime_t ScrubJob::get_sched_time() const
 void ScrubJob::adjust_deep_schedule(
     utime_t last_deep,
     const Scrub::sched_conf_t& app_conf,
-    utime_t scrub_clock_now,
-    delay_ready_t modify_ready_targets)
+    utime_t scrub_clock_now)
 {
   dout(10) << fmt::format(
-		  "at entry: deep target:{}, conf:{}, last-stamp:{:s} "
-		  "also-ready?{:c}",
-		  deep_target, app_conf, last_deep,
-		  (modify_ready_targets == delay_ready_t::delay_ready) ? 'y'
-								       : 'n')
+		  "at entry: deep target:{}, conf:{}, last-stamp:{:s}",
+		  deep_target, app_conf, last_deep)
 	   << dendl;
 
   auto& dp_times = deep_target.sched_info.schedule;  // shorthand
@@ -256,23 +243,19 @@ void ScrubJob::adjust_deep_schedule(
     utime_t adj_target = last_deep;
     dp_times.deadline = adj_target;
 
-    // add a random delay to the proposed scheduled time - but only for periodic
-    // scrubs that are not already eligible for scrubbing.
-    if ((modify_ready_targets == delay_ready_t::delay_ready) ||
-	adj_not_before > scrub_clock_now) {
-      double sdv = app_conf.deep_interval * app_conf.deep_randomize_ratio;
-      std::normal_distribution<double> normal_dist{app_conf.deep_interval, sdv};
-      auto next_delay = std::clamp(
-	  normal_dist(random_gen), app_conf.deep_interval - 2 * sdv,
-	  app_conf.deep_interval + 2 * sdv);
-      adj_target += next_delay;
-      dout(20) << fmt::format(
-		      "deep scrubbing: next_delay={:.0f} (interval={:.0f}, "
-		      "ratio={:.3f}), adjusted:{:s}",
-		      next_delay, app_conf.deep_interval,
-		      app_conf.deep_randomize_ratio, adj_target)
-	       << dendl;
-    }
+    // add a random delay to the proposed scheduled time
+    const double sdv = app_conf.deep_interval * app_conf.deep_randomize_ratio;
+    std::normal_distribution<double> normal_dist{app_conf.deep_interval, sdv};
+    auto next_delay =
+        std::clamp(normal_dist(random_gen), app_conf.deep_interval - 2 * sdv,
+                   app_conf.deep_interval + 2 * sdv);
+    adj_target += next_delay;
+    dout(20) << fmt::format(
+                    "deep scrubbing: next_delay={:.0f} (interval={:.0f}, "
+                    "ratio={:.3f}), adjusted:{:s}",
+                    next_delay, app_conf.deep_interval,
+                    app_conf.deep_randomize_ratio, adj_target)
+             << dendl;
 
     // the deadline can be updated directly into the scrub-job
     if (app_conf.max_shallow) {

src/osd/scrubber/scrub_job.h

@@ -235,14 +235,12 @@ class ScrubJob {
   void adjust_shallow_schedule(
     utime_t last_scrub,
     const Scrub::sched_conf_t& app_conf,
-    utime_t scrub_clock_now,
-    delay_ready_t modify_ready_targets);
+    utime_t scrub_clock_now);
 
   void adjust_deep_schedule(
     utime_t last_deep,
     const Scrub::sched_conf_t& app_conf,
-    utime_t scrub_clock_now,
-    delay_ready_t modify_ready_targets);
+    utime_t scrub_clock_now);
 
   /**
    * For the level specified, set the 'not-before' time to 'now+delay',

src/osd/scrubber_common.h

@@ -168,10 +168,6 @@ struct scrub_schedule_t {
   bool operator==(const scrub_schedule_t& rhs) const = default;
 };
 
-
-/// rescheduling param: should we delay jobs already ready to execute?
-enum class delay_ready_t : bool { delay_ready = true, no_delay = false };
-
 }  // namespace Scrub
 
 namespace fmt {
@@ -522,7 +518,7 @@ struct ScrubPgIF {
    *
    * Dequeues the scrub job, and re-queues it with the new schedule.
    */
-  virtual void update_scrub_job(Scrub::delay_ready_t delay_ready) = 0;
+  virtual void update_scrub_job() = 0;
 
   virtual scrub_level_t scrub_requested(
       scrub_level_t scrub_level,