crimson/os/seastore: measure the number of conflicting transactions by srcs

Signed-off-by: Yingxin Cheng <yingxin.cheng@intel.com>

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 7e56789382fcde4948094d006e2c117c763fa8ce..13c0c2651b3bb6b14fabe2655ec14f539ec4cbd1 100644 (file)
--- 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 <sstream>
 #include <string_view>
 
 #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<std::size_t>(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<Transaction::src_t>(src2_int);
+    for (uint8_t src1_int = src2_int; src1_int < num_srcs; ++src1_int) {
+      ++srcs_index;
+      auto src1 = static_cast<Transaction::src_t>(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<Transaction::src_t>(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<RootBlockRef>();
       } else {

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 8d89423cacb756bc6a0e0ad94318304dd338aa7d..190ba5d62eaa8c066f93ce17e696edf8d58ef499 100644 (file)
--- 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<TCachedExtentRef<T>>();
        } 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<CachedExtentRef>();
         } 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<uint64_t> trans_created_by_src;
     counter_by_src_t<commit_trans_efforts_t> committed_efforts_by_src;
@@ -724,6 +729,9 @@ private:
     uint64_t lba_tree_depth = 0;
     counter_by_src_t<tree_efforts_t> committed_lba_tree_efforts;
     counter_by_src_t<tree_efforts_t> invalidated_lba_tree_efforts;
+
+    std::array<uint64_t, NUM_SRC_COMB> trans_conflicts_by_srcs;
+    counter_by_src_t<uint64_t> trans_conflicts_by_unknown;
   } stats;
 
   template <typename CounterT>
@@ -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<std::size_t>(src1);
+    auto src2_value = static_cast<std::size_t>(src2);
+    auto num_srcs = static_cast<std::size_t>(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(