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 <[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/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));