From 336fe386c72d070ced7e9660c68ff824e3ef519a Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Tue, 28 Sep 2021 14:48:09 +0800 Subject: [PATCH] crimson/os/seastore: cleanup, consolidate metrics about transactional efforts Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/cache.cc | 162 +++++++++++++------------------ src/crimson/os/seastore/cache.h | 28 +++--- 2 files changed, 83 insertions(+), 107 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 5cb9befc85329..f1c79c21c4ff6 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -141,62 +141,6 @@ void Cache::register_metrics() ); } - /* - * trans_committed - */ - for (auto& [src, src_label] : labels_by_src) { - if (src == src_t::READ) { - // READ transaction won't commit - continue; - } - metrics.add_group( - "cache", - { - sm::make_counter( - "trans_committed", - get_by_src(stats.trans_committed_by_src, src), - sm::description("total number of transaction committed"), - {src_label} - ), - } - ); - } - - /* - * trans_invalidated - */ - for (auto& [src, src_label] : labels_by_src) { - for (auto& [ext, ext_label] : labels_by_ext) { - auto& counter_by_extent = get_by_src(stats.trans_invalidated, src); - auto& counter = get_by_ext(counter_by_extent, ext); - metrics.add_group( - "cache", - { - sm::make_counter( - "trans_invalidated", - counter, - sm::description("total number of transaction invalidated"), - {src_label, ext_label} - ), - } - ); - } - } - - /** - * trans_read_successful - */ - metrics.add_group( - "cache", - { - sm::make_counter( - "trans_read_successful", - stats.read_transactions_successful, - sm::description("total number of successful read transactions") - ), - } - ); - /* * cache_query: cache_access and cache_hit */ @@ -233,13 +177,48 @@ void Cache::register_metrics() "FRESH"sv, }; - // invalidated efforts (non READ) + // invalidated efforts for (auto& [src, src_label] : labels_by_src) { + auto& efforts = get_by_src(stats.invalidated_efforts_by_src, src); + for (auto& [ext, ext_label] : labels_by_ext) { + auto& counter = get_by_ext(efforts.num_trans_invalidated, ext); + metrics.add_group( + "cache", + { + sm::make_counter( + "trans_invalidated", + counter, + sm::description("total number of transaction invalidated"), + {src_label, ext_label} + ), + } + ); + } + if (src == src_t::READ) { - // register src_t::READ later + // read transaction won't have non-read efforts + auto read_effort_label = effort_label("READ"); + metrics.add_group( + "cache", + { + sm::make_counter( + "invalidated_extents", + efforts.read.extents, + sm::description("extents of invalidated transactions"), + {src_label, read_effort_label} + ), + sm::make_counter( + "invalidated_extent_bytes", + efforts.read.bytes, + sm::description("extent bytes of invalidated transactions"), + {src_label, read_effort_label} + ), + } + ); continue; } - auto& efforts = get_by_src(stats.invalidated_efforts_by_src, src); + + // non READ invalidated efforts for (auto& effort_name : effort_names) { auto& effort = [&effort_name, &efforts]() -> effort_t& { if (effort_name == "READ") { @@ -285,36 +264,24 @@ void Cache::register_metrics() ); } // src - // invalidated efforts (READ) - // read transaction won't have non-read efforts - auto read_src_label = labels_by_src.find(src_t::READ)->second; - auto read_effort_label = effort_label("READ"); - auto& read_efforts = get_by_src(stats.invalidated_efforts_by_src, src_t::READ); - metrics.add_group( - "cache", - { - sm::make_counter( - "invalidated_extents", - read_efforts.read.extents, - sm::description("extents of invalidated transactions"), - {read_src_label, read_effort_label} - ), - sm::make_counter( - "invalidated_extent_bytes", - read_efforts.read.bytes, - sm::description("extent bytes of invalidated transactions"), - {read_src_label, read_effort_label} - ), - } - ); - - // by-extent committed efforts + // committed efforts for (auto& [src, src_label] : labels_by_src) { if (src == src_t::READ) { // READ transaction won't commit continue; } auto& efforts = get_by_src(stats.committed_efforts_by_src, src); + metrics.add_group( + "cache", + { + sm::make_counter( + "trans_committed", + efforts.num_trans, + sm::description("total number of transaction committed"), + {src_label} + ), + } + ); for (auto& effort_name : effort_names) { auto& effort_by_ext = [&efforts, &effort_name]() -> counter_by_extent_t& { @@ -368,20 +335,23 @@ void Cache::register_metrics() } // ext } // src - /** - * read_effort_successful - */ + // successful read efforts metrics.add_group( "cache", { + sm::make_counter( + "trans_read_successful", + stats.success_read_efforts.num_trans, + sm::description("total number of successful read transactions") + ), sm::make_counter( "successful_read_extents", - stats.read_effort_successful.extents, + stats.success_read_efforts.read.extents, sm::description("extents of successful read transactions") ), sm::make_counter( "successful_read_extent_bytes", - stats.read_effort_successful.bytes, + stats.success_read_efforts.read.bytes, sm::description("extent bytes of successful read transactions") ), } @@ -626,16 +596,18 @@ void Cache::mark_transaction_conflicted( DEBUGT("set conflict", t); t.conflicted = true; - auto& counter_by_extent = get_by_src(stats.trans_invalidated, t.get_src()); - auto& counter = get_by_ext(counter_by_extent, conflicting_extent.get_type()); - ++counter; - auto& efforts = get_by_src(stats.invalidated_efforts_by_src, t.get_src()); + + auto& counter = get_by_ext(efforts.num_trans_invalidated, + conflicting_extent.get_type()); + ++counter; + efforts.read.extents += t.read_set.size(); for (auto &i: t.read_set) { efforts.read.bytes += i.ref->get_length(); } + if (t.get_src() != Transaction::src_t::READ) { efforts.retire.extents += t.retired_set.size(); for (auto &i: t.retired_set) { @@ -683,9 +655,9 @@ void Cache::on_transaction_destruct(Transaction& t) t.conflicted == false && !t.is_weak()) { DEBUGT("read is successful", t); - ++stats.read_transactions_successful; + ++stats.success_read_efforts.num_trans; - auto& effort = stats.read_effort_successful; + auto& effort = stats.success_read_efforts.read; effort.extents += t.read_set.size(); for (auto &i: t.read_set) { effort.bytes += i.ref->get_length(); @@ -788,9 +760,9 @@ record_t Cache::prepare_record(Transaction &t) get_by_src(stats.committed_lba_tree_efforts, t.get_src() ).increment(t.lba_tree_stats); - ++(get_by_src(stats.trans_committed_by_src, t.get_src())); auto& efforts = get_by_src(stats.committed_efforts_by_src, t.get_src()); + ++(efforts.num_trans); // Should be valid due to interruptible future for (auto &i: t.read_set) { diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 786d2798cf6db..7ae90a9b038af 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -638,7 +638,7 @@ private: * * Each effort_t represents the effort of a set of extents involved in the * transaction, classified by read, mutate, retire and allocate behaviors, - * see trans_efforts_t. + * see XXX_trans_efforts_t. */ struct effort_t { uint64_t extents = 0; @@ -650,23 +650,30 @@ private: } }; - struct trans_efforts_t { + template + using counter_by_extent_t = std::array; + + struct invalid_trans_efforts_t { effort_t read; effort_t mutate; uint64_t mutate_delta_bytes = 0; effort_t retire; effort_t fresh; + counter_by_extent_t num_trans_invalidated; }; - template - using counter_by_extent_t = std::array; - - struct trans_byextent_efforts_t { + struct commit_trans_efforts_t { counter_by_extent_t read_by_ext; counter_by_extent_t mutate_by_ext; counter_by_extent_t delta_bytes_by_ext; counter_by_extent_t retire_by_ext; counter_by_extent_t fresh_by_ext; + uint64_t num_trans = 0; + }; + + struct success_read_trans_efforts_t { + effort_t read; + uint64_t num_trans = 0; }; struct tree_efforts_t { @@ -684,13 +691,10 @@ private: struct { counter_by_src_t trans_created_by_src; - counter_by_src_t trans_committed_by_src; - counter_by_src_t committed_efforts_by_src; - counter_by_src_t> trans_invalidated; - counter_by_src_t invalidated_efforts_by_src; + counter_by_src_t committed_efforts_by_src; + counter_by_src_t invalidated_efforts_by_src; counter_by_src_t cache_query_by_src; - uint64_t read_transactions_successful = 0; - effort_t read_effort_successful; + success_read_trans_efforts_t success_read_efforts; uint64_t dirty_bytes = 0; uint64_t onode_tree_depth = 0; -- 2.39.5