crimson/os/seastore: be explicit about detecting out-dated delta using segment info

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

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index e1678dc7f0986e0abdf11b62dbc56e3c242ef805..107574cb7d1c88242dd45a317704e1893d233dc2 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -1102,13 +1102,16 @@ record_t Cache::prepare_record(
     } else {
       auto sseq = NULL_SEG_SEQ;
       auto stype = segment_type_t::NULL_SEG;
-      if (cleaner != nullptr && i->get_paddr().get_addr_type() ==
-         paddr_types_t::SEGMENT) {
+
+      // FIXME: This is specific to the segmented implementation
+      if (segment_provider != nullptr &&
+          i->get_paddr().get_addr_type() == paddr_types_t::SEGMENT) {
         auto sid = i->get_paddr().as_seg_paddr().get_segment_id();
-        auto &sinfo = cleaner->get_seg_info(sid);
+        auto &sinfo = segment_provider->get_seg_info(sid);
         sseq = sinfo.seq;
         stype = sinfo.type;
       }
+
       record.push_back(
        delta_info_t{
          i->get_type(),
@@ -1636,6 +1639,32 @@ Cache::replay_delta(
   assert(alloc_tail != JOURNAL_SEQ_NULL);
   ceph_assert(modify_time != NULL_TIME);
 
+  // FIXME: This is specific to the segmented implementation
+  /* The journal may validly contain deltas for extents in
+   * since released segments.  We can detect those cases by
+   * checking whether the segment in question currently has a
+   * sequence number > the current journal segment seq. We can
+   * safetly skip these deltas because the extent must already
+   * have been rewritten.
+   */
+  if (segment_provider != nullptr &&
+      delta.paddr != P_ADDR_NULL &&
+      delta.paddr.get_addr_type() == paddr_types_t::SEGMENT) {
+    auto& seg_addr = delta.paddr.as_seg_paddr();
+    auto& seg_info = segment_provider->get_seg_info(seg_addr.get_segment_id());
+    auto delta_paddr_segment_seq = seg_info.seq;
+    auto delta_paddr_segment_type = seg_info.type;
+    if (delta_paddr_segment_seq != delta.ext_seq ||
+        delta_paddr_segment_type != delta.seg_type) {
+      DEBUG("delta is obsolete, delta_paddr_segment_seq={},"
+            " delta_paddr_segment_type={} -- {}",
+            segment_seq_printer_t{delta_paddr_segment_seq},
+            delta_paddr_segment_type,
+            delta);
+      return replay_delta_ertr::make_ready_future<bool>(false);
+    }
+  }
+
   if (delta.type == extent_types_t::JOURNAL_TAIL) {
     // this delta should have been dealt with during segment cleaner mounting
     return replay_delta_ertr::make_ready_future<bool>(false);

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 4a4a280bcde7231c92e83c6a642a9d7bc6c12264..c57a0b8e96c341b88fb4e35ee071221387f50541 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -27,6 +27,7 @@ namespace crimson::os::seastore {
 
 class BackrefManager;
 class AsyncCleaner;
+class SegmentProvider;
 
 struct backref_entry_t {
   backref_entry_t(
@@ -679,6 +680,18 @@ public:
     CachedExtentRef i  ///< [in] ref to existing extent
   );
 
+  /**
+   * set_segment_provider
+   *
+   * Set to provide segment information to help identify out-dated delta.
+   *
+   * FIXME: This is specific to the segmented implementation
+   */
+  void set_segment_provider(SegmentProvider &sp) {
+    assert(segment_provider == nullptr);
+    segment_provider = &sp;
+  }
+
   /**
    * prepare_record
    *
@@ -968,6 +981,9 @@ private:
 
   journal_seq_t last_commit = JOURNAL_SEQ_MIN;
 
+  // FIXME: This is specific to the segmented implementation
+  SegmentProvider *segment_provider = nullptr;
+
   /**
    * dirty
    *

diff --git a/src/crimson/os/seastore/journal/segmented_journal.cc b/src/crimson/os/seastore/journal/segmented_journal.cc
index 82eadf80fbae0ca27a820e711837f618e9e3df99..19008138df4ba72bbf4875b0d1b507cd70eedf10 100644 (file)
--- a/src/crimson/os/seastore/journal/segmented_journal.cc
+++ b/src/crimson/os/seastore/journal/segmented_journal.cc
@@ -234,7 +234,7 @@ SegmentedJournal::replay_segment(
   return seastar::do_with(
     scan_valid_records_cursor(seq),
     SegmentManagerGroup::found_record_handler_t(
-      [s_type=header.type, &handler, this, &stats](
+      [&handler, this, &stats](
       record_locator_t locator,
       const record_group_header_t& header,
       const bufferlist& mdbuf)
@@ -254,7 +254,6 @@ SegmentedJournal::replay_segment(
       return seastar::do_with(
         std::move(*maybe_record_deltas_list),
         [write_result=locator.write_result,
-        s_type,
          this,
          FNAME,
          &handler,
@@ -263,7 +262,6 @@ SegmentedJournal::replay_segment(
         return crimson::do_for_each(
           record_deltas_list,
           [write_result,
-          s_type,
            this,
            FNAME,
            &handler,
@@ -280,38 +278,12 @@ SegmentedJournal::replay_segment(
           return crimson::do_for_each(
             record_deltas.deltas,
             [locator,
-            s_type,
              this,
-             FNAME,
              &handler,
              &stats](auto &p)
           {
            auto& modify_time = p.first;
            auto& delta = p.second;
-            /* The journal may validly contain deltas for extents in
-             * since released segments.  We can detect those cases by
-             * checking whether the segment in question currently has a
-             * sequence number > the current journal segment seq. We can
-             * safetly skip these deltas because the extent must already
-             * have been rewritten.
-             */
-            if (delta.paddr != P_ADDR_NULL) {
-              auto& seg_addr = delta.paddr.as_seg_paddr();
-              auto& seg_info = segment_provider.get_seg_info(seg_addr.get_segment_id());
-              auto delta_paddr_segment_seq = seg_info.seq;
-             auto delta_paddr_segment_type = seg_info.type;
-              if (s_type == segment_type_t::NULL_SEG ||
-                  (delta_paddr_segment_seq != delta.ext_seq ||
-                  delta_paddr_segment_type != delta.seg_type)) {
-                SUBDEBUG(seastore_cache,
-                         "delta is obsolete, delta_paddr_segment_seq={},"
-                        " delta_paddr_segment_type={} -- {}",
-                         segment_seq_printer_t{delta_paddr_segment_seq},
-                        delta_paddr_segment_type,
-                         delta);
-                return replay_ertr::now();
-              }
-            }
            return handler(
              locator,
              delta,

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 806e646daff8afc0e299ced8f91b5052de3da2fc..d97c30a45395154c0a385a9244b92f4c0009544f 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -654,6 +654,10 @@ TransactionManagerRef make_transaction_manager(
     *backref_manager,
     cleaner_is_detailed);
 
+  if (primary_device->get_device_type() == device_type_t::SEGMENTED) {
+    cache->set_segment_provider(*async_cleaner);
+  }
+
   auto p_device_type = primary_device->get_device_type();
   JournalRef journal;
   if (p_device_type == device_type_t::SEGMENTED) {