From a9f8fa455555b5dd23a01026eb74d1454213cdc3 Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Thu, 13 Oct 2022 14:27:34 +0800 Subject: [PATCH] crimson/os/seastore/btree: avoid searching transactions' read_set when retrieving btree nodes Signed-off-by: Xuehan Xu (cherry picked from commit c29051c4c747c5b8415b8600affc24c0025507fb) --- .../os/seastore/btree/fixed_kv_btree.h | 6 +- src/crimson/os/seastore/cache.h | 170 ++++++++++++++++-- src/crimson/os/seastore/transaction_manager.h | 4 +- 3 files changed, 156 insertions(+), 24 deletions(-) diff --git a/src/crimson/os/seastore/btree/fixed_kv_btree.h b/src/crimson/os/seastore/btree/fixed_kv_btree.h index ca6e9359e7a96..b7056e4657862 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_btree.h +++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h @@ -1052,13 +1052,13 @@ private: c.pins->add_pin(node.pin); } }; - return c.cache.template get_extent( + return c.cache.template get_absent_extent( c.trans, offset, node_size, init_internal ).si_then([FNAME, c, offset, init_internal, depth, begin, end]( - typename internal_node_t::Ref ret) { + typename internal_node_t::Ref ret) { SUBTRACET( seastore_fixedkv_tree, "read internal at offset {} {}", @@ -1119,7 +1119,7 @@ private: c.pins->add_pin(node.pin); } }; - return c.cache.template get_extent( + return c.cache.template get_absent_extent( c.trans, offset, node_size, diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 32ff392b62c1b..a04693e73d3e3 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -424,32 +424,71 @@ public: auto result = t.get_extent(offset, &ret); if (result != Transaction::get_extent_ret::ABSENT) { SUBTRACET(seastore_cache, "{} {}~{} is {} on t -- {}", - t, - T::TYPE, - offset, - length, - result == Transaction::get_extent_ret::PRESENT ? "present" : "retired", - *ret); + t, + T::TYPE, + offset, + length, + result == Transaction::get_extent_ret::PRESENT ? "present" : "retired", + *ret); assert(result != Transaction::get_extent_ret::RETIRED); return ret->wait_io().then([ret] { return seastar::make_ready_future>( ret->cast()); }); - } else { - SUBTRACET(seastore_cache, "{} {}~{} is absent on t, query cache ...", - t, T::TYPE, offset, length); - auto f = [&t, this](CachedExtent &ext) { - t.add_to_read_set(CachedExtentRef(&ext)); - touch_extent(ext); - }; - auto metric_key = std::make_pair(t.get_src(), T::TYPE); - return trans_intr::make_interruptible( - get_extent( - offset, length, &metric_key, - std::forward(extent_init_func), std::move(f)) - ); } + + SUBTRACET(seastore_cache, "{} {}~{} is absent on t, query cache ...", + t, T::TYPE, offset, length); + auto f = [&t, this](CachedExtent &ext) { + t.add_to_read_set(CachedExtentRef(&ext)); + touch_extent(ext); + }; + auto metric_key = std::make_pair(t.get_src(), T::TYPE); + return trans_intr::make_interruptible( + get_extent( + offset, length, &metric_key, + std::forward(extent_init_func), std::move(f)) + ); } + + /* + * get_absent_extent + * + * Mostly the same as Cache::get_extent(), with the only difference + * that get_absent_extent won't search the transaction's context for + * the specific CachedExtent + */ + template + get_extent_iertr::future> get_absent_extent( + Transaction &t, + paddr_t offset, + extent_len_t length, + Func &&extent_init_func) { + CachedExtentRef ret; + LOG_PREFIX(Cache::get_extent); + +#ifndef NDEBUG + auto r = t.get_extent(offset, &ret); + if (r != Transaction::get_extent_ret::ABSENT) { + SUBERRORT(seastore_cache, "unexpected non-absent extent {}", t, *ret); + ceph_abort(); + } +#endif + + SUBTRACET(seastore_cache, "{} {}~{} is absent on t, query cache ...", + t, T::TYPE, offset, length); + auto f = [&t, this](CachedExtent &ext) { + t.add_to_read_set(CachedExtentRef(&ext)); + touch_extent(ext); + }; + auto metric_key = std::make_pair(t.get_src(), T::TYPE); + return trans_intr::make_interruptible( + get_extent( + offset, length, &metric_key, + std::forward(extent_init_func), std::move(f)) + ); + } + template get_extent_iertr::future> get_extent( Transaction &t, @@ -458,6 +497,21 @@ public: return get_extent(t, offset, length, [](T &){}); } + /* + * get_absent_extent + * + * Mostly the same as Cache::get_extent(), with the only difference + * that get_absent_extent won't search the transaction's context for + * the specific CachedExtent + */ + template + get_extent_iertr::future> get_absent_extent( + Transaction &t, + paddr_t offset, + extent_len_t length) { + return get_absent_extent(t, offset, length, [](T &){}); + } + extent_len_t get_block_size() const { return epm.get_block_size(); } @@ -539,6 +593,39 @@ private: } } + get_extent_by_type_ret _get_absent_extent_by_type( + Transaction &t, + extent_types_t type, + paddr_t offset, + laddr_t laddr, + extent_len_t length, + extent_init_func_t &&extent_init_func + ) { + LOG_PREFIX(Cache::_get_absent_extent_by_type); + +#ifndef NDEBUG + CachedExtentRef ret; + auto r = t.get_extent(offset, &ret); + if (r != Transaction::get_extent_ret::ABSENT) { + SUBERRORT(seastore_cache, "unexpected non-absent extent {}", t, *ret); + ceph_abort(); + } +#endif + + SUBTRACET(seastore_cache, "{} {}~{} {} is absent on t, query cache ...", + t, type, offset, length, laddr); + auto f = [&t, this](CachedExtent &ext) { + t.add_to_read_set(CachedExtentRef(&ext)); + touch_extent(ext); + }; + auto src = t.get_src(); + return trans_intr::make_interruptible( + _get_extent_by_type( + type, offset, laddr, length, &src, + std::move(extent_init_func), std::move(f)) + ); + } + backref_entryrefs_by_seq_t backref_entryrefs_by_seq; backref_entry_mset_t backref_entry_mset; @@ -603,6 +690,32 @@ public: length, extent_init_func_t(std::forward(extent_init_func))); } + + /* + * get_absent_extent_by_type + * + * Mostly the same as Cache::get_extent_by_type(), with the only difference + * that get_absent_extent_by_type won't search the transaction's context for + * the specific CachedExtent + */ + template + get_extent_by_type_ret get_absent_extent_by_type( + Transaction &t, ///< [in] transaction + extent_types_t type, ///< [in] type tag + paddr_t offset, ///< [in] starting addr + laddr_t laddr, ///< [in] logical address if logical + extent_len_t length, ///< [in] length + Func &&extent_init_func ///< [in] extent init func + ) { + return _get_absent_extent_by_type( + t, + type, + offset, + laddr, + length, + extent_init_func_t(std::forward(extent_init_func))); + } + get_extent_by_type_ret get_extent_by_type( Transaction &t, extent_types_t type, @@ -614,6 +727,25 @@ public: t, type, offset, laddr, length, [](CachedExtent &) {}); } + + /* + * get_absent_extent_by_type + * + * Mostly the same as Cache::get_extent_by_type(), with the only difference + * that get_absent_extent_by_type won't search the transaction's context for + * the specific CachedExtent + */ + get_extent_by_type_ret get_absent_extent_by_type( + Transaction &t, + extent_types_t type, + paddr_t offset, + laddr_t laddr, + extent_len_t length + ) { + return get_absent_extent_by_type( + t, type, offset, laddr, length, [](CachedExtent &) {}); + } + void trim_backref_bufs(const journal_seq_t &trim_to) { LOG_PREFIX(Cache::trim_backref_bufs); SUBDEBUG(seastore_cache, "trimming to {}", trim_to); diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 6cb18c560fab4..e00290d88e2a2 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -134,7 +134,7 @@ public: static_assert(is_logical_type(T::TYPE)); using ret = pin_to_extent_ret; auto &pref = *pin; - return cache->get_extent( + return cache->get_absent_extent( t, pref.get_val(), pref.get_length(), @@ -168,7 +168,7 @@ public: SUBTRACET(seastore_tm, "getting extent {} type {}", t, *pin, type); assert(is_logical_type(type)); auto &pref = *pin; - return cache->get_extent_by_type( + return cache->get_absent_extent_by_type( t, type, pref.get_val(), -- 2.39.5