Merge pull request ceph#58250 from cyx1231st/wip-seastore-optimize-ool-padding

crimson/os/seastore: write ool extents without padding

Reviewed-by: Samuel Just <[email protected]>

Author: cyx1231st

src/crimson/os/seastore/cache.cc

@@ -1136,7 +1136,7 @@ record_t Cache::prepare_record(
   t.read_set.clear();
   t.write_set.clear();
 
-  record_t record(trans_src);
+  record_t record(record_type_t::JOURNAL, trans_src);
   auto commit_time = seastar::lowres_system_clock::now();
 
   // Add new copy of mutated blocks, set_io_wait to block until written

src/crimson/os/seastore/extent_placement_manager.cc

@@ -84,7 +84,7 @@ SegmentedOolWriter::do_write(
       return do_write(t, extents);
     });
   }
-  record_t record(t.get_src());
+  record_t record(record_type_t::OOL, t.get_src());
   std::list<LogicalCachedExtentRef> pending_extents;
   auto commit_time = seastar::lowres_system_clock::now();
 

src/crimson/os/seastore/journal/record_submitter.cc

@@ -493,7 +493,7 @@ void RecordSubmitter::account_submission(
   stats.record_batch_stats.increment(rg.get_size());
 
   for (const record_t& r : rg.records) {
-    auto src = r.type;
+    auto src = r.trans_type;
     assert(is_modify_transaction(src));
     auto& trans_stats = get_by_src(stats.stats_by_src, src);
     ++(trans_stats.num_records);

src/crimson/os/seastore/seastore_types.cc

@@ -388,20 +388,32 @@ std::ostream &operator<<(std::ostream &out, const segment_tail_t &tail)
 
 extent_len_t record_size_t::get_raw_mdlength() const
 {
+  assert(record_type < record_type_t::MAX);
   // empty record is allowed to submit
-  return plain_mdlength +
-         ceph::encoded_sizeof_bounded<record_header_t>();
+  extent_len_t ret = plain_mdlength;
+  if (record_type == record_type_t::JOURNAL) {
+    ret += ceph::encoded_sizeof_bounded<record_header_t>();
+  } else {
+    // OOL won't contain metadata
+    assert(ret == 0);
+  }
+  return ret;
 }
 
 void record_size_t::account_extent(extent_len_t extent_len)
 {
   assert(extent_len);
-  plain_mdlength += ceph::encoded_sizeof_bounded<extent_info_t>();
+  if (record_type == record_type_t::JOURNAL) {
+    plain_mdlength += ceph::encoded_sizeof_bounded<extent_info_t>();
+  } else {
+    // OOL won't contain metadata
+  }
   dlength += extent_len;
 }
 
 void record_size_t::account(const delta_info_t& delta)
 {
+  assert(record_type == record_type_t::JOURNAL);
   assert(delta.bl.length());
   plain_mdlength += ceph::encoded_sizeof(delta);
 }
@@ -433,15 +445,32 @@ std::ostream &operator<<(std::ostream &os, transaction_type_t type)
 std::ostream &operator<<(std::ostream& out, const record_size_t& rsize)
 {
   return out << "record_size_t("
+             << "record_type=" << rsize.record_type
              << "raw_md=" << rsize.get_raw_mdlength()
              << ", data=" << rsize.dlength
              << ")";
 }
 
+std::ostream &operator<<(std::ostream& out, const record_type_t& type)
+{
+  switch (type) {
+  case record_type_t::JOURNAL:
+    return out << "JOURNAL";
+  case record_type_t::OOL:
+    return out << "OOL";
+  case record_type_t::MAX:
+    return out << "NULL";
+  default:
+    return out << "INVALID_RECORD_TYPE("
+               << static_cast<std::size_t>(type)
+               << ")";
+  }
+}
+
 std::ostream &operator<<(std::ostream& out, const record_t& r)
 {
   return out << "record_t("
-             << "type=" << r.type
+             << "trans_type=" << r.trans_type
              << ", num_extents=" << r.extents.size()
              << ", num_deltas=" << r.deltas.size()
              << ", modify_time=" << sea_time_point_printer_t{r.modify_time}
@@ -472,9 +501,16 @@ std::ostream& operator<<(std::ostream& out, const record_group_header_t& h)
 
 extent_len_t record_group_size_t::get_raw_mdlength() const
 {
-  return plain_mdlength +
-         sizeof(checksum_t) +
-         ceph::encoded_sizeof_bounded<record_group_header_t>();
+  assert(record_type < record_type_t::MAX);
+  extent_len_t ret = plain_mdlength;
+  if (record_type == record_type_t::JOURNAL) {
+    ret += sizeof(checksum_t);
+    ret += ceph::encoded_sizeof_bounded<record_group_header_t>();
+  } else {
+    // OOL won't contain metadata
+    assert(ret == 0);
+  }
+  return ret;
 }
 
 void record_group_size_t::account(
@@ -485,14 +521,23 @@ void record_group_size_t::account(
   assert(_block_size > 0);
   assert(rsize.dlength % _block_size == 0);
   assert(block_size == 0 || block_size == _block_size);
-  plain_mdlength += rsize.get_raw_mdlength();
-  dlength += rsize.dlength;
+  assert(record_type == RECORD_TYPE_NULL ||
+         record_type == rsize.record_type);
   block_size = _block_size;
+  record_type = rsize.record_type;
+  if (record_type == record_type_t::JOURNAL) {
+    plain_mdlength += rsize.get_raw_mdlength();
+  } else {
+    // OOL won't contain metadata
+    assert(rsize.get_raw_mdlength() == 0);
+  }
+  dlength += rsize.dlength;
 }
 
 std::ostream& operator<<(std::ostream& out, const record_group_size_t& size)
 {
   return out << "record_group_size_t("
+             << "record_type=" << size.record_type
              << "raw_md=" << size.get_raw_mdlength()
              << ", data=" << size.dlength
              << ", block_size=" << size.block_size
@@ -526,6 +571,7 @@ ceph::bufferlist encode_records(
   const journal_seq_t& committed_to,
   segment_nonce_t current_segment_nonce)
 {
+  assert(record_group.size.record_type < record_type_t::MAX);
   assert(record_group.size.block_size > 0);
   assert(record_group.records.size() > 0);
 
@@ -537,6 +583,15 @@ ceph::bufferlist encode_records(
     }
   }
 
+  if (record_group.size.record_type == record_type_t::OOL) {
+    // OOL won't contain metadata
+    assert(record_group.size.get_mdlength() == 0);
+    ceph_assert(data_bl.length() ==
+                record_group.size.get_encoded_length());
+    record_group.clear();
+    return data_bl;
+  }
+  // JOURNAL
   bufferlist bl;
   record_group_header_t header{
     static_cast<extent_len_t>(record_group.records.size()),
@@ -552,7 +607,7 @@ ceph::bufferlist encode_records(
 
   for (auto& r: record_group.records) {
     record_header_t rheader{
-      r.type,
+      r.trans_type,
       (extent_len_t)r.deltas.size(),
       (extent_len_t)r.extents.size(),
       timepoint_to_mod(r.modify_time)

src/crimson/os/seastore/seastore_types.h

@@ -1893,7 +1893,20 @@ constexpr bool is_modify_transaction(transaction_type_t type) {
       is_background_transaction(type));
 }
 
+// Note: It is possible to statically introduce structs for OOL, which must be
+// more efficient, but that requires to specialize the RecordSubmitter as well.
+// Let's delay this optimization until necessary.
+enum class record_type_t {
+  JOURNAL = 0,
+  OOL, // no header, no metadata, so no padding
+  MAX
+};
+std::ostream &operator<<(std::ostream&, const record_type_t&);
+
+static constexpr auto RECORD_TYPE_NULL = record_type_t::MAX;
+
 struct record_size_t {
+  record_type_t record_type = RECORD_TYPE_NULL; // must not be NULL in use
   extent_len_t plain_mdlength = 0; // mdlength without the record header
   extent_len_t dlength = 0;
 
@@ -1917,30 +1930,37 @@ struct record_size_t {
 std::ostream &operator<<(std::ostream&, const record_size_t&);
 
 struct record_t {
-  transaction_type_t type = TRANSACTION_TYPE_NULL;
+  transaction_type_t trans_type = TRANSACTION_TYPE_NULL;
   std::vector<extent_t> extents;
   std::vector<delta_info_t> deltas;
   record_size_t size;
   sea_time_point modify_time = NULL_TIME;
 
-  record_t(transaction_type_t type) : type{type} { }
+  record_t(record_type_t r_type,
+           transaction_type_t t_type)
+  : trans_type{t_type} {
+    assert(r_type != RECORD_TYPE_NULL);
+    size.record_type = r_type;
+  }
 
   // unit test only
   record_t() {
-    type = transaction_type_t::MUTATE;
+    trans_type = transaction_type_t::MUTATE;
+    size.record_type = record_type_t::JOURNAL;
   }
 
   // unit test only
   record_t(std::vector<extent_t>&& _extents,
            std::vector<delta_info_t>&& _deltas) {
+    trans_type = transaction_type_t::MUTATE;
+    size.record_type = record_type_t::JOURNAL;
     auto modify_time = seastar::lowres_system_clock::now();
     for (auto& e: _extents) {
       push_back(std::move(e), modify_time);
     }
     for (auto& d: _deltas) {
       push_back(std::move(d));
     }
-    type = transaction_type_t::MUTATE;
   }
 
   bool is_empty() const {
@@ -1949,6 +1969,13 @@ struct record_t {
   }
 
   std::size_t get_delta_size() const {
+    assert(size.record_type < record_type_t::MAX);
+    if (size.record_type == record_type_t::OOL) {
+      // OOL won't contain metadata
+      assert(deltas.size() == 0);
+      return 0;
+    }
+    // JOURNAL
     auto delta_size = std::accumulate(
         deltas.begin(), deltas.end(), 0,
         [](uint64_t sum, auto& delta) {
@@ -2018,6 +2045,7 @@ struct record_group_header_t {
 std::ostream& operator<<(std::ostream&, const record_group_header_t&);
 
 struct record_group_size_t {
+  record_type_t record_type = RECORD_TYPE_NULL; // must not be NULL in use
   extent_len_t plain_mdlength = 0; // mdlength without the group header
   extent_len_t dlength = 0;
   extent_len_t block_size = 0;
@@ -2033,7 +2061,14 @@ struct record_group_size_t {
 
   extent_len_t get_mdlength() const {
     assert(block_size > 0);
-    return p2roundup(get_raw_mdlength(), block_size);
+    assert(record_type < record_type_t::MAX);
+    if (record_type == record_type_t::JOURNAL) {
+      return p2roundup(get_raw_mdlength(), block_size);
+    } else {
+      // OOL won't contain metadata
+      assert(get_raw_mdlength() == 0);
+      return 0;
+    }
   }
 
   extent_len_t get_encoded_length() const {
@@ -2401,6 +2436,7 @@ template <> struct fmt::formatter<crimson::os::seastore::record_group_header_t>
 template <> struct fmt::formatter<crimson::os::seastore::record_group_size_t> : fmt::ostream_formatter {};
 template <> struct fmt::formatter<crimson::os::seastore::record_header_t> : fmt::ostream_formatter {};
 template <> struct fmt::formatter<crimson::os::seastore::record_locator_t> : fmt::ostream_formatter {};
+template <> struct fmt::formatter<crimson::os::seastore::record_type_t> : fmt::ostream_formatter {};
 template <> struct fmt::formatter<crimson::os::seastore::record_t> : fmt::ostream_formatter {};
 template <> struct fmt::formatter<crimson::os::seastore::rewrite_gen_printer_t> : fmt::ostream_formatter {};
 template <> struct fmt::formatter<crimson::os::seastore::scan_valid_records_cursor> : fmt::ostream_formatter {};