crimson/os/seastore: cleanup dependency between epm and journal

OOL segments won't contain deltas and don't need to replay. There is no
need to assign segment sequence from journal.

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 6db0b9b9c7631ae9bb3c889fb0b2a8e27f27a333..217ccdb840003b073b98402c660410a71b99c830 100644 (file)
--- a/src/crimson/os/seastore/extent_placement_manager.cc
+++ b/src/crimson/os/seastore/extent_placement_manager.cc
@@ -17,11 +17,9 @@ namespace crimson::os::seastore {
 
 SegmentedAllocator::SegmentedAllocator(
   SegmentProvider& sp,
-  SegmentManager& sm,
-  Journal& journal)
+  SegmentManager& sm)
   : segment_provider(sp),
-    segment_manager(sm),
-    journal(journal)
+    segment_manager(sm)
 {
   std::generate_n(
     std::back_inserter(writers),
@@ -30,8 +28,7 @@ SegmentedAllocator::SegmentedAllocator(
     [&] {
       return Writer{
        segment_provider,
-       segment_manager,
-       journal};
+       segment_manager};
       });
 }
 
@@ -201,7 +198,7 @@ SegmentedAllocator::Writer::init_segment(Segment& segment) {
       segment_manager.get_block_size()));
   bp.zero();
   auto header =segment_header_t{
-    journal.get_segment_seq(),
+    OOL_SEG_SEQ,
     segment.get_segment_id(),
     NO_DELTAS, 0, true};
   logger().debug("SegmentedAllocator::Writer::init_segment: initting {}, {}",

diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h
index b78e5b28b2eeadf5d48158c17da0de09017bc4fa..b0bbba49fced2ea5282c368961d22bc5fd252386 100644 (file)
--- a/src/crimson/os/seastore/extent_placement_manager.h
+++ b/src/crimson/os/seastore/extent_placement_manager.h
@@ -7,7 +7,6 @@
 
 #include "crimson/common/condition_variable.h"
 #include "crimson/os/seastore/cached_extent.h"
-#include "crimson/os/seastore/journal.h"
 #include "crimson/os/seastore/logging.h"
 #include "crimson/os/seastore/segment_manager.h"
 #include "crimson/os/seastore/transaction.h"
@@ -183,11 +182,9 @@ class SegmentedAllocator : public ExtentAllocator {
   public:
     Writer(
       SegmentProvider& sp,
-      SegmentManager& sm,
-      Journal& journal)
+      SegmentManager& sm)
       : segment_provider(sp),
-        segment_manager(sm),
-        journal(journal)
+        segment_manager(sm)
     {}
     Writer(Writer &&) = default;
 
@@ -225,7 +222,6 @@ class SegmentedAllocator : public ExtentAllocator {
     open_segment_wrapper_ref current_segment;
     std::list<open_segment_wrapper_ref> open_segments;
     seastore_off_t allocated_to = 0;
-    Journal& journal;
     crimson::condition_variable segment_rotation_guard;
     seastar::gate writer_guard;
     bool rolling_segment = false;
@@ -233,8 +229,7 @@ class SegmentedAllocator : public ExtentAllocator {
 public:
   SegmentedAllocator(
     SegmentProvider& sp,
-    SegmentManager& sm,
-    Journal& journal);
+    SegmentManager& sm);
 
   Writer &get_writer(placement_hint_t hint) {
     return writers[std::rand() % writers.size()];
@@ -269,7 +264,6 @@ private:
   SegmentProvider& segment_provider;
   SegmentManager& segment_manager;
   std::vector<Writer> writers;
-  Journal& journal;
 };
 
 class ExtentPlacementManager {

diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc
index 1f3f467eabbe7fa3bfa7049107089cf8081827fa..576790e55b933a527b94d5eb852c929cf015ac69 100644 (file)
--- a/src/crimson/os/seastore/journal.cc
+++ b/src/crimson/os/seastore/journal.cc
@@ -202,18 +202,18 @@ Journal::replay_segment(
              * sequence number > the current journal segment seq. We can
              * safetly skip these deltas because the extent must already
              * have been rewritten.
-             *
-             * Note, this comparison exploits the fact that
-             * SEGMENT_SEQ_NULL is a large number.
              */
-            auto& seg_addr = delta.paddr.as_seg_paddr();
-            if (delta.paddr != P_ADDR_NULL &&
-                (segment_provider->get_seq(seg_addr.get_segment_id()) >
-                 locator.write_result.start_seq.segment_seq)) {
-              return replay_ertr::now();
-            } else {
-              return handler(locator, delta);
+            if (delta.paddr != P_ADDR_NULL) {
+              auto& seg_addr = delta.paddr.as_seg_paddr();
+              auto delta_paddr_segment_seq = segment_provider->get_seq(seg_addr.get_segment_id());
+              auto locator_segment_seq = locator.write_result.start_seq.segment_seq;
+              if (delta_paddr_segment_seq == NULL_SEG_SEQ ||
+                  (delta_paddr_segment_seq <= MAX_VALID_SEG_SEQ &&
+                   delta_paddr_segment_seq > locator_segment_seq)) {
+                return replay_ertr::now();
+              }
             }
+            return handler(locator, delta);
           });
         });
       });

diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h
index 13527de0ea96baad2a0dfbbc756d159c1dc410eb..a9b895fd58bf8fc97a8f29e607dfbb206ba90b48 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.h
+++ b/src/crimson/os/seastore/seastore_types.h
@@ -191,10 +191,11 @@ std::ostream &offset_to_stream(std::ostream &, const seastore_off_t &t);
 /* Monotonically increasing segment seq, uniquely identifies
  * the incarnation of a segment */
 using segment_seq_t = uint32_t;
-static constexpr segment_seq_t NULL_SEG_SEQ =
-  std::numeric_limits<segment_seq_t>::max();
 static constexpr segment_seq_t MAX_SEG_SEQ =
   std::numeric_limits<segment_seq_t>::max();
+static constexpr segment_seq_t NULL_SEG_SEQ = MAX_SEG_SEQ;
+static constexpr segment_seq_t OOL_SEG_SEQ = MAX_SEG_SEQ - 1;
+static constexpr segment_seq_t MAX_VALID_SEG_SEQ = MAX_SEG_SEQ - 2;
 
 // Offset of delta within a record
 using record_delta_idx_t = uint32_t;

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index a7371ae4a3ce6d318c5b4c5589ef947a0129ccba..b9192214af96282c7a5b96d58331c0f565f57054 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -549,8 +549,7 @@ public:
       device_type_t::SEGMENTED,
       std::make_unique<SegmentedAllocator>(
        *segment_cleaner,
-       *sm,
-       *journal));
+       *sm));
   }
 
   ~TransactionManager();

diff --git a/src/crimson/tools/store_nbd/tm_driver.cc b/src/crimson/tools/store_nbd/tm_driver.cc
index 3ac4cccef1616cd920e5e107200a11ad1df4e106..28b070bf598b69fd0799e322e2450fbedc8cce50 100644 (file)
--- a/src/crimson/tools/store_nbd/tm_driver.cc
+++ b/src/crimson/tools/store_nbd/tm_driver.cc
@@ -149,8 +149,7 @@ void TMDriver::init()
     device_type_t::SEGMENTED,
     std::make_unique<SegmentedAllocator>(
       *segment_cleaner,
-      *segment_manager,
-      *journal));
+      *segment_manager));
 
   journal->set_segment_provider(&*segment_cleaner);
 

diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h
index c177eafbf10115e5ad9b825175b18313bb523528..4fdf7895410a3ce875c8a59be032e0336d04157d 100644 (file)
--- a/src/test/crimson/seastore/transaction_manager_test_state.h
+++ b/src/test/crimson/seastore/transaction_manager_test_state.h
@@ -88,8 +88,7 @@ auto get_transaction_manager(
     device_type_t::SEGMENTED,
     std::make_unique<SegmentedAllocator>(
       *segment_cleaner,
-      segment_manager,
-      *journal));
+      segment_manager));
 
   journal->set_segment_provider(&*segment_cleaner);