crimson/os/seastore: simplify cache access metrics

Don't distinguish between cache_absent and load_absent.

Drop cache_access_stats_t::cache_absent and unify with
extent_access_stats_t.

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

diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc
index c021ad92b3f9f94f49ea127352f96ddb2dc0fd23..25d67aed76f7cd8a3db9a912c81fa4e747bcb292 100644 (file)
--- a/src/crimson/os/seastore/backref/btree_backref_manager.cc
+++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc
@@ -36,12 +36,10 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node<
       return {true,
               c.cache.get_extent_viewable_by_trans(c.trans, backref_root)};
     } else {
-      c.cache.account_absent_access(c.trans.get_src());
       return {false,
               Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
     }
   } else {
-    c.cache.account_absent_access(c.trans.get_src());
     return {false,
             Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
   }

diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h
index b3270a94b6cd1035cfcf172e7bfbef367bba729a..a635407dcadb8c33e4950428c2b2481a4b94fec7 100644 (file)
--- a/src/crimson/os/seastore/btree/fixed_kv_btree.h
+++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h
@@ -1503,7 +1503,6 @@ private:
         return on_found(child->template cast<internal_node_t>());
       });
     }
-    c.cache.account_absent_access(c.trans.get_src());
 
     auto child_pos = v.get_child_pos();
     auto next_iter = node_iter + 1;
@@ -1575,7 +1574,6 @@ private:
         return on_found(child->template cast<leaf_node_t>());
       });
     }
-    c.cache.account_absent_access(c.trans.get_src());
 
     auto child_pos = v.get_child_pos();
     auto next_iter = node_iter + 1;
@@ -2135,7 +2133,6 @@ private:
         return do_merge(child->template cast<NodeType>());
       });
     }
-    c.cache.account_absent_access(c.trans.get_src());
 
     auto child_pos = v.get_child_pos();
     return get_node<NodeType>(

diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc
index 53b23025cd6655475a00866b55611b259ed49e4d..9b27ad56fe278a7ee0fd2254bc4bebfd4549c0da 100644 (file)
--- a/src/crimson/os/seastore/cache.cc
+++ b/src/crimson/os/seastore/cache.cc
@@ -208,7 +208,7 @@ void Cache::register_metrics()
       sm::make_counter(
         "cache_hit",
         [this] {
-          return stats.access.s.get_cache_hit();
+          return stats.access.get_cache_hit();
         },
         sm::description("total number of cache hits")
       ),
@@ -2417,22 +2417,20 @@ cache_stats_t Cache::get_stats(
     oss << "\ncache total"
         << cache_size_stats_t{extents_index.get_bytes(), extents_index.size()};
 
-    counter_by_src_t<counter_by_extent_t<extent_access_stats_t> >
+    counter_by_src_t<counter_by_extent_t<cache_access_stats_t> >
       _access_by_src_ext = stats.access_by_src_ext;
     counter_by_src_t<cache_access_stats_t> access_by_src;
     for (uint8_t _src=0; _src<TRANSACTION_TYPE_MAX; ++_src) {
       auto src = static_cast<transaction_type_t>(_src);
       cache_access_stats_t& trans_access = get_by_src(access_by_src, src);
-      trans_access.cache_absent = get_by_src(stats.cache_absent_by_src, src);
-      trans_access.cache_absent -= get_by_src(last_cache_absent_by_src, src);
       auto& access_by_ext = get_by_src(_access_by_src_ext, src);
       const auto& last_access_by_ext = get_by_src(last_access_by_src_ext, src);
       for (uint8_t _ext=0; _ext<EXTENT_TYPES_MAX; ++_ext) {
         auto ext = static_cast<extent_types_t>(_ext);
-        extent_access_stats_t& extent_access = get_by_ext(access_by_ext, ext);
+        cache_access_stats_t& extent_access = get_by_ext(access_by_ext, ext);
         const auto& last_extent_access = get_by_ext(last_access_by_ext, ext);
         extent_access.minus(last_extent_access);
-        trans_access.s.add(extent_access);
+        trans_access.add(extent_access);
       }
     }
     oss << "\naccess: total"
@@ -2443,9 +2441,9 @@ cache_stats_t Cache::get_stats(
       if (trans_access.is_empty()) {
         continue;
       }
-      extent_access_stats_t data_access;
-      extent_access_stats_t mdat_access;
-      extent_access_stats_t phys_access;
+      cache_access_stats_t data_access;
+      cache_access_stats_t mdat_access;
+      cache_access_stats_t phys_access;
       const auto& access_by_ext = get_by_src(_access_by_src_ext, src);
       for (uint8_t _ext=0; _ext<EXTENT_TYPES_MAX; ++_ext) {
         auto ext = static_cast<extent_types_t>(_ext);
@@ -2461,11 +2459,11 @@ cache_stats_t Cache::get_stats(
       oss << "\n  " << src << ": "
           << cache_access_stats_printer_t{seconds, trans_access}
           << "\n    data"
-          << extent_access_stats_printer_t{seconds, data_access}
+          << cache_access_stats_printer_t{seconds, data_access}
           << "\n    mdat"
-          << extent_access_stats_printer_t{seconds, mdat_access}
+          << cache_access_stats_printer_t{seconds, mdat_access}
           << "\n    phys"
-          << extent_access_stats_printer_t{seconds, phys_access};
+          << cache_access_stats_printer_t{seconds, phys_access};
     }
 
     INFO("{}", oss.str());

diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h
index 8e597cafd1160d0884df080e2767dbd8ada840f6..9eef19fc48f8979cba990ee41486d4505ab5e8cf 100644 (file)
--- a/src/crimson/os/seastore/cache.h
+++ b/src/crimson/os/seastore/cache.h
@@ -186,11 +186,6 @@ public:
     return t.root;
   }
 
-  void account_absent_access(Transaction::src_t src) {
-    ++(get_by_src(stats.cache_absent_by_src, src));
-    ++stats.access.cache_absent;
-  }
-
   /**
    * get_extent_if_cached
    *
@@ -208,7 +203,7 @@ public:
     const auto t_src = t.get_src();
     CachedExtentRef ret;
     auto result = t.get_extent(paddr, &ret);
-    extent_access_stats_t& access_stats = get_by_ext(
+    cache_access_stats_t& access_stats = get_by_ext(
       get_by_src(stats.access_by_src_ext, t_src),
       type);
     if (result == Transaction::get_extent_ret::RETIRED) {
@@ -220,14 +215,14 @@ public:
       if (ret->is_stable()) {
         if (ret->is_dirty()) {
           ++access_stats.trans_dirty;
-          ++stats.access.s.trans_dirty;
+          ++stats.access.trans_dirty;
         } else {
           ++access_stats.trans_lru;
-          ++stats.access.s.trans_lru;
+          ++stats.access.trans_lru;
         }
       } else {
         ++access_stats.trans_pending;
-        ++stats.access.s.trans_pending;
+        ++stats.access.trans_pending;
       }
 
       if (ret->get_length() != len) {
@@ -261,7 +256,6 @@ public:
       SUBDEBUGT(seastore_cache,
         "{} {}~0x{:x} is absent in cache",
         t, type, paddr, len);
-      account_absent_access(t_src);
       return get_extent_if_cached_iertr::make_ready_future<CachedExtentRef>();
     }
 
@@ -270,16 +264,15 @@ public:
       SUBDEBUGT(seastore_cache,
         "{} {}~0x{:x} ~0x{:x} is absent(placeholder) in cache",
         t, type, paddr, len, ret->get_length());
-      account_absent_access(t_src);
       return get_extent_if_cached_iertr::make_ready_future<CachedExtentRef>();
     }
 
     if (ret->is_dirty()) {
       ++access_stats.cache_dirty;
-      ++stats.access.s.cache_dirty;
+      ++stats.access.cache_dirty;
     } else {
       ++access_stats.cache_lru;
-      ++stats.access.s.cache_lru;
+      ++stats.access.cache_lru;
     }
 
     if (ret->get_length() != len) {
@@ -402,11 +395,11 @@ public:
       // FIXME: assert(ext.is_stable_clean());
       assert(ext.is_stable());
       assert(T::TYPE == ext.get_type());
-      extent_access_stats_t& access_stats = get_by_ext(
+      cache_access_stats_t& access_stats = get_by_ext(
         get_by_src(stats.access_by_src_ext, t_src),
         T::TYPE);
       ++access_stats.load_absent;
-      ++stats.access.s.load_absent;
+      ++stats.access.load_absent;
 
       t.add_to_read_set(CachedExtentRef(&ext));
       touch_extent(ext, &t_src, t.get_cache_hint());
@@ -475,7 +468,7 @@ public:
 
     const auto t_src = t.get_src();
     auto ext_type = extent->get_type();
-    extent_access_stats_t& access_stats = get_by_ext(
+    cache_access_stats_t& access_stats = get_by_ext(
       get_by_src(stats.access_by_src_ext, t_src),
       ext_type);
 
@@ -487,7 +480,7 @@ public:
         assert(p_extent->is_pending_in_trans(t.get_trans_id()));
         assert(!p_extent->is_stable_writting());
         ++access_stats.trans_pending;
-        ++stats.access.s.trans_pending;
+        ++stats.access.trans_pending;
         if (p_extent->is_mutable()) {
           assert(p_extent->is_fully_loaded());
           assert(!p_extent->is_pending_io());
@@ -502,19 +495,19 @@ public:
         if (t.maybe_add_to_read_set(p_extent)) {
           if (p_extent->is_dirty()) {
             ++access_stats.cache_dirty;
-            ++stats.access.s.cache_dirty;
+            ++stats.access.cache_dirty;
           } else {
             ++access_stats.cache_lru;
-            ++stats.access.s.cache_lru;
+            ++stats.access.cache_lru;
           }
           touch_extent(*p_extent, &t_src, t.get_cache_hint());
         } else {
           if (p_extent->is_dirty()) {
             ++access_stats.trans_dirty;
-            ++stats.access.s.trans_dirty;
+            ++stats.access.trans_dirty;
           } else {
             ++access_stats.trans_lru;
-            ++stats.access.s.trans_lru;
+            ++stats.access.trans_lru;
           }
         }
       }
@@ -522,7 +515,7 @@ public:
       assert(!extent->is_stable_writting());
       assert(extent->is_pending_in_trans(t.get_trans_id()));
       ++access_stats.trans_pending;
-      ++stats.access.s.trans_pending;
+      ++stats.access.trans_pending;
       if (extent->is_mutable()) {
         assert(extent->is_fully_loaded());
         assert(!extent->is_pending_io());
@@ -564,11 +557,11 @@ public:
         t, extent->get_type(), extent->get_paddr(), extent->get_length(),
         partial_off, partial_len, *extent);
       const auto t_src = t.get_src();
-      extent_access_stats_t& access_stats = get_by_ext(
+      cache_access_stats_t& access_stats = get_by_ext(
         get_by_src(stats.access_by_src_ext, t_src),
         extent->get_type());
       ++access_stats.load_present;
-      ++stats.access.s.load_present;
+      ++stats.access.load_present;
       return trans_intr::make_interruptible(
         do_read_extent_maybe_partial(
           std::move(extent), partial_off, partial_len, &t_src));
@@ -878,11 +871,11 @@ private:
     auto f = [&t, this, t_src](CachedExtent &ext) {
       // FIXME: assert(ext.is_stable_clean());
       assert(ext.is_stable());
-      extent_access_stats_t& access_stats = get_by_ext(
+      cache_access_stats_t& access_stats = get_by_ext(
         get_by_src(stats.access_by_src_ext, t_src),
         ext.get_type());
       ++access_stats.load_absent;
-      ++stats.access.s.load_absent;
+      ++stats.access.load_absent;
 
       t.add_to_read_set(CachedExtentRef(&ext));
       touch_extent(ext, &t_src, t.get_cache_hint());
@@ -1763,7 +1756,7 @@ private:
 
     cache_access_stats_t access;
     counter_by_src_t<uint64_t> cache_absent_by_src;
-    counter_by_src_t<counter_by_extent_t<extent_access_stats_t> >
+    counter_by_src_t<counter_by_extent_t<cache_access_stats_t> >
       access_by_src_ext;
 
     uint64_t onode_tree_depth = 0;
@@ -1800,7 +1793,7 @@ private:
   mutable rewrite_stats_t last_reclaim_rewrites;
   mutable cache_access_stats_t last_access;
   mutable counter_by_src_t<uint64_t> last_cache_absent_by_src;
-  mutable counter_by_src_t<counter_by_extent_t<extent_access_stats_t> >
+  mutable counter_by_src_t<counter_by_extent_t<cache_access_stats_t> >
     last_access_by_src_ext;
 
   void account_conflict(Transaction::src_t src1, Transaction::src_t src2) {

diff --git a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
index 5dbbbaf8a9fda85d386b9356383822eac369554a..967cc682e4b4c92093829bce197af0f5560556a0 100644 (file)
--- a/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
+++ b/src/crimson/os/seastore/lba_manager/btree/btree_lba_manager.cc
@@ -60,12 +60,10 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node<
       return {true,
               c.cache.get_extent_viewable_by_trans(c.trans, lba_root)};
     } else {
-      c.cache.account_absent_access(c.trans.get_src());
       return {false,
               Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
     }
   } else {
-    c.cache.account_absent_access(c.trans.get_src());
     return {false,
             Cache::get_extent_iertr::make_ready_future<CachedExtentRef>()};
   }
@@ -103,7 +101,6 @@ BtreeLBAMapping::get_logical_extent(Transaction &t)
     : get_key();
   auto v = p.template get_child<LogicalChildNode>(ctx.trans, ctx.cache, pos, k);
   if (!v.has_child()) {
-    ctx.cache.account_absent_access(ctx.trans.get_src());
     this->child_pos = v.get_child_pos();
   }
   return v;

diff --git a/src/crimson/os/seastore/seastore_types.cc b/src/crimson/os/seastore/seastore_types.cc
index 24fb135cfa5851e5f2834a20da51b023304dc0e1..bd813236119a445d3c4258c9505bcbc42ba4f346 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.cc
+++ b/src/crimson/os/seastore/seastore_types.cc
@@ -1042,42 +1042,6 @@ std::ostream& operator<<(std::ostream& out, const dirty_io_stats_printer_t& p)
   return out;
 }
 
-std::ostream& operator<<(std::ostream& out, const extent_access_stats_printer_t& p)
-{
-  constexpr const char* dfmt = "{:.2f}";
-  double est_total_access = static_cast<double>(p.stats.get_estimated_total_access());
-  out << "(~";
-  if (est_total_access > 1000000) {
-    out << fmt::format(dfmt, est_total_access/1000000)
-        << "M, ";
-  } else {
-    out << fmt::format(dfmt, est_total_access/1000)
-        << "K, ";
-  }
-  double trans_hit = static_cast<double>(p.stats.get_trans_hit());
-  double cache_hit = static_cast<double>(p.stats.get_cache_hit());
-  double est_cache_access = static_cast<double>(p.stats.get_estimated_cache_access());
-  double load_absent = static_cast<double>(p.stats.load_absent);
-  out << "trans-hit=~"
-      << fmt::format(dfmt, trans_hit/est_total_access*100)
-      << "%(p"
-      << fmt::format(dfmt, p.stats.trans_pending/trans_hit)
-      << ",d"
-      << fmt::format(dfmt, p.stats.trans_dirty/trans_hit)
-      << ",l"
-      << fmt::format(dfmt, p.stats.trans_lru/trans_hit)
-      << "), cache-hit=~"
-      << fmt::format(dfmt, cache_hit/est_cache_access*100)
-      << "%(d"
-      << fmt::format(dfmt, p.stats.cache_dirty/cache_hit)
-      << ",l"
-      << fmt::format(dfmt, p.stats.cache_lru/cache_hit)
-      <<"), load-present/absent="
-      << fmt::format(dfmt, p.stats.load_present/load_absent)
-      << ")";
-  return out;
-}
-
 std::ostream& operator<<(std::ostream& out, const cache_access_stats_printer_t& p)
 {
   constexpr const char* dfmt = "{:.2f}";
@@ -1090,28 +1054,26 @@ std::ostream& operator<<(std::ostream& out, const cache_access_stats_printer_t&
     out << fmt::format(dfmt, total_access/1000)
         << "K, ";
   }
-  double trans_hit = static_cast<double>(p.stats.s.get_trans_hit());
-  double cache_hit = static_cast<double>(p.stats.s.get_cache_hit());
+  double trans_hit = static_cast<double>(p.stats.get_trans_hit());
+  double cache_hit = static_cast<double>(p.stats.get_cache_hit());
   double cache_access = static_cast<double>(p.stats.get_cache_access());
-  double load_absent = static_cast<double>(p.stats.s.load_absent);
+  double load_absent = static_cast<double>(p.stats.load_absent);
   out << "trans-hit="
       << fmt::format(dfmt, trans_hit/total_access*100)
-      << "%(p"
-      << fmt::format(dfmt, p.stats.s.trans_pending/trans_hit)
-      << ",d"
-      << fmt::format(dfmt, p.stats.s.trans_dirty/trans_hit)
-      << ",l"
-      << fmt::format(dfmt, p.stats.s.trans_lru/trans_hit)
+      << "%(pend"
+      << fmt::format(dfmt, p.stats.trans_pending/trans_hit)
+      << ",dirt"
+      << fmt::format(dfmt, p.stats.trans_dirty/trans_hit)
+      << ",lru"
+      << fmt::format(dfmt, p.stats.trans_lru/trans_hit)
       << "), cache-hit="
       << fmt::format(dfmt, cache_hit/cache_access*100)
-      << "%(d"
-      << fmt::format(dfmt, p.stats.s.cache_dirty/cache_hit)
-      << ",l"
-      << fmt::format(dfmt, p.stats.s.cache_lru/cache_hit)
-      <<"), load/absent="
-      << fmt::format(dfmt, load_absent/p.stats.cache_absent*100)
-      << "%, load-present/absent="
-      << fmt::format(dfmt, p.stats.s.load_present/load_absent)
+      << "%(dirt"
+      << fmt::format(dfmt, p.stats.cache_dirty/cache_hit)
+      << ",lru"
+      << fmt::format(dfmt, p.stats.cache_lru/cache_hit)
+      <<"), load-present/absent="
+      << fmt::format(dfmt, p.stats.load_present/load_absent)
       << ")";
   return out;
 }

diff --git a/src/crimson/os/seastore/seastore_types.h b/src/crimson/os/seastore/seastore_types.h
index 29bd7b9c85f12a87f0f82cb6ceb488e502bac9ac..5b3d18a709432525ef0e744f267c0addde429823 100644 (file)
--- a/src/crimson/os/seastore/seastore_types.h
+++ b/src/crimson/os/seastore/seastore_types.h
@@ -2962,7 +2962,7 @@ std::ostream& operator<<(std::ostream&, const dirty_io_stats_printer_t&);
  *   get_caching_extent() -- test only
  *   get_caching_extent_by_type() -- test only
  */
-struct extent_access_stats_t {
+struct cache_access_stats_t {
   uint64_t trans_pending = 0;
   uint64_t trans_dirty = 0;
   uint64_t trans_lru = 0;
@@ -2980,19 +2980,19 @@ struct extent_access_stats_t {
     return cache_dirty + cache_lru;
   }
 
-  uint64_t get_estimated_cache_access() const {
+  uint64_t get_cache_access() const {
     return get_cache_hit() + load_absent;
   }
 
-  uint64_t get_estimated_total_access() const {
-    return get_trans_hit() + get_cache_hit() + load_absent;
+  uint64_t get_total_access() const {
+    return get_trans_hit() + get_cache_access();
   }
 
   bool is_empty() const {
-    return get_estimated_total_access() == 0;
+    return get_total_access() == 0;
   }
 
-  void add(const extent_access_stats_t& o) {
+  void add(const cache_access_stats_t& o) {
     trans_pending += o.trans_pending;
     trans_dirty += o.trans_dirty;
     trans_lru += o.trans_lru;
@@ -3002,7 +3002,7 @@ struct extent_access_stats_t {
     load_present += o.load_present;
   }
 
-  void minus(const extent_access_stats_t& o) {
+  void minus(const cache_access_stats_t& o) {
     trans_pending -= o.trans_pending;
     trans_dirty -= o.trans_dirty;
     trans_lru -= o.trans_lru;
@@ -3022,43 +3022,6 @@ struct extent_access_stats_t {
     load_present /= d;
   }
 };
-struct extent_access_stats_printer_t {
-  double seconds;
-  const extent_access_stats_t& stats;
-};
-std::ostream& operator<<(std::ostream&, const extent_access_stats_printer_t&);
-
-struct cache_access_stats_t {
-  extent_access_stats_t s;
-  uint64_t cache_absent = 0;
-
-  uint64_t get_cache_access() const {
-    return s.get_cache_hit() + cache_absent;
-  }
-
-  uint64_t get_total_access() const {
-    return s.get_trans_hit() + get_cache_access();
-  }
-
-  bool is_empty() const {
-    return get_total_access() == 0;
-  }
-
-  void add(const cache_access_stats_t& o) {
-    s.add(o.s);
-    cache_absent += o.cache_absent;
-  }
-
-  void minus(const cache_access_stats_t& o) {
-    s.minus(o.s);
-    cache_absent -= o.cache_absent;
-  }
-
-  void divide_by(unsigned d) {
-    s.divide_by(d);
-    cache_absent /= d;
-  }
-};
 struct cache_access_stats_printer_t {
   double seconds;
   const cache_access_stats_t& stats;