crimson/os/seastore/journal: return addr and seq from submit_record

Reworks journal_seq_t to be a monotonically increasing, but non-dense
value from the current segment_seq_t and segment offset.

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

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 8df0478ff8dc226c43a6321b38d59d3d173d285b..264a8f24ec919659343a4a9432aa803858869198 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -223,7 +223,8 @@ std::optional<record_t> Cache::try_construct_record(Transaction &t)
 
 void Cache::complete_commit(
   Transaction &t,
-  paddr_t final_block_start)
+  paddr_t final_block_start,
+  journal_seq_t seq)
 {
   if (t.root) {
     remove_extent(root);

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 9ec64611c365a48788bdb05f2718fa009a4eec82..46ea4f453013ec87b5a405ceef323b60d736c6bd 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -281,8 +281,9 @@ public:
    * and mutated exents.
    */
   void complete_commit(
-    Transaction &t,           ///< [in, out] current transaction
-    paddr_t final_block_start ///< [in] offset of initial block
+    Transaction &t,            ///< [in, out] current transaction
+    paddr_t final_block_start, ///< [in] offset of initial block
+    journal_seq_t seq          ///< [in] journal commit seq
   );
 
   /**

diff --git a/src/crimson/os/seastore/journal.cc b/src/crimson/os/seastore/journal.cc
index fe2594c3119835e4fe7749a2618420aebec715b2..098fb80b2a62eb22d47449522bcc306b89fb2740 100644 (file)
--- a/src/crimson/os/seastore/journal.cc
+++ b/src/crimson/os/seastore/journal.cc
@@ -59,7 +59,6 @@ ceph::bufferlist Journal::encode_record(
   record_header_t header{
     rsize.mdlength,
     rsize.dlength,
-    current_journal_seq,
     0 /* checksum, TODO */,
     record.deltas.size(),
     record.extents.size()
@@ -85,7 +84,7 @@ ceph::bufferlist Journal::encode_record(
   return metadatabl;
 }
 
-Journal::write_record_ertr::future<> Journal::write_record(
+Journal::write_record_ret Journal::write_record(
   record_size_t rsize,
   record_t &&record)
 {
@@ -99,7 +98,13 @@ Journal::write_record_ertr::future<> Journal::write_record(
     rsize.dlength);
   return current_journal_segment->write(target, to_write).handle_error(
     write_record_ertr::pass_further{},
-    crimson::ct_error::assert_all{ "TODO" });
+    crimson::ct_error::assert_all{ "TODO" }).safe_then([this, target] {
+      return write_record_ret(
+       write_record_ertr::ready_future_marker{},
+       paddr_t{
+         current_journal_segment->get_segment_id(),
+         target});
+    });
 }
 
 Journal::record_size_t Journal::get_encoded_record_length(
@@ -123,13 +128,6 @@ bool Journal::needs_roll(segment_off_t length) const
     current_journal_segment->get_write_capacity();
 }
 
-paddr_t Journal::next_record_addr() const
-{
-  paddr_t ret{current_journal_segment->get_segment_id(), written_to};
-  logger().debug("next_record_addr: {}", ret);
-  return ret;
-}
-
 Journal::roll_journal_segment_ertr::future<>
 Journal::roll_journal_segment()
 {

diff --git a/src/crimson/os/seastore/journal.h b/src/crimson/os/seastore/journal.h
index ab65f5f29e557b4b8e6b1bb941d46509f9424feb..6d3b3cfee3f4ee9ad0ad4b896607fa39f24f8e53 100644 (file)
--- a/src/crimson/os/seastore/journal.h
+++ b/src/crimson/os/seastore/journal.h
@@ -19,10 +19,6 @@
 
 namespace crimson::os::seastore {
 
-using journal_seq_t = uint64_t;
-static constexpr journal_seq_t NO_DELTAS =
-  std::numeric_limits<journal_seq_t>::max();
-
 /**
  * Segment header
  *
@@ -50,7 +46,6 @@ struct record_header_t {
   // Fixed portion
   extent_len_t  mdlength;       // block aligned, length of metadata
   extent_len_t  dlength;        // block aligned, length of data
-  journal_seq_t seq;            // current journal seqid
   checksum_t    full_checksum;  // checksum for full record (TODO)
   size_t deltas;                // number of deltas
   size_t extents;               // number of extents
@@ -59,7 +54,6 @@ struct record_header_t {
     DENC_START(1, 1, p);
     denc(v.mdlength, p);
     denc(v.dlength, p);
-    denc(v.seq, p);
     denc(v.full_checksum, p);
     denc(v.deltas, p);
     denc(v.extents, p);
@@ -126,15 +120,17 @@ public:
   close_ertr::future<> close() { return close_ertr::now(); }
 
   /**
-   * write_record
+   * submit_record
    *
-   * @param write record and returns offset of first block
+   * @param write record and returns offset of first block and seq
    */
   using submit_record_ertr = crimson::errorator<
     crimson::ct_error::erange,
     crimson::ct_error::input_output_error
     >;
-  using submit_record_ret = submit_record_ertr::future<paddr_t>;
+  using submit_record_ret = submit_record_ertr::future<
+    std::pair<paddr_t, journal_seq_t>
+    >;
   submit_record_ret submit_record(record_t &&record) {
     auto rsize = get_encoded_record_length(record);
     auto total = rsize.mdlength + rsize.dlength;
@@ -146,10 +142,11 @@ public:
       : roll_journal_segment_ertr::now();
     return roll.safe_then(
       [this, rsize, record=std::move(record)]() mutable {
-       auto ret = next_record_addr();
        return write_record(rsize, std::move(record)
-       ).safe_then([this, ret] {
-         return ret.add_offset(block_size);
+       ).safe_then([this](auto addr) {
+         return std::make_pair(
+           addr.add_offset(block_size),
+           get_journal_seq(addr));
        });
       });
   }
@@ -179,7 +176,10 @@ private:
   segment_off_t written_to = 0;
 
   segment_id_t next_journal_segment_seq = NULL_SEG_ID;
-  journal_seq_t current_journal_seq = 0;
+
+  journal_seq_t get_journal_seq(paddr_t addr) {
+    return journal_seq_t{current_journal_segment_seq, addr};
+  }
 
   /// prepare segment for writes, writes out segment header
   using initialize_segment_ertr = crimson::errorator<
@@ -212,7 +212,8 @@ private:
   /// do record write
   using write_record_ertr = crimson::errorator<
     crimson::ct_error::input_output_error>;
-  write_record_ertr::future<> write_record(
+  using write_record_ret = write_record_ertr::future<paddr_t>;
+  write_record_ret write_record(
     record_size_t rsize,
     record_t &&record);
 
@@ -224,9 +225,6 @@ private:
   /// returns true iff current segment has insufficient space
   bool needs_roll(segment_off_t length) const;
 
-  /// returns next record addr
-  paddr_t next_record_addr() const;
-
   /// return ordered vector of segments to replay
   using find_replay_segments_ertr = crimson::errorator<
     crimson::ct_error::input_output_error

diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc
index 5e65c8d827a1c9c74e8e57771eb178c74e17e013..9c20fc3f0e8d689c3f5c2f70bd3bc51ee673b315 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.cc
+++ b/src/crimson/os/seastore/seastore_types.cc
@@ -36,6 +36,14 @@ std::ostream &operator<<(std::ostream &out, const paddr_t &rhs)
   return out << ">";
 }
 
+std::ostream &operator<<(std::ostream &out, const journal_seq_t &seq)
+{
+  return out << "journal_seq_t(segment_seq="
+            << seq.segment_seq << ", offset="
+            << seq.offset
+            << ")";
+}
+
 std::ostream &operator<<(std::ostream &out, extent_types_t t)
 {
   switch (t) {

diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h
index 2bb5a4939e1cdb53cf92fdd71a098a7f0754eba1..a17e47aac5ab84671b77d99868a866ece2379c08 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.h
+++ b/src/crimson/os/seastore/seastore_types.h
@@ -181,6 +181,30 @@ std::ostream &operator<<(std::ostream &out, const paddr_t &rhs);
 using objaddr_t = uint32_t;
 constexpr objaddr_t OBJ_ADDR_MIN = std::numeric_limits<objaddr_t>::min();
 
+/* Monotonically increasing identifier for the location of a
+ * journal_record.
+ */
+struct journal_seq_t {
+  segment_seq_t segment_seq = 0;
+  paddr_t offset;
+
+  DENC(journal_seq_t, v, p) {
+    DENC_START(1, 1, p);
+    denc(v.segment_seq, p);
+    denc(v.offset, p);
+    DENC_FINISH(p);
+  }
+};
+WRITE_CMP_OPERATORS_2(journal_seq_t, segment_seq, offset)
+WRITE_EQ_OPERATORS_2(journal_seq_t, segment_seq, offset)
+
+std::ostream &operator<<(std::ostream &out, const journal_seq_t &seq);
+
+static constexpr journal_seq_t NO_DELTAS = journal_seq_t{
+  NULL_SEG_SEQ,
+  P_ADDR_NULL
+};
+
 // logical addr, see LBAManager, TransactionManager
 using laddr_t = uint64_t;
 constexpr laddr_t L_ADDR_MIN = std::numeric_limits<laddr_t>::min();
@@ -314,4 +338,5 @@ struct record_t {
 }
 
 WRITE_CLASS_DENC_BOUNDED(crimson::os::seastore::paddr_t)
+WRITE_CLASS_DENC_BOUNDED(crimson::os::seastore::journal_seq_t)
 WRITE_CLASS_DENC_BOUNDED(crimson::os::seastore::delta_info_t)

diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc
index 8b035768106fb1ab610fe77a96c94d08b7807588..cf56154f35f05705a942313469aad7e1aa00c894 100644 (file)
--- a/src/crimson/os/seastore/transaction_manager.cc
+++ b/src/crimson/os/seastore/transaction_manager.cc
@@ -164,8 +164,9 @@ TransactionManager::submit_transaction(
   logger().debug("TransactionManager::submit_transaction");
 
   return journal.submit_record(std::move(*record)).safe_then(
-    [this, t=std::move(t)](paddr_t addr) mutable {
-      cache.complete_commit(*t, addr);
+    [this, t=std::move(t)](auto p) mutable {
+      auto [addr, journal_seq] = p;
+      cache.complete_commit(*t, addr, journal_seq);
       lba_manager.complete_transaction(*t);
     },
     submit_transaction_ertr::pass_further{},

diff --git a/src/test/crimson/seastore/test_btree_lba_manager.cc b/src/test/crimson/seastore/test_btree_lba_manager.cc
index 71ddbb29c8754a2a7ecf290e7784c1a479f19c78..28059c4c90138533f130f12b26097d52e3a7751a 100644 (file)
--- a/src/test/crimson/seastore/test_btree_lba_manager.cc
+++ b/src/test/crimson/seastore/test_btree_lba_manager.cc
@@ -65,8 +65,9 @@ struct btree_lba_manager_test :
     }
 
     return journal.submit_record(std::move(*record)).safe_then(
-      [this, t=std::move(t)](paddr_t addr) mutable {
-       cache.complete_commit(*t, addr);
+      [this, t=std::move(t)](auto p) mutable {
+       auto [addr, seq] = p;
+       cache.complete_commit(*t, addr, seq);
        lba_manager->complete_transaction(*t);
       },
       crimson::ct_error::all_same_way([](auto e) {

diff --git a/src/test/crimson/seastore/test_seastore_cache.cc b/src/test/crimson/seastore/test_seastore_cache.cc
index f480cd59ef8140a2c8a169b8d5106d780088af45..e6b0fa7f393f1874339d8b7833ef2379322b67aa 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_cache.cc
+++ b/src/test/crimson/seastore/test_seastore_cache.cc
@@ -23,6 +23,7 @@ struct cache_test_t : public seastar_test_suite_t {
   segment_manager::EphemeralSegmentManager segment_manager;
   Cache cache;
   paddr_t current{0, 0};
+  journal_seq_t seq;
 
   cache_test_t()
     : segment_manager(segment_manager::DEFAULT_TEST_EPHEMERAL),
@@ -55,7 +56,7 @@ struct cache_test_t : public seastar_test_suite_t {
       true
     ).safe_then(
       [this, prev, t=std::move(t)]() mutable {
-       cache.complete_commit(*t, prev);
+       cache.complete_commit(*t, prev, seq /* TODO */);
        return seastar::make_ready_future<std::optional<paddr_t>>(prev);
       },
       crimson::ct_error::all_same_way([](auto e) {

diff --git a/src/test/crimson/seastore/test_seastore_journal.cc b/src/test/crimson/seastore/test_seastore_journal.cc
index ef5c1efa44b160ef1f282b73a437d418ffaa0282..2529f3753fea010b597ef2136b369862cd8b735b 100644 (file)
--- a/src/test/crimson/seastore/test_seastore_journal.cc
+++ b/src/test/crimson/seastore/test_seastore_journal.cc
@@ -154,7 +154,7 @@ struct journal_test_t : seastar_test_suite_t, JournalSegmentProvider {
   auto submit_record(T&&... _record) {
     auto record{std::forward<T>(_record)...};
     records.push_back(record);
-    auto addr = journal->submit_record(std::move(record)).unsafe_get0();
+    auto [addr, _] = journal->submit_record(std::move(record)).unsafe_get0();
     records.back().record_final_offset = addr;
     return addr;
   }