crimson/os/seastore/async_cleaner: fix incorrect get_num_rolls()

The number of journal segments should not be based on the committed
journal_head. Otherwise, if a new journal segment is just opened and the
committed journal_head hasn't been updated, the result will be wrong.

This causes ceph_assert(get_segments_reclaimable() == 0) in
SegmentCleaner::get_next_reclaim_segment().

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>
(cherry picked from commit 5ae59cf52159bdc1ad3637da8654524cb7b4b4b6)

diff --git a/src/crimson/os/seastore/async_cleaner.cc b/src/crimson/os/seastore/async_cleaner.cc
index aa6fcd8a95d6c08086b70347d90b0079f75e92af..a9d22b1269caa96b8e0bea2a8e3da27f62a3ce51 100644 (file)
--- a/src/crimson/os/seastore/async_cleaner.cc
+++ b/src/crimson/os/seastore/async_cleaner.cc
@@ -443,6 +443,18 @@ void JournalTrimmerImpl::set_journal_head(journal_seq_t head)
   background_callback->maybe_wake_background();
 }
 
+void JournalTrimmerImpl::set_journal_head_sequence(
+  segment_seq_t seq)
+{
+  if (journal_head != JOURNAL_SEQ_NULL) {
+    ceph_assert(seq == journal_head.segment_seq + 1);
+  }
+  if (journal_head_seq != NULL_SEG_SEQ) {
+    ceph_assert(seq == journal_head_seq + 1);
+  }
+  journal_head_seq = seq;
+}
+
 void JournalTrimmerImpl::update_journal_tails(
   journal_seq_t dirty_tail,
   journal_seq_t alloc_tail)
@@ -976,6 +988,10 @@ segment_id_t SegmentCleaner::allocate_segment(
     if (segment_info.is_empty()) {
       auto old_usage = calc_utilization(seg_id);
       segments.mark_open(seg_id, seq, type, category, generation);
+      if (type == segment_type_t::JOURNAL) {
+        assert(trimmer != nullptr);
+        trimmer->set_journal_head_sequence(seq);
+      }
       background_callback->maybe_wake_background();
       auto new_usage = calc_utilization(seg_id);
       adjust_segment_util(old_usage, new_usage);

diff --git a/src/crimson/os/seastore/async_cleaner.h b/src/crimson/os/seastore/async_cleaner.h
index 1c94745d820c2196999c183678e12c8deea7d105..2f8ae53022e862700612d0c8e3e42a3aca648911 100644 (file)
--- a/src/crimson/os/seastore/async_cleaner.h
+++ b/src/crimson/os/seastore/async_cleaner.h
@@ -416,6 +416,12 @@ public:
   // set the committed journal head
   virtual void set_journal_head(journal_seq_t) = 0;
 
+  // get the opened journal head sequence
+  virtual segment_seq_t get_journal_head_sequence() const = 0;
+
+  // set the opened journal head sequence
+  virtual void set_journal_head_sequence(segment_seq_t) = 0;
+
   // get the committed journal dirty tail
   virtual journal_seq_t get_dirty_tail() const = 0;
 
@@ -455,7 +461,9 @@ public:
     }
     assert(get_journal_head().segment_seq >=
            get_journal_tail().segment_seq);
-    return get_journal_head().segment_seq + 1 -
+    assert(get_journal_head_sequence() >=
+           get_journal_head().segment_seq);
+    return get_journal_head_sequence() + 1 -
            get_journal_tail().segment_seq;
   }
 };
@@ -510,6 +518,12 @@ public:
 
   void set_journal_head(journal_seq_t) final;
 
+  segment_seq_t get_journal_head_sequence() const final {
+    return journal_head_seq;
+  }
+
+  void set_journal_head_sequence(segment_seq_t) final;
+
   journal_seq_t get_dirty_tail() const final {
     return journal_dirty_tail;
   }
@@ -539,6 +553,7 @@ public:
   }
 
   void reset() {
+    journal_head_seq = NULL_SEG_SEQ;
     journal_head = JOURNAL_SEQ_NULL;
     journal_dirty_tail = JOURNAL_SEQ_NULL;
     journal_alloc_tail = JOURNAL_SEQ_NULL;
@@ -618,6 +633,7 @@ private:
   device_off_t roll_start;
   device_off_t roll_size;
 
+  segment_seq_t journal_head_seq = NULL_SEG_SEQ;
   journal_seq_t journal_head;
   journal_seq_t journal_dirty_tail;
   journal_seq_t journal_alloc_tail;

diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc
index 977b8c386506946cc23acbdf0743be9ba2f5cefa..d584d6c22b68d6d1dd6f8516eca11e465e59e0a0 100644 (file)
--- a/src/test/crimson/seastore/test_btree_lba_manager.cc
+++ b/src/test/crimson/seastore/test_btree_lba_manager.cc
@@ -55,6 +55,12 @@ struct btree_test_base :
 
   void set_journal_head(journal_seq_t) final {}
 
+  segment_seq_t get_journal_head_sequence() const final {
+    return NULL_SEG_SEQ;
+  }
+
+  void set_journal_head_sequence(segment_seq_t) final {}
+
   journal_seq_t get_dirty_tail() const final { return dummy_tail; }
 
   journal_seq_t get_alloc_tail() const final { return dummy_tail; }

diff --git a/src/test/crimson/seastore/test_cbjournal.cc b/src/test/crimson/seastore/test_cbjournal.cc
index bacb3cd2f78ec8044ba4ea0aefbb90d1a3c053af..3c7007a5024d820dbee8c979cf318d39d9edeb69 100644 (file)
--- a/src/test/crimson/seastore/test_cbjournal.cc
+++ b/src/test/crimson/seastore/test_cbjournal.cc
@@ -147,10 +147,14 @@ struct cbjournal_test_t : public seastar_test_suite_t, JournalTrimmer
   /*
    * JournalTrimmer interfaces
    */
-  journal_seq_t get_journal_head() const {
+  journal_seq_t get_journal_head() const final {
     return JOURNAL_SEQ_NULL;
   }
 
+  segment_seq_t get_journal_head_sequence() const final {
+    return NULL_SEG_SEQ;
+  }
+
   journal_seq_t get_dirty_tail() const final {
     return JOURNAL_SEQ_NULL;
   }
@@ -161,6 +165,8 @@ struct cbjournal_test_t : public seastar_test_suite_t, JournalTrimmer
 
   void set_journal_head(journal_seq_t head) final {}
 
+  void set_journal_head_sequence(segment_seq_t) final {}
+
   void update_journal_tails(
     journal_seq_t dirty_tail,
     journal_seq_t alloc_tail) final {}

diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc
index ddd894349d21e6e88ad3dd16fd1e2b0bc371e704..e275213047b6c2fffeb27c198fd591487ea5b72d 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_journal.cc
+++ b/src/test/crimson/seastore/test_seastore_journal.cc
@@ -94,6 +94,12 @@ struct journal_test_t : seastar_test_suite_t, SegmentProvider, JournalTrimmer {
 
   void set_journal_head(journal_seq_t) final {}
 
+  segment_seq_t get_journal_head_sequence() const final {
+    return NULL_SEG_SEQ;
+  }
+
+  void set_journal_head_sequence(segment_seq_t) final {}
+
   journal_seq_t get_dirty_tail() const final { return dummy_tail; }
 
   journal_seq_t get_alloc_tail() const final { return dummy_tail; }