From bf9f669e06e6ad8d152840843bc6df8ac757d246 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Tue, 7 Dec 2021 16:48:03 +0800 Subject: [PATCH] crimson/os/seastore: measure the number of conflicting transactions by srcs Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/cache.cc | 95 +++++++++++++++++++++++++++----- src/crimson/os/seastore/cache.h | 40 +++++++++++++- 2 files changed, 119 insertions(+), 16 deletions(-) diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 7e56789382fcd..13c0c2651b3bb 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -3,6 +3,7 @@ #include "crimson/os/seastore/cache.h" +#include #include #include "crimson/os/seastore/logging.h" @@ -564,6 +565,64 @@ void Cache::register_metrics() stats.lba_tree_depth, stats.committed_lba_tree_efforts, stats.invalidated_lba_tree_efforts); + + /** + * conflict combinations + */ + auto srcs_label = sm::label("srcs"); + auto num_srcs = static_cast(Transaction::src_t::MAX); + std::size_t srcs_index = 0; + for (uint8_t src2_int = 0; src2_int < num_srcs; ++src2_int) { + auto src2 = static_cast(src2_int); + for (uint8_t src1_int = src2_int; src1_int < num_srcs; ++src1_int) { + ++srcs_index; + auto src1 = static_cast(src1_int); + // impossible combinations + // should be consistent with checks in account_conflict() + if ((src1 == Transaction::src_t::READ && + src2 == Transaction::src_t::READ) || + (src1 == Transaction::src_t::CLEANER_TRIM && + src2 == Transaction::src_t::CLEANER_TRIM) || + (src1 == Transaction::src_t::CLEANER_RECLAIM && + src2 == Transaction::src_t::CLEANER_RECLAIM) || + (src1 == Transaction::src_t::CLEANER_TRIM && + src2 == Transaction::src_t::CLEANER_RECLAIM)) { + continue; + } + std::ostringstream oss; + oss << src1 << "," << src2; + metrics.add_group( + "cache", + { + sm::make_counter( + "trans_srcs_invalidated", + stats.trans_conflicts_by_srcs[srcs_index - 1], + sm::description("total number conflicted transactions by src pair"), + {srcs_label(oss.str())} + ), + } + ); + } + } + assert(srcs_index == NUM_SRC_COMB); + srcs_index = 0; + for (uint8_t src_int = 0; src_int < num_srcs; ++src_int) { + ++srcs_index; + auto src = static_cast(src_int); + std::ostringstream oss; + oss << "UNKNOWN," << src; + metrics.add_group( + "cache", + { + sm::make_counter( + "trans_srcs_invalidated", + stats.trans_conflicts_by_unknown[srcs_index - 1], + sm::description("total number conflicted transactions by src pair"), + {srcs_label(oss.str())} + ), + } + ); + } } void Cache::add_extent(CachedExtentRef ref) @@ -624,23 +683,28 @@ void Cache::remove_extent(CachedExtentRef ref) extents.erase(*ref); } -void Cache::retire_extent(CachedExtentRef ref) +void Cache::commit_retire_extent( + Transaction& t, + CachedExtentRef ref) { - LOG_PREFIX(Cache::retire_extent); - DEBUG("extent {}", *ref); + LOG_PREFIX(Cache::commit_retire_extent); + DEBUGT("extent {}", t, *ref); assert(ref->is_valid()); remove_from_dirty(ref); ref->dirty_from_or_retired_at = JOURNAL_SEQ_MAX; - invalidate_extent(*ref); + invalidate_extent(t, *ref); extents.erase(*ref); } -void Cache::replace_extent(CachedExtentRef next, CachedExtentRef prev) +void Cache::commit_replace_extent( + Transaction& t, + CachedExtentRef next, + CachedExtentRef prev) { - LOG_PREFIX(Cache::replace_extent); - DEBUG("prev {}, next {}", *prev, *next); + LOG_PREFIX(Cache::commit_replace_extent); + DEBUGT("prev {}, next {}", t, *prev, *next); assert(next->get_paddr() == prev->get_paddr()); assert(next->version == prev->version + 1); extents.replace(*next, *prev); @@ -666,20 +730,23 @@ void Cache::replace_extent(CachedExtentRef next, CachedExtentRef prev) add_to_dirty(next); } - invalidate_extent(*prev); + invalidate_extent(t, *prev); } -void Cache::invalidate_extent(CachedExtent &extent) +void Cache::invalidate_extent( + Transaction& t, + CachedExtent& extent) { LOG_PREFIX(Cache::invalidate); - DEBUG("conflict begin -- extent {}", extent); + DEBUGT("conflict begin -- extent {}", t, extent); for (auto &&i: extent.transactions) { if (!i.t->conflicted) { assert(!i.t->is_weak()); + account_conflict(t.get_src(), i.t->get_src()); mark_transaction_conflicted(*i.t, extent); } } - DEBUG("conflict end"); + DEBUGT("conflict end", t); extent.state = CachedExtent::extent_state_t::INVALID; } @@ -900,7 +967,7 @@ record_t Cache::prepare_record(Transaction &t) i->get_type()).increment(i->get_length()); assert(i->prior_instance); - replace_extent(i, i->prior_instance); + commit_replace_extent(t, i, i->prior_instance); i->prepare_write(); i->set_io_wait(); @@ -949,7 +1016,7 @@ record_t Cache::prepare_record(Transaction &t) DEBUGT("retiring {}", t, *i); get_by_ext(efforts.retire_by_ext, i->get_type()).increment(i->get_length()); - retire_extent(i); + commit_retire_extent(t, i); if (i->backend_type == device_type_t::RANDOM_BLOCK) { paddr_t paddr = i->get_paddr(); rbm_alloc_delta_t delta; @@ -1244,6 +1311,7 @@ Cache::get_next_dirty_extents_ret Cache::get_next_dirty_extents( ext->wait_io() ).then_interruptible([FNAME, this, ext, &t, &ret] { if (!ext->is_valid()) { + ++(get_by_src(stats.trans_conflicts_by_unknown, t.get_src())); mark_transaction_conflicted(t, *ext); return; } @@ -1291,6 +1359,7 @@ Cache::get_root_ret Cache::get_root(Transaction &t) DEBUGT("got root read {}", t, *ret); if (!ret->is_valid()) { DEBUGT("root became invalid: {}", t, *ret); + ++(get_by_src(stats.trans_conflicts_by_unknown, t.get_src())); mark_transaction_conflicted(t, *ret); return get_root_iertr::make_ready_future(); } else { diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 8d89423cacb75..190ba5d62eaa8 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -321,6 +321,7 @@ public: (void)this; // silence incorrect clang warning about capture if (!ref->is_valid()) { DEBUGT("got invalid extent: {}", t, ref); + ++(get_by_src(stats.trans_conflicts_by_unknown, t.get_src())); mark_transaction_conflicted(t, *ref); return get_extent_iertr::make_ready_future>(); } else { @@ -373,6 +374,7 @@ public: if (!ret->is_valid()) { LOG_PREFIX(Cache::get_extent_by_type); DEBUGT("got invalid extent: {}", t, ret); + ++(get_by_src(stats.trans_conflicts_by_unknown, t.get_src())); mark_transaction_conflicted(t, *ret.get()); return get_extent_ertr::make_ready_future(); } else { @@ -708,6 +710,9 @@ private: fill_stat_t ool_stats; }; + static constexpr std::size_t NUM_SRC_COMB = + Transaction::SRC_MAX * (Transaction::SRC_MAX + 1) / 2; + struct { counter_by_src_t trans_created_by_src; counter_by_src_t committed_efforts_by_src; @@ -724,6 +729,9 @@ private: uint64_t lba_tree_depth = 0; counter_by_src_t committed_lba_tree_efforts; counter_by_src_t invalidated_lba_tree_efforts; + + std::array trans_conflicts_by_srcs; + counter_by_src_t trans_conflicts_by_unknown; } stats; template @@ -743,6 +751,32 @@ private: return counters_by_ext[index]; } + void account_conflict(Transaction::src_t src1, Transaction::src_t src2) { + assert(src1 < Transaction::src_t::MAX); + assert(src2 < Transaction::src_t::MAX); + if (src1 > src2) { + std::swap(src1, src2); + } + // impossible combinations + // should be consistent with trans_srcs_invalidated in register_metrics() + assert(!(src1 == Transaction::src_t::READ && + src2 == Transaction::src_t::READ)); + assert(!(src1 == Transaction::src_t::CLEANER_TRIM && + src2 == Transaction::src_t::CLEANER_TRIM)); + assert(!(src1 == Transaction::src_t::CLEANER_RECLAIM && + src2 == Transaction::src_t::CLEANER_RECLAIM)); + assert(!(src1 == Transaction::src_t::CLEANER_TRIM && + src2 == Transaction::src_t::CLEANER_RECLAIM)); + + auto src1_value = static_cast(src1); + auto src2_value = static_cast(src2); + auto num_srcs = static_cast(Transaction::src_t::MAX); + auto conflict_index = num_srcs * src1_value + src2_value - + src1_value * (src1_value + 1) / 2; + assert(conflict_index < NUM_SRC_COMB); + ++stats.trans_conflicts_by_srcs[conflict_index]; + } + seastar::metrics::metric_group metrics; void register_metrics(); @@ -771,13 +805,13 @@ private: void remove_extent(CachedExtentRef ref); /// Retire extent - void retire_extent(CachedExtentRef ref); + void commit_retire_extent(Transaction& t, CachedExtentRef ref); /// Replace prev with next - void replace_extent(CachedExtentRef next, CachedExtentRef prev); + void commit_replace_extent(Transaction& t, CachedExtentRef next, CachedExtentRef prev); /// Invalidate extent and mark affected transactions - void invalidate_extent(CachedExtent &extent); + void invalidate_extent(Transaction& t, CachedExtent& extent); /// Mark a valid transaction as conflicted void mark_transaction_conflicted( -- 2.39.5