crimson/os/seastore: initialize segment_info_t correctly when open/empty

Previously, segment_info_t was not initialized for out-of-line segments
when open, including journal_segment_seq and the legacy out_of_line.

And the journal_segment_seq was not reset when empty.

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/extent_placement_manager.cc b/src/crimson/os/seastore/extent_placement_manager.cc
index be35bb09b428f3a5887870a709cf1ec958a502c7..8d4f632ab6d30097cd5cd71ce4237158a442fb6d 100644 (file)
--- a/src/crimson/os/seastore/extent_placement_manager.cc
+++ b/src/crimson/os/seastore/extent_placement_manager.cc
@@ -252,7 +252,7 @@ SegmentedAllocator::Writer::roll_segment(bool set_rolling) {
   }
 
   return segment_provider.get_segment(
-    segment_manager.get_device_id()
+    segment_manager.get_device_id(), OOL_SEG_SEQ
   ).safe_then([this](auto segment) {
     return segment_manager.open(segment);
   }).safe_then([this](auto segref) {

diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc
index b742bec0a026eee8e4d3a86b3045d0320abe5e1f..dd2bd28ca5628af9b92c1309b9213d3472c3cb25 100644 (file)
--- a/src/crimson/os/seastore/journal.cc
+++ b/src/crimson/os/seastore/journal.cc
@@ -385,7 +385,8 @@ Journal::JournalSegmentManager::roll()
     current_journal_segment->close() :
     Segment::close_ertr::now()
   ).safe_then([this] {
-    return segment_provider->get_segment(get_device_id());
+    return segment_provider->get_segment(
+        get_device_id(), next_journal_segment_seq);
   }).safe_then([this](auto segment) {
     return segment_manager.open(segment);
   }).safe_then([this](auto sref) {
@@ -395,9 +396,6 @@ Journal::JournalSegmentManager::roll()
     if (old_segment_id != NULL_SEG_ID) {
       segment_provider->close_segment(old_segment_id);
     }
-    segment_provider->set_journal_segment(
-      current_journal_segment->get_segment_id(),
-      get_segment_seq());
   }).handle_error(
     roll_ertr::pass_further{},
     crimson::ct_error::assert_all{

diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc
index e0ccd19026ad5bc4bee0068e6e7e29e8a5c8fc8b..846bb72b24c4c5d060533f388cc22acce1678a5b 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.cc
+++ b/src/crimson/os/seastore/segment_cleaner.cc
@@ -17,14 +17,17 @@ SET_SUBSYS(seastore_cleaner);
 
 namespace crimson::os::seastore {
 
-void segment_info_set_t::segment_info_t::set_open() {
+void segment_info_set_t::segment_info_t::set_open(segment_seq_t seq) {
   assert(state == Segment::segment_state_t::EMPTY);
+  assert(segment_seq_to_type(seq) != segment_type_t::NULL_SEG);
   state = Segment::segment_state_t::OPEN;
+  journal_segment_seq = seq;
 }
 
 void segment_info_set_t::segment_info_t::set_empty() {
   assert(state == Segment::segment_state_t::CLOSED);
   state = Segment::segment_state_t::EMPTY;
+  journal_segment_seq = NULL_SEG_SEQ;
 }
 
 void segment_info_set_t::segment_info_t::set_closed() {
@@ -204,16 +207,19 @@ void SegmentCleaner::register_metrics()
   });
 }
 
-SegmentCleaner::get_segment_ret SegmentCleaner::get_segment(device_id_t id)
+SegmentCleaner::get_segment_ret SegmentCleaner::get_segment(
+    device_id_t id, segment_seq_t seq)
 {
+  assert(segment_seq_to_type(seq) != segment_type_t::NULL_SEG);
   for (auto it = segments.device_begin(id);
        it != segments.device_end(id);
        ++it) {
     auto id = it->first;
     auto& segment_info = it->second;
     if (segment_info.is_empty()) {
-      mark_open(id);
-      logger().debug("{}: returning segment {}", __func__, id);
+      logger().debug("{}: returning segment {} {}",
+                     __func__, id, segment_seq_printer_t{seq});
+      mark_open(id, seq);
       return get_segment_ret(
        get_segment_ertr::ready_future_marker{},
        id);
@@ -262,6 +268,8 @@ void SegmentCleaner::update_journal_tail_committed(journal_seq_t committed)
 
 void SegmentCleaner::close_segment(segment_id_t segment)
 {
+  ceph_assert(segment_seq_to_type(segments[segment].journal_segment_seq) !=
+              segment_type_t::NULL_SEG);
   mark_closed(segment);
 }
 

diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h
index 3d0168dd380968a30f6ddad2c7b2d89a16612bd3..e8b917d7533b5d3e7ad87f447f70ef4876a715da 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.h
+++ b/src/crimson/os/seastore/segment_cleaner.h
@@ -61,7 +61,7 @@ class segment_info_set_t {
       return segment_seq_to_type(journal_segment_seq);
     }
 
-    void set_open();
+    void set_open(segment_seq_t);
     void set_empty();
     void set_closed();
 
@@ -275,15 +275,13 @@ public:
   using get_segment_ertr = crimson::errorator<
     crimson::ct_error::input_output_error>;
   using get_segment_ret = get_segment_ertr::future<segment_id_t>;
-  virtual get_segment_ret get_segment(device_id_t id) = 0;
+  virtual get_segment_ret get_segment(
+      device_id_t id, segment_seq_t seq) = 0;
 
   virtual void close_segment(segment_id_t) {}
 
-  virtual void set_journal_segment(
-    segment_id_t segment,
-    segment_seq_t seq) {}
-
   virtual journal_seq_t get_journal_tail_target() const = 0;
+
   virtual void update_journal_tail_committed(journal_seq_t tail_committed) = 0;
 
   virtual segment_seq_t get_seq(segment_id_t id) { return 0; }
@@ -717,22 +715,11 @@ public:
   using init_segments_ret = init_segments_ertr::future<init_segments_ret_bare>;
   init_segments_ret init_segments();
 
-  get_segment_ret get_segment(device_id_t id) final;
+  get_segment_ret get_segment(
+      device_id_t id, segment_seq_t seq) final;
 
   void close_segment(segment_id_t segment) final;
 
-  void set_journal_segment(
-    segment_id_t segment, segment_seq_t seq) final {
-    assert(segment.device_id() ==
-      segments[segment.device_id()]->device_id);
-    assert(segment.device_segment_id() <
-      segments[segment.device_id()]->num_segments);
-    segments[segment].journal_segment_seq = seq;
-    segments[segment].out_of_line = false;
-    segments.new_journal_segment();
-    assert(segments[segment].is_open());
-  }
-
   journal_seq_t get_journal_tail_target() const final {
     return journal_tail_target;
   }
@@ -1263,6 +1250,7 @@ private:
       "SegmentCleaner::init_mark_segment_closed: segment {}, seq {}",
       segment,
       segment_seq_printer_t{seq});
+    ceph_assert(segment_seq_to_type(seq) != segment_type_t::NULL_SEG);
     mark_closed(segment);
     segments[segment].journal_segment_seq = seq;
     auto s_type = segments[segment].get_type();
@@ -1311,6 +1299,7 @@ private:
       space_tracker->dump_usage(segment);
       assert(space_tracker->get_usage(segment) == 0);
     }
+    auto s_type = segment_info.get_type();
     segment_info.set_empty();
     stats.empty_segments++;
     crimson::get_logger(ceph_subsys_seastore_cleaner
@@ -1323,7 +1312,6 @@ private:
        should_block_on_gc(),
        get_projected_available_ratio(),
        get_projected_reclaim_ratio());
-    auto s_type = segment_info.get_type();
     ceph_assert(s_type != segment_type_t::NULL_SEG);
     if (s_type == segment_type_t::JOURNAL) {
       segments.journal_segment_emptied();
@@ -1331,7 +1319,7 @@ private:
     maybe_wake_gc_blocked_io();
   }
 
-  void mark_open(segment_id_t segment) {
+  void mark_open(segment_id_t segment, segment_seq_t seq) {
     assert(segment.device_id() ==
       segments[segment.device_id()]->device_id);
     assert(segment.device_segment_id() <
@@ -1339,14 +1327,22 @@ private:
     assert(segments[segment].is_empty());
     assert(segments.get_empty_segments(segment.device_id()) > 0);
     segments.segment_opened(segment);
-    segments[segment].set_open();
+    auto& segment_info = segments[segment];
+    segment_info.set_open(seq);
+
+    auto s_type = segment_info.get_type();
+    ceph_assert(s_type != segment_type_t::NULL_SEG);
+    if (s_type == segment_type_t::JOURNAL) {
+      segments.new_journal_segment();
+    }
     assert(stats.empty_segments > 0);
     stats.empty_segments--;
     crimson::get_logger(ceph_subsys_seastore_cleaner
-      ).info("mark open: {}, empty_segments {}"
+      ).info("mark open: {} {}, empty_segments {}"
        ", opened_segments {}, should_block_on_gc {}"
        ", projected_avail_ratio {}, projected_reclaim_ratio {}",
        segment,
+       segment_seq_printer_t{seq},
        stats.empty_segments,
        segments.get_opened_segments(),
        should_block_on_gc(),

diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc
index 7bf2e01a4ad385e9493efd63c7e5f96c099b7f60..9e2fe06692cbf048df641a82f8dc02318d4864cf 100644 (file)
--- a/src/test/crimson/seastore/test_btree_lba_manager.cc
+++ b/src/test/crimson/seastore/test_btree_lba_manager.cc
@@ -43,7 +43,7 @@ struct btree_test_base :
 
   void update_segment_avail_bytes(paddr_t offset) final {}
 
-  get_segment_ret get_segment(device_id_t id) final {
+  get_segment_ret get_segment(device_id_t id, segment_seq_t seq) final {
     auto ret = next;
     next = segment_id_t{
       next.device_id(),

diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc
index 9fc4421c4065ad33ace52c4a44eb7c158ea3b4d9..f1271e2c8229fbc2fbe8c81f6ec3a9690992b586 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_journal.cc
+++ b/src/test/crimson/seastore/test_seastore_journal.cc
@@ -84,7 +84,7 @@ struct journal_test_t : seastar_test_suite_t, SegmentProvider {
 
   void update_segment_avail_bytes(paddr_t offset) final {}
 
-  get_segment_ret get_segment(device_id_t id) final {
+  get_segment_ret get_segment(device_id_t id, segment_seq_t seq) final {
     auto ret = next;
     next = segment_id_t{
       next.device_id(),