From: Yingxin Cheng Date: Tue, 15 Apr 2025 07:24:05 +0000 (+0800) Subject: crimson/os/seastore: simplify cache access metrics X-Git-Tag: v20.3.0~7^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=dfba56d9606f4729fd212ea7e6061f07d7deb305;p=ceph.git 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 --- diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc index c021ad92b3f9..25d67aed76f7 100644 --- 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()}; } } else { - c.cache.account_absent_access(c.trans.get_src()); return {false, Cache::get_extent_iertr::make_ready_future()}; } diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index b3270a94b6cd..a635407dcadb 100644 --- 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()); }); } - 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()); }); } - 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()); }); } - c.cache.account_absent_access(c.trans.get_src()); auto child_pos = v.get_child_pos(); return get_node( diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index 53b23025cd66..9b27ad56fe27 100644 --- 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_src_t > _access_by_src_ext = stats.access_by_src_ext; counter_by_src_t access_by_src; for (uint8_t _src=0; _src(_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(_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(_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 8e597cafd116..9eef19fc48f8 100644 --- 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(); } @@ -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(); } 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 cache_absent_by_src; - counter_by_src_t > + counter_by_src_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 last_cache_absent_by_src; - mutable counter_by_src_t > + mutable counter_by_src_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 5dbbbaf8a9fd..967cc682e4b4 100644 --- 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()}; } } else { - c.cache.account_absent_access(c.trans.get_src()); return {false, Cache::get_extent_iertr::make_ready_future()}; } @@ -103,7 +101,6 @@ BtreeLBAMapping::get_logical_extent(Transaction &t) : get_key(); auto v = p.template get_child(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 24fb135cfa58..bd813236119a 100644 --- 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(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(p.stats.get_trans_hit()); - double cache_hit = static_cast(p.stats.get_cache_hit()); - double est_cache_access = static_cast(p.stats.get_estimated_cache_access()); - double load_absent = static_cast(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(p.stats.s.get_trans_hit()); - double cache_hit = static_cast(p.stats.s.get_cache_hit()); + double trans_hit = static_cast(p.stats.get_trans_hit()); + double cache_hit = static_cast(p.stats.get_cache_hit()); double cache_access = static_cast(p.stats.get_cache_access()); - double load_absent = static_cast(p.stats.s.load_absent); + double load_absent = static_cast(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 29bd7b9c85f1..5b3d18a70943 100644 --- 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;