crimson/os/seastore: rework jorunal::scan_extents to use scan_valid_records

Signed-off-by: Samuel Just <sjust@redhat.com>

diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc
index a7ccb55c847b92e994ee44fb6a10b278fe176b1c..c5e8c2f3f40a5bb4094a95d3cfed00855f502489 100644 (file)
--- a/src/crimson/os/seastore/journal.cc
+++ b/src/crimson/os/seastore/journal.cc
@@ -601,37 +601,51 @@ Journal::replay_ret Journal::replay(delta_handler_t &&delta_handler)
 }
 
 Journal::scan_extents_ret Journal::scan_extents(
-  paddr_t addr,
+  scan_extents_cursor &cursor,
   extent_len_t bytes_to_read)
 {
-  // Caller doesn't know addr of first record, so addr.offset == 0 is special
-  if (addr.offset == 0) addr.offset = block_size;
-
-  return read_segment_header(addr.segment
+  auto ret = std::make_unique<scan_extents_ret_bare>();
+  auto &retref = *ret;
+  return read_segment_header(cursor.get_offset().segment
   ).handle_error(
     scan_extents_ertr::pass_further{},
     crimson::ct_error::assert_all{}
-  ).safe_then([=](auto header) {
+  ).safe_then([&](auto segment_header) {
+    auto segment_nonce = segment_header.segment_nonce;
     return seastar::do_with(
-      scan_extents_ret_bare(),
-      [=](auto &ret) {
-       return seastar::do_with(
-         extent_handler_t([&ret](auto addr, const auto &info) mutable {
-           ret.second.push_back(std::make_pair(addr, info));
-           return scan_extents_ertr::now();
-         }),
-         [=, &ret](auto &handler) mutable {
-           return scan_segment(
-             addr,
-             bytes_to_read,
-             header.segment_nonce,
-             nullptr,
-             &handler).safe_then([&ret](auto next) mutable {
-               ret.first = next;
-               return std::move(ret);
-             });
-         });
+      found_record_handler_t(
+       [&](
+         paddr_t base,
+         const record_header_t &header,
+         const bufferlist &mdbuf) mutable {
+
+         auto infos = try_decode_extent_infos(
+           header,
+           mdbuf);
+         if (!infos) {
+           // This should be impossible, we did check the crc on the mdbuf
+           logger().error(
+             "Journal::scan_extents unable to decode extents for record {}",
+             base);
+           assert(infos);
+         }
+
+         paddr_t extent_offset = base.add_offset(header.mdlength);
+         for (const auto &i : *infos) {
+           retref.emplace_back(extent_offset, i);
+           extent_offset.offset += i.len;
+         }
+         return scan_extents_ertr::now();
+       }),
+      [=, &cursor](auto &dhandler) {
+       return scan_valid_records(
+         cursor,
+         segment_nonce,
+         std::numeric_limits<size_t>::max(),
+         dhandler).safe_then([](auto){});
       });
+  }).safe_then([ret=std::move(ret)] {
+    return std::move(*ret);
   });
 }
 

diff --git a/src/crimson/os/seastore/journal.h b/src/crimson/os/seastore/journal.h
index 5caa380b131487213fc71edf6fc521f05d83fef8..7a65b4500f59b4250b7bf14ce78eee67d071cca9 100644 (file)
--- a/src/crimson/os/seastore/journal.h
+++ b/src/crimson/os/seastore/journal.h
@@ -210,18 +210,16 @@ public:
    * Scans records beginning at addr until the first record boundary after
    * addr + bytes_to_read.
    *
-   * Returns <next_addr, list<extent, extent_info>>
-   * next_addr will be P_ADDR_NULL if no further extents exist in segment.
-   * If addr.offset == 0, scan will adjust to first record in segment.
+   * Returns list<extent, extent_info>
+   * cursor.is_complete() will be true when no further extents exist in segment.
    */
+  class scan_valid_records_cursor;
+  using scan_extents_cursor = scan_valid_records_cursor;
   using scan_extents_ertr = SegmentManager::read_ertr;
-  using scan_extents_ret_bare = std::pair<
-    paddr_t,
-    std::list<std::pair<paddr_t, extent_info_t>>
-    >;
+  using scan_extents_ret_bare = std::list<std::pair<paddr_t, extent_info_t>>;
   using scan_extents_ret = scan_extents_ertr::future<scan_extents_ret_bare>;
   scan_extents_ret scan_extents(
-    paddr_t addr,
+    scan_extents_cursor &cursor,
     extent_len_t bytes_to_read
   );
 

diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc
index a3ed047d7d47486bbb25b74aa77cbcf9a50ca089..3597c21dfc846e2baa9fcad99d5d77412ebd5a97 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.cc
+++ b/src/crimson/os/seastore/segment_cleaner.cc
@@ -273,28 +273,30 @@ SegmentCleaner::do_gc_ret SegmentCleaner::do_gc(
     return do_gc_ertr::now();
   }
 
-  if (gc_current_pos == P_ADDR_NULL) {
-    gc_current_pos.segment = get_next_gc_target();
-    if (gc_current_pos == P_ADDR_NULL) {
-      // apparently there are no segments to gc
+  if (!scan_cursor) {
+    paddr_t next = P_ADDR_NULL;
+    next.segment = get_next_gc_target();
+    if (next == P_ADDR_NULL) {
       logger().debug(
        "SegmentCleaner::do_gc: no segments to gc");
       return do_gc_ertr::now();
     }
+    next.offset = 0;
+    scan_cursor =
+      std::make_unique<ExtentCallbackInterface::scan_extents_cursor>(
+       next);
     logger().debug(
       "SegmentCleaner::do_gc: starting gc on segment {}",
-      gc_current_pos.segment);
-    gc_current_pos.offset = 0;
+      scan_cursor->get_offset().segment);
   }
 
   return ecb->scan_extents(
-    gc_current_pos,
+    *scan_cursor,
     bytes
   ).safe_then([=, &t](auto addrs) {
     return seastar::do_with(
       std::move(addrs),
-      [=, &t](auto &addrs) {
-       auto &[next, addr_list] = addrs;
+      [=, &t](auto &addr_list) {
        return crimson::do_for_each(
          addr_list,
          [=, &t](auto &addr_pair) {
@@ -324,10 +326,10 @@ SegmentCleaner::do_gc_ret SegmentCleaner::do_gc(
                t,
                ext);
            });
-         }).safe_then([next=next, &t, this] {
-           auto old_pos = std::exchange(gc_current_pos, next);
-           if (gc_current_pos == P_ADDR_NULL) {
-             t.mark_segment_to_release(old_pos.segment);
+         }).safe_then([&t, this] {
+           if (scan_cursor->is_complete()) {
+             t.mark_segment_to_release(scan_cursor->get_offset().segment);
+             scan_cursor.reset();
            }
            return ExtentCallbackInterface::release_segment_ertr::now();
          });

diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h
index 51a5636029efd28748c991d0129a46727c4aca60..38ebd05bc21fd6eab37c7ade53b40807b50f71a5 100644 (file)
--- a/src/crimson/os/seastore/segment_cleaner.h
+++ b/src/crimson/os/seastore/segment_cleaner.h
@@ -292,9 +292,11 @@ public:
      *
      * Interface shim for Journal::scan_extents
      */
+    using scan_extents_cursor = Journal::scan_valid_records_cursor;
+    using scan_extents_ertr = Journal::scan_extents_ertr;
     using scan_extents_ret = Journal::scan_extents_ret;
     virtual scan_extents_ret scan_extents(
-      paddr_t addr,
+      scan_extents_cursor &cursor,
       extent_len_t bytes_to_read) = 0;
 
     /**
@@ -518,7 +520,7 @@ private:
   }
 
   // GC status helpers
-  paddr_t gc_current_pos = P_ADDR_NULL;
+  std::unique_ptr<ExtentCallbackInterface::scan_extents_cursor> scan_cursor;
 
   /**
    * do_gc
@@ -547,10 +549,10 @@ private:
    * have been scanned.
    */
   size_t get_bytes_scanned_current_segment() const {
-    if (gc_current_pos == P_ADDR_NULL)
+    if (!scan_cursor)
       return 0;
 
-    return gc_current_pos.offset;
+    return scan_cursor->get_offset().offset;
   }
 
   size_t get_available_bytes() const {

diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h
index 18259a45b3cc89d82bea795472e8ae8afda8218a..f6a9b125413be90310951961d597d0f2c3190e8b 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.h
+++ b/src/crimson/os/seastore/transaction_manager.h
@@ -235,12 +235,16 @@ public:
     laddr_t laddr,
     segment_off_t len) final;
 
+  using scan_extents_cursor =
+    SegmentCleaner::ExtentCallbackInterface::scan_extents_cursor;
+  using scan_extents_ertr =
+    SegmentCleaner::ExtentCallbackInterface::scan_extents_ertr;
   using scan_extents_ret =
     SegmentCleaner::ExtentCallbackInterface::scan_extents_ret;
   scan_extents_ret scan_extents(
-    paddr_t addr,
+    scan_extents_cursor &cursor,
     extent_len_t bytes_to_read) final {
-    return journal.scan_extents(addr, bytes_to_read);
+    return journal.scan_extents(cursor, bytes_to_read);
   }
 
   using release_segment_ret =