From: Ronen Friedman <rfriedma@redhat.com>
Date: Wed, 26 Feb 2025 15:07:39 +0000 (-0600)
Subject: common/not_before_queue_t: tolerate non-monotonic cut-off values
X-Git-Tag: v20.0.0~10^2~1
X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=7ba63ac29476416c2369f96f5373ff29e1cbac07;p=ceph.git

common/not_before_queue_t: tolerate non-monotonic cut-off values

as the system monotonic clock is used when the container is used
in Scrub implementation, and on some kernels there are rare cases
where the monotonic clock can go backwards, we need to tolerate
such events.

Fixes: https://tracker.ceph.com/issues/70055

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

diff --git a/src/common/not_before_queue.h b/src/common/not_before_queue.h
index 75dd25ae496ca..65d674bb6f1eb 100644
--- a/src/common/not_before_queue.h
+++ b/src/common/not_before_queue.h
@@ -210,11 +210,21 @@ public:
   /**
    * 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 @@ public:
       ineligible_queue.erase(typename ineligible_queue_t::const_iterator(iter));
       eligible_queue.insert(item);
     }
+    return true;
   }
 
   /**
diff --git a/src/test/test_not_before_queue.cc b/src/test/test_not_before_queue.cc
index 2dbb67f4813d2..84506f228e95e 100644
--- a/src/test/test_not_before_queue.cc
+++ b/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));
@@ -237,17 +239,17 @@ TEST_F(NotBeforeTest, RemoveIfByClass_with_cond) {
   // rm from both eligible and non-eligible
   EXPECT_EQ(
       queue.remove_if_by_class(
-	  57, [](const tv_t &v) { return v.ordering_value == 43; }),
+	  57U, [](const tv_t &v) { return v.ordering_value == 43; }),
       3);
   EXPECT_EQ(
       queue.remove_if_by_class(
-	  53, [](const tv_t &v) { return v.ordering_value == 44; }),
+	  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(
-	  57, [](const tv_t &v) { return v.ordering_value > 10; }, 20),
+	  57U, [](const tv_t &v) { return v.ordering_value > 10; }, 20),
       5);
 }
 
@@ -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));