Merge pull request ceph#63263 from ronen-fr/wip-rf-63183-tentacle

tentacle: osd/scrub: remove (was: fix) deadline calculations

Reviewed-by: Samuel Just <[email protected]>

Author: ronen-fr

doc/rados/operations/pools.rst

@@ -551,7 +551,7 @@ You may set values for the following keys:
 
 .. describe:: scrub_min_interval
 
-   :Description: Sets the minimum interval (in seconds) between successive shallow / light scrubs of the pool's PGs when the load is low. If the default value of ``0`` is in effect, then the value of ``osd_scrub_min_interval`` from central config is used.
+   :Description: Sets the minimum interval (in seconds) between successive shallow (light) scrubs of the pool's PGs. If this pool attribute is unchanged from its default (``0``), the value of ``osd_scrub_min_interval`` from central config is used instead.
 
    :Type: Double
    :Default: ``0``
@@ -560,7 +560,7 @@ You may set values for the following keys:
 
 .. describe:: scrub_max_interval
 
-   :Description: Sets the maximum interval (in seconds) between successive shallow / light scrubs of the pool's PGs regardless of cluster load. If the value of ``scrub_max_interval`` is ``0``, then the value ``osd_scrub_max_interval`` from central config is used.
+   :Description: Sets the maximum interval (in seconds) between successive shallow (light) scrubs of the pool's PGs. Affects the 'overdue' attribute appearing in scrub scheduler dumps. If unchanged from its default of ``0``, the value of ``osd_scrub_max_interval`` from central config is used instead.
 
    :Type: Double
    :Default: ``0``
@@ -569,7 +569,7 @@ You may set values for the following keys:
 
 .. describe:: deep_scrub_interval
 
-   :Description: Sets the interval (in seconds) for successive pool deep scrubs of the pool's PGs. If the value of ``deep_scrub_interval`` is ``0``, the value ``osd_deep_scrub_interval`` from central config is used.
+   :Description: Sets the interval (in seconds) for successive pool deep scrubs of the pool's PGs. If unchanged from its default of ``0``, the value of ``osd_deep_scrub_interval`` from central config is used instead.
 
    :Type: Double
    :Default: ``0``

qa/standalone/scrub/osd-scrub-test.sh

@@ -131,15 +131,11 @@ DATEFORMAT="%Y-%m-%d"
 function check_dump_scrubs() {
     local primary=$1
     local sched_time_check="$2"
-    local deadline_check="$3"
 
     DS="$(CEPH_ARGS='' ceph --admin-daemon $(get_asok_path osd.${primary}) dump_scrubs)"
     # use eval to drop double-quotes
     eval SCHED_TIME=$(echo $DS | jq '.[0].sched_time')
     test $(echo $SCHED_TIME | sed $DATESED) = $(date +${DATEFORMAT} -d "now + $sched_time_check") || return 1
-    # use eval to drop double-quotes
-    eval DEADLINE=$(echo $DS | jq '.[0].deadline')
-    test $(echo $DEADLINE | sed $DATESED) = $(date +${DATEFORMAT} -d "now + $deadline_check") || return 1
 }
 
 function TEST_interval_changes() {
@@ -178,27 +174,27 @@ function TEST_interval_changes() {
     local primary=$(get_primary $poolname obj1)
 
     # Check initial settings from above (min 1 day, min 1 week)
-    check_dump_scrubs $primary "1 day" "1 week" || return 1
+    check_dump_scrubs $primary "1 day" || return 1
 
     # Change global osd_scrub_min_interval to 2 days
     CEPH_ARGS='' ceph --admin-daemon $(get_asok_path osd.${primary}) config set osd_scrub_min_interval $(expr $day \* 2)
     sleep $WAIT_FOR_UPDATE
-    check_dump_scrubs $primary "2 days" "1 week" || return 1
+    check_dump_scrubs $primary "2 days" || return 1
 
     # Change global osd_scrub_max_interval to 2 weeks
     CEPH_ARGS='' ceph --admin-daemon $(get_asok_path osd.${primary}) config set osd_scrub_max_interval $(expr $week \* 2)
     sleep $WAIT_FOR_UPDATE
-    check_dump_scrubs $primary "2 days" "2 week" || return 1
+    check_dump_scrubs $primary "2 days" || return 1
 
     # Change pool osd_scrub_min_interval to 3 days
     ceph osd pool set $poolname scrub_min_interval $(expr $day \* 3)
     sleep $WAIT_FOR_UPDATE
-    check_dump_scrubs $primary "3 days" "2 week" || return 1
+    check_dump_scrubs $primary "3 days" || return 1
 
     # Change pool osd_scrub_max_interval to 3 weeks
     ceph osd pool set $poolname scrub_max_interval $(expr $week \* 3)
     sleep $WAIT_FOR_UPDATE
-    check_dump_scrubs $primary "3 days" "3 week" || return 1
+    check_dump_scrubs $primary "3 days" || return 1
     perf_counters $dir $OSDS
 }
 

src/osd/scrubber/osd_scrub_sched.cc

@@ -157,7 +157,6 @@ void ScrubQueue::dump_scrubs(ceph::Formatter* f) const
 	f->dump_stream("pgid") << e.pgid;
 	f->dump_stream("sched_time") << e.schedule.not_before;
 	f->dump_stream("orig_sched_time") << e.schedule.scheduled_at;
-	f->dump_stream("deadline") << e.schedule.deadline;
 	f->dump_bool(
 	    "forced",
 	    e.schedule.scheduled_at == PgScrubber::scrub_must_stamp());
@@ -167,7 +166,6 @@ void ScrubQueue::dump_scrubs(ceph::Formatter* f) const
                                        : "deep");
         f->dump_stream("urgency") << fmt::format("{}", e.urgency);
         f->dump_bool("eligible", e.schedule.not_before <= query_time);
-        f->dump_bool("overdue", e.schedule.deadline < query_time);
         f->dump_stream("last_issue") << fmt::format("{}", e.last_issue);
       },
       std::numeric_limits<int>::max());

src/osd/scrubber/pg_scrubber.cc

@@ -674,46 +674,29 @@ Scrub::sched_conf_t PgScrubber::populate_config_params() const
   const auto& conf = get_pg_cct()->_conf;  // for brevity
   Scrub::sched_conf_t configs;
 
-  // deep-scrub optimal interval
-  configs.deep_interval =
-      pool_conf.value_or(pool_opts_t::DEEP_SCRUB_INTERVAL, 0.0);
-  if (configs.deep_interval <= 0.0) {
-    configs.deep_interval = conf->osd_deep_scrub_interval;
-  }
-
-  // shallow-scrub interval
-  configs.shallow_interval =
+  // shallow scrubs interval
+  const auto shallow_pool =
       pool_conf.value_or(pool_opts_t::SCRUB_MIN_INTERVAL, 0.0);
-  if (configs.shallow_interval <= 0.0) {
-    configs.shallow_interval = conf->osd_scrub_min_interval;
-  }
-
-  // the max allowed delay between scrubs.
-  // For deep scrubs - there is no equivalent of scrub_max_interval. Per the
-  // documentation, once deep_scrub_interval has passed, we are already
-  // "overdue", at least as far as the "ignore allowed load" window is
-  // concerned.
-
-  configs.max_deep = configs.deep_interval + configs.shallow_interval;
-
-  auto max_shallow = pool_conf.value_or(pool_opts_t::SCRUB_MAX_INTERVAL, 0.0);
-  if (max_shallow <= 0.0) {
-    max_shallow = conf->osd_scrub_max_interval;
-  }
-  if (max_shallow > 0.0) {
-    configs.max_shallow = max_shallow;
-    // otherwise - we're left with the default nullopt
-  }
+  configs.shallow_interval =
+      shallow_pool > 0.0 ? shallow_pool : conf->osd_scrub_min_interval;
 
-  // but seems like our tests require: \todo fix!
-  configs.max_deep =
-      std::max(configs.max_shallow.value_or(0.0), configs.deep_interval);
+  // deep scrubs optimal interval
+  const auto deep_pool =
+      pool_conf.value_or(pool_opts_t::DEEP_SCRUB_INTERVAL, 0.0);
+  configs.deep_interval =
+      deep_pool > 0.0 ? deep_pool : conf->osd_deep_scrub_interval;
 
   configs.interval_randomize_ratio = conf->osd_scrub_interval_randomize_ratio;
-  configs.deep_randomize_ratio = conf.get_val<double>("osd_deep_scrub_interval_cv");
+  configs.deep_randomize_ratio =
+      conf.get_val<double>("osd_deep_scrub_interval_cv");
   configs.mandatory_on_invalid = conf->osd_scrub_invalid_stats;
 
-  dout(15) << fmt::format("{}: updated config:{}", __func__, configs) << dendl;
+  dout(15) << fmt::format(
+		  "{}: inputs: intervals: sh:{}(pl:{}),dp:{}(pl:{})",
+		  __func__, configs.shallow_interval, shallow_pool,
+		  configs.deep_interval, deep_pool)
+	   << dendl;
+  dout(10) << fmt::format("{}: updated config:{}", __func__, configs) << dendl;
   return configs;
 }
 
@@ -740,33 +723,29 @@ void PgScrubber::on_operator_periodic_cmd(
     scrub_level_t scrub_level,
     int64_t offset)
 {
-  const auto cnf = populate_config_params();
-  dout(10) << fmt::format(
-		  "{}: {} (cmd offset:{}) conf:{}", __func__,
-		  (scrub_level == scrub_level_t::deep ? "deep" : "shallow"), offset,
-		  cnf)
-	   << dendl;
-
-  // move the relevant time-stamp backwards - enough to trigger a scrub
-  utime_t stamp = ceph_clock_now();
-  if (offset > 0) {
-    stamp -= offset;
+  // if 'offset' wasn't specified - find a value that guarantees the scrub
+  // target will appear ready for scrubbing (even after random adjustments)
+  if (offset == 0) {
+    const auto cnf = populate_config_params();
+    offset = m_scrub_job->guaranteed_offset(scrub_level, cnf);
+    dout(15) << fmt::format(
+		    "{}: {} (calculated offset:{}) conf:{}", __func__,
+		    (scrub_level == scrub_level_t::deep ? "deep" : "shallow"),
+		    offset, cnf)
+	     << dendl;
   } else {
-    double max_iv =
-	(scrub_level == scrub_level_t::deep)
-	    ? 2 * cnf.max_deep
-	    : (cnf.max_shallow ? *cnf.max_shallow : cnf.shallow_interval);
-    dout(20) << fmt::format(
-		    "{}: stamp:{:s} ms:{}/{}/{}", __func__, stamp,
-		    (cnf.max_shallow ? "ms+" : "ms-"),
-		    (cnf.max_shallow ? *cnf.max_shallow : -999.99),
-		    cnf.shallow_interval)
+    dout(15) << fmt::format(
+		    "{}: {} command offset:{}", __func__,
+		    (scrub_level == scrub_level_t::deep ? "deep" : "shallow"),
+		    offset)
 	     << dendl;
-    stamp -= max_iv;
   }
-  stamp -= 100.0;  // for good measure
+  // move the relevant time-stamp backwards - enough to trigger a scrub
+  utime_t stamp = ceph_clock_now();
+  stamp -= offset;  // can't combine with prev line
 
-  dout(10) << fmt::format("{}: stamp:{:s} ", __func__, stamp) << dendl;
+  dout(10) << fmt::format("{}: calculated stamp:{:s}", __func__, stamp)
+	   << dendl;
   asok_response_section(f, true, scrub_level, stamp);
 
   if (scrub_level == scrub_level_t::deep) {
@@ -2080,7 +2059,7 @@ void PgScrubber::on_digest_updates()
 void PgScrubber::on_mid_scrub_abort(Scrub::delay_cause_t issue)
 {
   if (!m_scrub_job->is_registered()) {
-    dout(10) << fmt::format(
+    dout(5) << fmt::format(
 		    "{}: PG not registered for scrubbing on this OSD. Won't "
 		    "requeue!",
 		    __func__)
@@ -2110,7 +2089,6 @@ void PgScrubber::on_mid_scrub_abort(Scrub::delay_cause_t issue)
       std::max(current_targ.urgency(), aborted_target.urgency());
   curr_sched.scheduled_at =
       std::min(curr_sched.scheduled_at, abrt_sched.scheduled_at);
-  curr_sched.deadline = std::min(curr_sched.deadline, abrt_sched.deadline);
   curr_sched.not_before =
       std::min(curr_sched.not_before, abrt_sched.not_before);
 

src/osd/scrubber/scrub_job.cc

@@ -110,20 +110,13 @@ void ScrubJob::adjust_shallow_schedule(
   if (ScrubJob::requires_randomization(shallow_target.urgency())) {
     utime_t adj_not_before = last_scrub;
     utime_t adj_target = last_scrub;
-    sh_times.deadline = 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;
+	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) {
-      sh_times.deadline += *app_conf.max_shallow;
-    } else {
-      sh_times.deadline = utime_t{};
-    }
     if (adj_not_before < adj_target) {
       adj_not_before = adj_target;
     }
@@ -132,20 +125,33 @@ void ScrubJob::adjust_shallow_schedule(
 
   } else {
 
-    // the target time is already set. Make sure to reset the n.b. and
-    // the (irrelevant) deadline
+    // the target time is already set. Make sure to reset the n.b.
     sh_times.not_before = sh_times.scheduled_at;
-    sh_times.deadline = sh_times.scheduled_at;
   }
 
   dout(10) << fmt::format(
-		  "adjusted: nb:{:s} target:{:s} deadline:{:s} ({})",
-		  sh_times.not_before, sh_times.scheduled_at, sh_times.deadline,
-		  state_desc())
+		  "adjusted: nb:{:s} target:{:s} ({})", sh_times.not_before,
+		  sh_times.scheduled_at, state_desc())
 	   << dendl;
 }
 
 
+double ScrubJob::guaranteed_offset(
+    scrub_level_t s_or_d,
+    const Scrub::sched_conf_t& app_conf)
+{
+  if (s_or_d == scrub_level_t::deep) {
+    // use the sdv of the deep scrub distribution, times 3 (3-sigma...)
+    const double sdv = app_conf.deep_interval * app_conf.deep_randomize_ratio;
+  // note: the '+10.0' is there just to guarantee inequality if '._ratio' is 0
+    return app_conf.deep_interval + abs(3 * sdv) + 10.0;
+  }
+
+  // shallow scrub
+  return app_conf.shallow_interval * (2.0 + app_conf.interval_randomize_ratio);
+}
+
+
 void ScrubJob::operator_forced(scrub_level_t s_or_d, scrub_type_t scrub_type)
 {
   auto& trgt = get_target(s_or_d);
@@ -239,46 +245,32 @@ void ScrubJob::adjust_deep_schedule(
   auto& dp_times = deep_target.sched_info.schedule;  // shorthand
 
   if (ScrubJob::requires_randomization(deep_target.urgency())) {
-    utime_t adj_not_before = last_deep;
     utime_t adj_target = last_deep;
-    dp_times.deadline = adj_target;
 
     // 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);
+    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) {
-      dp_times.deadline += *app_conf.max_shallow;  // RRR fix
-    } else {
-      dp_times.deadline = utime_t{};
-    }
-    if (adj_not_before < adj_target) {
-      adj_not_before = adj_target;
-    }
+		    "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;
+
     dp_times.scheduled_at = adj_target;
-    dp_times.not_before = adj_not_before;
+    dp_times.not_before = adj_target;
   } else {
-    // the target time is already set. Make sure to reset the n.b. and
-    // the (irrelevant) deadline
+    // the target time is already set. The n.b. is set to same
     dp_times.not_before = dp_times.scheduled_at;
-    dp_times.deadline = dp_times.scheduled_at;
   }
 
   dout(10) << fmt::format(
-		  "adjusted: nb:{:s} target:{:s} deadline:{:s} ({})",
-		  dp_times.not_before, dp_times.scheduled_at, dp_times.deadline,
-		  state_desc())
+		  "adjusted: nb:{:s} target:{:s} ({})", dp_times.not_before,
+		  dp_times.scheduled_at, state_desc())
 	   << dendl;
 }
 
@@ -366,7 +358,6 @@ void ScrubJob::dump(ceph::Formatter* f) const
   f->dump_stream("pgid") << pgid;
   f->dump_stream("sched_time") << get_sched_time();
   f->dump_stream("orig_sched_time") << sch.scheduled_at;
-  f->dump_stream("deadline") << sch.deadline;
   f->dump_bool("forced", entry.urgency >= urgency_t::operator_requested);
 }