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 75dd25ae496ca7e82672c3859d41b92cdb385b65..65d674bb6f1eb0b16b85bbb2e995c1317acf25bb 100644 (file)
--- 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 2dbb67f4813d2dbd86184105cce104d17a063172..84506f228e95eb2051f9f20bb20a9160b2278634 100644 (file)
--- 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));