Merge pull request ceph#62010 from ronen-fr/wip-rf-monotime

osd/scrub: tolerate non-monotonic scrub queue cutoff updates

Reviewed-by: Radoslaw Zarzynski <[email protected]>

Author: ronen-fr

src/common/not_before_queue.h

@@ -210,11 +210,21 @@ class not_before_queue_t {
   /**
    * advance_time
    *
-   * Advances the eligibility cutoff, argument must be non-decreasing in
-   * successive calls.
+   * Advances the eligibility cutoff.
+   * Argument should be non-decreasing in successive calls.
+   *
+   * As for the scrub queue the cutoff is a time value, and as we have
+   * encountered rare cases of clock resets (even the monotonic clock
+   * can be slightly adjusted backwards on some kernels), "backward"
+   * updates will be tolerated - ignored and not assert.
+   *
+   * \retval: true if the cutoff was advanced. False if we
+   *          had to ignore the update.
    */
-  void advance_time(T next_time) {
-    assert(next_time >= current_time);
+  bool advance_time(T next_time) {
+    if (next_time < current_time) {
+      return false;
+    }
     current_time = next_time;
     while (true) {
       if (ineligible_queue.empty()) {
@@ -233,6 +243,7 @@ class not_before_queue_t {
       ineligible_queue.erase(typename ineligible_queue_t::const_iterator(iter));
       eligible_queue.insert(item);
     }
+    return true;
   }
 
   /**

src/osd/scrubber/osd_scrub_sched.cc

@@ -92,7 +92,15 @@ std::optional<Scrub::SchedEntry> ScrubQueue::pop_ready_entry(
   };
 
   std::unique_lock lck{jobs_lock};
-  to_scrub.advance_time(time_now);
+  if (!to_scrub.advance_time(time_now)) {
+    // the clock was not advanced
+    dout(5) << fmt::format(
+		   ": time now ({}) is earlier than the previous not-before "
+		   "cut-off time",
+		   time_now)
+	    << dendl;
+    // we still try to dequeue, mainly to handle possible corner cases
+  }
   return to_scrub.dequeue_by_pred(eligible_filtr);
 }
 

src/test/test_not_before_queue.cc

@@ -111,7 +111,9 @@ TEST_F(NotBeforeTest, NotBefore) {
   ASSERT_EQ(queue.dequeue(), std::make_optional(e5));
   ASSERT_EQ(queue.dequeue(), std::nullopt);
 
-  queue.advance_time(1);
+  EXPECT_TRUE(queue.advance_time(1U));
+  // a 2'nd call using earlier time - should fail
+  EXPECT_FALSE(queue.advance_time(0U));
 
   EXPECT_EQ(queue.dequeue(), std::make_optional(e1));
   EXPECT_EQ(queue.dequeue(), std::make_optional(e2));
@@ -244,7 +246,7 @@ TEST_F(NotBeforeTest, RemoveIfByClass_with_cond) {
 	  53U, [](const tv_t &v) { return v.ordering_value == 44; }),
       2);
 
-  ASSERT_EQ(queue.total_count(), 17);
+  ASSERT_EQ(queue.total_count(), 17U);
   EXPECT_EQ(
       queue.remove_if_by_class(
 	  57U, [](const tv_t &v) { return v.ordering_value > 10; }, 20),
@@ -294,14 +296,15 @@ TEST_F(NotBeforeTest, accumulate_1) {
   EXPECT_EQ(res_all, "abcdefgh");
 
   // set time to 20: the order changes: 2, 4, 1, 3
-  queue.advance_time(20);
+  EXPECT_TRUE(queue.advance_time(20));
+  EXPECT_FALSE(queue.advance_time(18));
   acc_just_elig = acc_just_elig_templ;
   res = queue.accumulate<std::string, decltype(acc_just_elig)>(
       std::move(acc_just_elig));
   EXPECT_EQ(res, "jpor");
 
   // at 35: 2, 4, 1, 7, 8, 3
-  queue.advance_time(35);
+  EXPECT_TRUE(queue.advance_time(35));
   acc_just_elig = acc_just_elig_templ;
   res = queue.accumulate<std::string, decltype(acc_just_elig)>(
       std::move(acc_just_elig));