From e428c13d34f18da80ae9f455f49168be81ccd384 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Thu, 25 Nov 2021 09:23:53 +0800 Subject: [PATCH] crimson/os/seastore: fix record metrics in cache Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/cache.cc | 108 +++++------------- src/crimson/os/seastore/cache.h | 19 +-- .../os/seastore/extent_placement_manager.cc | 1 + src/crimson/os/seastore/seastore_types.cc | 13 ++- src/crimson/os/seastore/seastore_types.h | 7 +- src/crimson/os/seastore/transaction.h | 2 + 6 files changed, 52 insertions(+), 98 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index e145b231c40c7..d8de84bdb7589 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -273,9 +273,9 @@ void Cache::register_metrics() {src_label} ), sm::make_counter( - "invalidated_ool_record_overhead_bytes", - efforts.ool_record_overhead_bytes, - sm::description("bytes of ool-record overhead from invalidated transactions"), + "invalidated_ool_record_bytes", + efforts.ool_record_bytes, + sm::description("bytes of ool-record from invalidated transactions"), {src_label} ), } @@ -313,15 +313,28 @@ void Cache::register_metrics() {src_label} ), sm::make_counter( - "committed_ool_record_overhead_bytes", - efforts.ool_record_overhead_bytes, - sm::description("bytes of ool-record overhead from committed transactions"), + "committed_ool_record_padding_bytes", + efforts.ool_record_padding_bytes, + sm::description("bytes of ool-record padding from committed transactions"), {src_label} ), sm::make_counter( - "committed_inline_record_overhead_bytes", - efforts.inline_record_overhead_bytes, - sm::description("bytes of inline-record overhead from committed transactions"), + "committed_ool_record_metadata_bytes", + efforts.ool_record_metadata_bytes, + sm::description("bytes of ool-record metadata from committed transactions"), + {src_label} + ), + sm::make_counter( + "committed_ool_record_data_bytes", + efforts.ool_record_data_bytes, + sm::description("bytes of ool-record data from committed transactions"), + {src_label} + ), + sm::make_counter( + "committed_inline_record_metadata_bytes", + efforts.inline_record_metadata_bytes, + sm::description("bytes of inline-record metadata from committed transactions" + "(excludes delta buffer)"), {src_label} ), } @@ -406,52 +419,6 @@ void Cache::register_metrics() ); } - /** - * Record header fullness - */ - auto record_type_label = sm::label("record_type"); - using namespace std::literals::string_view_literals; - const string_view record_type_names[] = { - "INLINE"sv, - "OOL"sv, - }; - for (auto& [src, src_label] : labels_by_src) { - if (src == src_t::READ) { - // READ transaction won't commit - continue; - } - auto& record_header_fullness = get_by_src( - stats.record_header_fullness_by_src, src); - for (auto& record_type_name : record_type_names) { - auto& fill_stat = [&record_type_name, - &record_header_fullness]() -> fill_stat_t& { - if (record_type_name == "INLINE") { - return record_header_fullness.inline_stats; - } else { - assert(record_type_name == "OOL"); - return record_header_fullness.ool_stats; - } - }(); - metrics.add_group( - "cache", - { - sm::make_counter( - "record_header_filled_bytes", - fill_stat.filled_bytes, - sm::description("filled bytes of record header"), - {src_label, record_type_label(record_type_name)} - ), - sm::make_counter( - "record_header_total_bytes", - fill_stat.total_bytes, - sm::description("total bytes of record header"), - {src_label, record_type_label(record_type_name)} - ), - } - ); - } - } - /** * Cached extents (including placeholders) * @@ -793,12 +760,9 @@ void Cache::mark_transaction_conflicted( efforts.fresh_ool_written.extents += ool_stats.extents.num; efforts.fresh_ool_written.bytes += ool_stats.extents.bytes; efforts.num_ool_records += ool_stats.num_records; - efforts.ool_record_overhead_bytes += ool_stats.header_bytes; - - auto& record_header_fullness = get_by_src( - stats.record_header_fullness_by_src, t.get_src()); - record_header_fullness.ool_stats.filled_bytes += ool_stats.header_raw_bytes; - record_header_fullness.ool_stats.total_bytes += ool_stats.header_bytes; + efforts.ool_record_bytes += + (ool_stats.header_bytes + ool_stats.data_bytes); + // Note: we only account overhead from committed ool records if (t.get_src() == Transaction::src_t::CLEANER_TRIM || t.get_src() == Transaction::src_t::CLEANER_RECLAIM) { @@ -1088,23 +1052,13 @@ record_t Cache::prepare_record(Transaction &t) ).increment(t.lba_tree_stats); ++(efforts.num_trans); - efforts.num_ool_records += ool_stats.num_records; - efforts.ool_record_overhead_bytes += ool_stats.header_bytes; - - auto& record_header_fullness = get_by_src( - stats.record_header_fullness_by_src, t.get_src()); - record_header_fullness.ool_stats.filled_bytes += ool_stats.header_raw_bytes; - record_header_fullness.ool_stats.total_bytes += ool_stats.header_bytes; - - // TODO: move to Journal to get accurate result - auto record_size = record_group_size_t( - record.size, reader.get_block_size()); - auto inline_overhead = - record_size.get_encoded_length() - record.get_raw_data_size(); - efforts.inline_record_overhead_bytes += inline_overhead; - record_header_fullness.inline_stats.filled_bytes += record_size.get_raw_mdlength(); - record_header_fullness.inline_stats.total_bytes += record_size.get_mdlength(); + efforts.ool_record_padding_bytes += + (ool_stats.header_bytes - ool_stats.header_raw_bytes); + efforts.ool_record_metadata_bytes += ool_stats.header_raw_bytes; + efforts.ool_record_data_bytes += ool_stats.data_bytes; + efforts.inline_record_metadata_bytes += + (record.size.get_raw_mdlength() - record.get_delta_size()); } return record; diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 9f508057ea627..561984d745a2b 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -734,7 +734,7 @@ private: effort_t fresh_ool_written; counter_by_extent_t num_trans_invalidated; uint64_t num_ool_records = 0; - uint64_t ool_record_overhead_bytes = 0; + uint64_t ool_record_bytes = 0; }; struct commit_trans_efforts_t { @@ -747,8 +747,10 @@ private: counter_by_extent_t fresh_ool_by_ext; uint64_t num_trans = 0; // the number of inline records uint64_t num_ool_records = 0; - uint64_t ool_record_overhead_bytes = 0; - uint64_t inline_record_overhead_bytes = 0; + uint64_t ool_record_padding_bytes = 0; + uint64_t ool_record_metadata_bytes = 0; + uint64_t ool_record_data_bytes = 0; + uint64_t inline_record_metadata_bytes = 0; // metadata exclude the delta bytes }; struct success_read_trans_efforts_t { @@ -769,16 +771,6 @@ private: template using counter_by_src_t = std::array; - struct fill_stat_t { - uint64_t filled_bytes = 0; - uint64_t total_bytes = 0; - }; - - struct record_header_fullness_t { - fill_stat_t inline_stats; - fill_stat_t ool_stats; - }; - static constexpr std::size_t NUM_SRC_COMB = Transaction::SRC_MAX * (Transaction::SRC_MAX + 1) / 2; @@ -787,7 +779,6 @@ private: counter_by_src_t committed_efforts_by_src; counter_by_src_t invalidated_efforts_by_src; counter_by_src_t cache_query_by_src; - counter_by_src_t record_header_fullness_by_src; success_read_trans_efforts_t success_read_efforts; uint64_t dirty_bytes = 0; diff --git a/src/crimson/os/seastore/extent_placement_manager.cc b/src/crimson/os/seastore/extent_placement_manager.cc index a4d335a637a05..fde7de1c55041 100644 --- a/src/crimson/os/seastore/extent_placement_manager.cc +++ b/src/crimson/os/seastore/extent_placement_manager.cc @@ -94,6 +94,7 @@ SegmentedAllocator::Writer::_write( stats.extents.bytes += record_size.dlength; stats.header_raw_bytes += record_size.get_raw_mdlength(); stats.header_bytes += record_size.get_mdlength(); + stats.data_bytes += record_size.dlength; stats.num_records += 1; return trans_intr::make_interruptible( diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc index 80f0420392cdd..d4adada28ab30 100644 --- a/src/crimson/os/seastore/seastore_types.cc +++ b/src/crimson/os/seastore/seastore_types.cc @@ -154,6 +154,14 @@ std::ostream &operator<<(std::ostream &out, const segment_header_t &header) << ")"; } +extent_len_t record_size_t::get_raw_mdlength() const +{ + // FIXME: prevent submitting empty records + // assert(!is_empty()); + return plain_mdlength + + ceph::encoded_sizeof_bounded(); +} + void record_size_t::account_extent(extent_len_t extent_len) { assert(extent_len); @@ -183,10 +191,7 @@ 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.plain_mdlength + - ceph::encoded_sizeof_bounded() - ); + plain_mdlength += rsize.get_raw_mdlength(); dlength += rsize.dlength; block_size = _block_size; } diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h index 9d9119a2ec093..3769d2bbf2b75 100644 --- a/src/crimson/os/seastore/seastore_types.h +++ b/src/crimson/os/seastore/seastore_types.h @@ -1196,6 +1196,8 @@ struct record_size_t { extent_len_t plain_mdlength = 0; // mdlength without the record header extent_len_t dlength = 0; + extent_len_t get_raw_mdlength() const; + bool is_empty() const { return plain_mdlength == 0 && dlength == 0; @@ -1231,15 +1233,14 @@ struct record_t { deltas.size() == 0; } - // the size of extents and delta buffers - std::size_t get_raw_data_size() const { + std::size_t get_delta_size() const { auto delta_size = std::accumulate( deltas.begin(), deltas.end(), 0, [](uint64_t sum, auto& delta) { return sum + delta.bl.length(); } ); - return size.dlength + delta_size; + return delta_size; } void push_back(extent_t&& extent) { diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index c41880550cf0d..0d9cb02c0bd61 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -320,12 +320,14 @@ public: io_stat_t extents; uint64_t header_raw_bytes = 0; uint64_t header_bytes = 0; + uint64_t data_bytes = 0; uint64_t num_records = 0; bool is_clear() const { return (extents.is_clear() && header_raw_bytes == 0 && header_bytes == 0 && + data_bytes == 0 && num_records == 0); } }; -- 2.39.5