From da5e195cb4d8c2e2299ab86da156e98cda16496b Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Wed, 20 Nov 2024 10:18:58 +0800 Subject: [PATCH] crimson/os/seastore: convert transaction related paths with interruptor Signed-off-by: Yingxin Cheng --- .../seastore/backref/btree_backref_manager.cc | 14 ++----- .../os/seastore/btree/fixed_kv_btree.h | 10 ++--- src/crimson/os/seastore/cache.h | 40 +++++++++++-------- src/crimson/os/seastore/cached_extent.h | 17 ++++---- .../lba_manager/btree/btree_lba_manager.cc | 14 ++----- src/crimson/os/seastore/transaction_manager.h | 5 ++- .../seastore/test_transaction_manager.cc | 8 ++-- 7 files changed, 51 insertions(+), 57 deletions(-) diff --git a/src/crimson/os/seastore/backref/btree_backref_manager.cc b/src/crimson/os/seastore/backref/btree_backref_manager.cc index f89698d602a..9cbf65f4033 100644 --- a/src/crimson/os/seastore/backref/btree_backref_manager.cc +++ b/src/crimson/os/seastore/backref/btree_backref_manager.cc @@ -28,28 +28,22 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node< ceph_assert(backref_root->is_initial_pending() == root_block->is_pending()); return {true, - trans_intr::make_interruptible( - c.cache.get_extent_viewable_by_trans(c.trans, backref_root))}; + c.cache.get_extent_viewable_by_trans(c.trans, backref_root)}; } else if (root_block->is_pending()) { auto &prior = static_cast(*root_block->get_prior_instance()); backref_root = prior.backref_root_node; if (backref_root) { return {true, - trans_intr::make_interruptible( - c.cache.get_extent_viewable_by_trans(c.trans, backref_root))}; + c.cache.get_extent_viewable_by_trans(c.trans, backref_root)}; } else { c.cache.account_absent_access(c.trans.get_src()); return {false, - trans_intr::make_interruptible( - Cache::get_extent_ertr::make_ready_future< - CachedExtentRef>())}; + Cache::get_extent_iertr::make_ready_future()}; } } else { c.cache.account_absent_access(c.trans.get_src()); return {false, - trans_intr::make_interruptible( - Cache::get_extent_ertr::make_ready_future< - CachedExtentRef>())}; + 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 cb4fff32750..04ebcc7e2ca 100644 --- a/src/crimson/os/seastore/btree/fixed_kv_btree.h +++ b/src/crimson/os/seastore/btree/fixed_kv_btree.h @@ -32,10 +32,6 @@ inline ChildableCachedExtent* get_reserved_ptr() { template phy_tree_root_t& get_phy_tree_root(root_t& r); -using get_child_iertr = - ::crimson::interruptible::interruptible_errorator< - typename trans_intr::condition, - get_child_ertr>; using get_phy_tree_root_node_ret = std::pair>; @@ -1501,7 +1497,7 @@ private: // checking the lba child must be atomic with creating // and linking the absent child if (v.has_child()) { - return trans_intr::make_interruptible(std::move(v.get_child_fut()) + return std::move(v.get_child_fut() ).si_then([on_found=std::move(on_found), node_iter, c, parent_entry](auto child) { LOG_PREFIX(FixedKVBtree::lookup_internal_level); @@ -1571,7 +1567,7 @@ private: // checking the lba child must be atomic with creating // and linking the absent child if (v.has_child()) { - return trans_intr::make_interruptible(std::move(v.get_child_fut()) + return std::move(v.get_child_fut() ).si_then([on_found=std::move(on_found), node_iter, c, parent_entry](auto child) { LOG_PREFIX(FixedKVBtree::lookup_leaf); @@ -2126,7 +2122,7 @@ private: // checking the lba child must be atomic with creating // and linking the absent child if (v.has_child()) { - return trans_intr::make_interruptible(std::move(v.get_child_fut()) + return std::move(v.get_child_fut() ).si_then([do_merge=std::move(do_merge), &pos, donor_iter, donor_is_left, c, parent_pos](auto child) { LOG_PREFIX(FixedKVBtree::merge_level); diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index b8542c393b0..520df473f2f 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -527,8 +527,7 @@ public: return view->is_data_stable(); } - using get_extent_ertr = base_ertr; - get_extent_ertr::future + get_extent_iertr::future get_extent_viewable_by_trans( Transaction &t, CachedExtentRef extent) @@ -553,7 +552,7 @@ public: if (p_extent->is_mutable()) { assert(p_extent->is_fully_loaded()); assert(!p_extent->is_pending_io()); - return get_extent_ertr::make_ready_future( + return get_extent_iertr::make_ready_future( CachedExtentRef(p_extent)); } else { assert(p_extent->is_exist_clean()); @@ -588,7 +587,7 @@ public: if (extent->is_mutable()) { assert(extent->is_fully_loaded()); assert(!extent->is_pending_io()); - return get_extent_ertr::make_ready_future(extent); + return get_extent_iertr::make_ready_future(extent); } else { assert(extent->is_exist_clean()); p_extent = extent.get(); @@ -601,30 +600,31 @@ public: // also see read_extent_maybe_partial() and get_absent_extent() assert(is_logical_type(p_extent->get_type()) || p_extent->is_fully_loaded()); - return p_extent->wait_io( - ).then([p_extent] { - return get_extent_ertr::make_ready_future( + + return trans_intr::make_interruptible( + p_extent->wait_io() + ).then_interruptible([p_extent] { + return get_extent_iertr::make_ready_future( CachedExtentRef(p_extent)); }); } template - using read_extent_ret = get_extent_ertr::future>; - - template - read_extent_ret get_extent_viewable_by_trans( + get_extent_iertr::future> + get_extent_viewable_by_trans( Transaction &t, TCachedExtentRef extent) { return get_extent_viewable_by_trans(t, CachedExtentRef(extent.get()) - ).safe_then([](auto p_extent) { + ).si_then([](auto p_extent) { return p_extent->template cast(); }); } // wait extent io or do partial reads template - read_extent_ret read_extent_maybe_partial( + get_extent_iertr::future> + read_extent_maybe_partial( Transaction &t, TCachedExtentRef extent, extent_len_t partial_off, @@ -642,13 +642,16 @@ public: extent->get_type()); ++access_stats.load_present; ++stats.access.s.load_present; - return do_read_extent_maybe_partial( - std::move(extent), partial_off, partial_len, &t_src); + return trans_intr::make_interruptible( + do_read_extent_maybe_partial( + std::move(extent), partial_off, partial_len, &t_src)); } else { // TODO(implement fine-grained-wait): // the range might be already loaded, but we don't know - return extent->wait_io().then([extent]() -> read_extent_ret { - return seastar::make_ready_future>(extent); + return trans_intr::make_interruptible( + extent->wait_io() + ).then_interruptible([extent] { + return get_extent_iertr::make_ready_future>(extent); }); } } @@ -664,6 +667,9 @@ public: } private: + using get_extent_ertr = base_ertr; + template + using read_extent_ret = get_extent_ertr::future>; /// Implements exclusive call to read_extent() for the extent template read_extent_ret do_read_extent_maybe_partial( diff --git a/src/crimson/os/seastore/cached_extent.h b/src/crimson/os/seastore/cached_extent.h index 0df66e37d11..2a54e8ded77 100644 --- a/src/crimson/os/seastore/cached_extent.h +++ b/src/crimson/os/seastore/cached_extent.h @@ -12,9 +12,8 @@ #include "seastar/core/shared_future.hh" #include "include/buffer.h" -#include "crimson/common/errorator.h" -#include "crimson/common/interruptible_future.h" #include "crimson/os/seastore/seastore_types.h" +#include "crimson/os/seastore/transaction_interruptor.h" struct btree_lba_manager_test; struct lba_btree_test; @@ -23,7 +22,6 @@ struct cache_test_t; namespace crimson::os::seastore { -class Transaction; class CachedExtent; using CachedExtentRef = boost::intrusive_ptr; class SegmentedAllocator; @@ -1303,14 +1301,17 @@ private: uint16_t pos = std::numeric_limits::max(); }; -using get_child_ertr = crimson::errorator< - crimson::ct_error::input_output_error>; +using get_child_iertr = trans_iertr>; +template +using get_child_ifut = get_child_iertr::future>; + template struct get_child_ret_t { - std::variant>> ret; + std::variant> ret; get_child_ret_t(child_pos_t pos) : ret(std::move(pos)) {} - get_child_ret_t(get_child_ertr::future> child) + get_child_ret_t(get_child_ifut child) : ret(std::move(child)) {} bool has_child() const { @@ -1322,7 +1323,7 @@ struct get_child_ret_t { return std::get<0>(ret); } - get_child_ertr::future> &get_child_fut() { + get_child_ifut &get_child_fut() { ceph_assert(ret.index() == 1); return std::get<1>(ret); } 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 b7a1d8f8ba9..007737ff450 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 @@ -52,28 +52,22 @@ const get_phy_tree_root_node_ret get_phy_tree_root_node< ceph_assert(lba_root->is_initial_pending() == root_block->is_pending()); return {true, - trans_intr::make_interruptible( - c.cache.get_extent_viewable_by_trans(c.trans, lba_root))}; + c.cache.get_extent_viewable_by_trans(c.trans, lba_root)}; } else if (root_block->is_pending()) { auto &prior = static_cast(*root_block->get_prior_instance()); lba_root = prior.lba_root_node; if (lba_root) { return {true, - trans_intr::make_interruptible( - c.cache.get_extent_viewable_by_trans(c.trans, lba_root))}; + c.cache.get_extent_viewable_by_trans(c.trans, lba_root)}; } else { c.cache.account_absent_access(c.trans.get_src()); return {false, - trans_intr::make_interruptible( - Cache::get_extent_ertr::make_ready_future< - CachedExtentRef>())}; + Cache::get_extent_iertr::make_ready_future()}; } } else { c.cache.account_absent_access(c.trans.get_src()); return {false, - trans_intr::make_interruptible( - Cache::get_extent_ertr::make_ready_future< - CachedExtentRef>())}; + Cache::get_extent_iertr::make_ready_future()}; } } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index c9ce635ee8c..5bd3c679dda 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -954,7 +954,7 @@ private: shard_stats_t& shard_stats; template - std::variant>> + std::variant> get_extent_if_linked( Transaction &t, LBAMappingRef pin) @@ -964,7 +964,8 @@ private: // and linking the absent child auto v = pin->get_logical_extent(t); if (v.has_child()) { - return v.get_child_fut().safe_then([pin=std::move(pin)](auto extent) { + return v.get_child_fut( + ).si_then([pin=std::move(pin)](auto extent) { #ifndef NDEBUG auto lextent = extent->template cast(); auto pin_laddr = pin->get_key(); diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 012d1819d85..6e0fe65c345 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -2209,9 +2209,11 @@ TEST_P(tm_single_device_test_t, invalid_lba_mapping_detect) assert(pin->is_parent_viewable()); assert(pin->parent_modified()); pin->maybe_fix_pos(); - auto v = pin->get_logical_extent(*t.t); - assert(v.has_child()); - auto extent2 = v.get_child_fut().unsafe_get(); + auto extent2 = with_trans_intr(*(t.t), [&pin](auto& trans) { + auto v = pin->get_logical_extent(trans); + assert(v.has_child()); + return std::move(v.get_child_fut()); + }).unsafe_get(); assert(extent.get() == extent2.get()); submit_transaction(std::move(t)); } -- 2.39.5