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>

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 40dda034daaaf2ed3aa5f4aba1cfd01c46d7e247..6ff1065c4e65e7bc8918a275388263996b6ba6ff 100644 (file)
--- a/src/crimson/os/seastore/async_cleaner.h
+++ b/src/crimson/os/seastore/async_cleaner.h
@@ -414,6 +414,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;
 
@@ -453,7 +459,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;
   }
 };
@@ -508,6 +516,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;
   }
@@ -537,6 +551,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;
@@ -616,6 +631,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 8cdadd097a3c552cf4a33abd3fa163f263cbf5f8..7ee079b0f27c9b837bb58bcf71fed4ea864065b7 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 fa9b7b1b809283107ccbd5ae5127f15d772d177c..a0ff2b2478ee8a96473d8bbc31cf5f5ead969660 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; }