From 19d12831f87fc82881457e1c658137125e4559de Mon Sep 17 00:00:00 2001 From: Xuehan Xu Date: Wed, 11 Jun 2025 10:12:58 +0800 Subject: [PATCH] crimson/os/seastore/lba_manager: replace refresh_lba_cursor() with cursor.refresh() Signed-off-by: Xuehan Xu --- src/crimson/os/seastore/btree/btree_types.cc | 76 ++++++++++++++ src/crimson/os/seastore/btree/btree_types.h | 6 ++ src/crimson/os/seastore/cache.cc | 21 ++++ src/crimson/os/seastore/cache.h | 1 + .../os/seastore/lba/btree_lba_manager.cc | 98 ++----------------- .../os/seastore/lba/btree_lba_manager.h | 12 +-- src/crimson/os/seastore/lba_mapping.cc | 26 +++++ src/crimson/os/seastore/lba_mapping.h | 4 + src/crimson/os/seastore/transaction.h | 19 ++++ 9 files changed, 163 insertions(+), 100 deletions(-) diff --git a/src/crimson/os/seastore/btree/btree_types.cc b/src/crimson/os/seastore/btree/btree_types.cc index 7665b4a1b87..f2a473d1546 100644 --- a/src/crimson/os/seastore/btree/btree_types.cc +++ b/src/crimson/os/seastore/btree/btree_types.cc @@ -4,9 +4,85 @@ #include "crimson/os/seastore/btree/btree_types.h" #include "crimson/os/seastore/lba/lba_btree_node.h" #include "crimson/os/seastore/backref/backref_tree_node.h" +#include "crimson/os/seastore/lba/btree_lba_manager.h" namespace crimson::os::seastore { +LBACursor::base_iertr::future<> LBACursor::refresh() +{ + LOG_PREFIX(LBACursor::refresh); + return with_btree( + ctx.cache, + ctx, + [this, FNAME, c=ctx](auto &btree) { + c.trans.cursor_stats.num_refresh_parent_total++; + + if (!parent->is_valid()) { + c.trans.cursor_stats.num_refresh_invalid_parent++; + SUBTRACET( + seastore_lba, + "cursor {} parent is invalid, re-search from scratch", + c.trans, *this); + return btree.lower_bound(c, this->get_laddr() + ).si_then([this](lba::LBABtree::iterator iter) { + auto leaf = iter.get_leaf_node(); + parent = leaf; + modifications = leaf->modifications; + pos = iter.get_leaf_pos(); + if (!is_end()) { + ceph_assert(!iter.is_end()); + ceph_assert(iter.get_key() == get_laddr()); + val = iter.get_val(); + assert(is_viewable()); + } + }); + } + assert(parent->is_stable() || + parent->is_pending_in_trans(c.trans.get_trans_id())); + auto leaf = parent->cast(); + if (leaf->is_pending_in_trans(c.trans.get_trans_id())) { + if (leaf->modified_since(modifications)) { + c.trans.cursor_stats.num_refresh_modified_viewable_parent++; + } else { + // no need to refresh + return base_iertr::now(); + } + } else { + auto [viewable, l] = leaf->resolve_transaction(c.trans, key); + SUBTRACET( + seastore_lba, + "cursor: {} viewable: {}", + c.trans, *this, viewable); + if (!viewable) { + leaf = l; + c.trans.cursor_stats.num_refresh_unviewable_parent++; + parent = leaf; + } else { + assert(leaf.get() == l.get()); + assert(leaf->is_stable()); + return base_iertr::now(); + } + } + + modifications = leaf->modifications; + if (is_end()) { + pos = leaf->get_size(); + assert(!val); + } else { + auto i = leaf->lower_bound(get_laddr()); + pos = i.get_offset(); + val = i.get_val(); + + auto iter = lba::LBALeafNode::iterator(leaf.get(), pos); + ceph_assert(iter.get_key() == key); + ceph_assert(iter.get_val() == val); + assert(is_viewable()); + } + + return base_iertr::now(); + }); +} + namespace lba { std::ostream& operator<<(std::ostream& out, const lba_map_val_t& v) diff --git a/src/crimson/os/seastore/btree/btree_types.h b/src/crimson/os/seastore/btree/btree_types.h index e3e8ca1f7e6..9266a3ad04b 100644 --- a/src/crimson/os/seastore/btree/btree_types.h +++ b/src/crimson/os/seastore/btree/btree_types.h @@ -279,9 +279,15 @@ struct LBACursor : BtreeCursor { assert(!is_indirect()); return val->refcount; } + std::unique_ptr duplicate() const { return std::make_unique(*this); } + + using base_ertr = crimson::errorator< + crimson::ct_error::input_output_error>; + using base_iertr = trans_iertr; + base_iertr::future<> refresh(); }; using LBACursorRef = std::unique_ptr; diff --git a/src/crimson/os/seastore/cache.cc b/src/crimson/os/seastore/cache.cc index c0ad5ab5e05..33660d34082 100644 --- a/src/crimson/os/seastore/cache.cc +++ b/src/crimson/os/seastore/cache.cc @@ -213,6 +213,26 @@ void Cache::register_metrics() }, sm::description("total number of cache hits") ), + sm::make_counter( + "refresh_parent_total", + cursor_stats.num_refresh_parent_total, + sm::description("total number of refreshed cursors") + ), + sm::make_counter( + "refresh_invalid_parent", + cursor_stats.num_refresh_invalid_parent, + sm::description("total number of refreshed cursors with invalid parents") + ), + sm::make_counter( + "refresh_unviewable_parent", + cursor_stats.num_refresh_unviewable_parent, + sm::description("total number of refreshed cursors with unviewable parents") + ), + sm::make_counter( + "refresh_modified_viewable_parent", + cursor_stats.num_refresh_modified_viewable_parent, + sm::description("total number of refreshed cursors with viewable but modified parents") + ), } ); @@ -1743,6 +1763,7 @@ record_t Cache::prepare_record( assert(rewrite_stats.is_clear()); } + cursor_stats.apply(t.cursor_stats); return record; } diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index 7227faebcd5..0ae19c34317 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -1670,6 +1670,7 @@ private: uint64_t hit = 0; }; + btree_cursor_stats_t cursor_stats; struct invalid_trans_efforts_t { io_stat_t read; io_stat_t mutate; diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.cc b/src/crimson/os/seastore/lba/btree_lba_manager.cc index e019211c826..bd809fa5798 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.cc +++ b/src/crimson/os/seastore/lba/btree_lba_manager.cc @@ -737,18 +737,18 @@ BtreeLBAManager::refresh_lba_mapping(Transaction &t, LBAMapping mapping) cache, c, std::move(mapping), - [c, this](LBABtree &btree, LBAMapping &mapping) mutable + [](LBABtree &btree, LBAMapping &mapping) mutable { - return seastar::futurize_invoke([c, this, &btree, &mapping] { + return seastar::futurize_invoke([&mapping] { if (mapping.direct_cursor) { - return refresh_lba_cursor(c, btree, *mapping.direct_cursor); + return mapping.direct_cursor->refresh(); } - return refresh_lba_cursor_iertr::make_ready_future(); - }).si_then([c, this, &btree, &mapping] { + return base_iertr::now(); + }).si_then([&mapping] { if (mapping.indirect_cursor) { - return refresh_lba_cursor(c, btree, *mapping.indirect_cursor); + return mapping.indirect_cursor->refresh(); } - return refresh_lba_cursor_iertr::make_ready_future(); + return base_iertr::now(); #ifndef NDEBUG }).si_then([&mapping] { assert(mapping.is_viewable()); @@ -757,68 +757,6 @@ BtreeLBAManager::refresh_lba_mapping(Transaction &t, LBAMapping mapping) }); } -BtreeLBAManager::refresh_lba_cursor_ret -BtreeLBAManager::refresh_lba_cursor( - op_context_t c, - LBABtree &btree, - LBACursor &cursor) -{ - LOG_PREFIX(BtreeLBAManager::refresh_lba_cursor); - stats.num_refresh_parent_total++; - - if (!cursor.parent->is_valid()) { - stats.num_refresh_invalid_parent++; - TRACET("cursor {} parent is invalid, re-search from scratch", - c.trans, cursor); - return btree.lower_bound(c, cursor.get_laddr() - ).si_then([&cursor](LBABtree::iterator iter) { - auto leaf = iter.get_leaf_node(); - cursor.parent = leaf; - cursor.modifications = leaf->modifications; - cursor.pos = iter.get_leaf_pos(); - if (!cursor.is_end()) { - ceph_assert(!iter.is_end()); - ceph_assert(iter.get_key() == cursor.get_laddr()); - cursor.val = iter.get_val(); - assert(cursor.is_viewable()); - } - }); - } - - auto leaf = cursor.parent->cast(); - auto [viewable, l] = leaf->resolve_transaction(c.trans, cursor.key); - TRACET("cursor: {} viewable: {}", c.trans, cursor, viewable); - if (!viewable) { - leaf = l; - stats.num_refresh_unviewable_parent++; - cursor.parent = leaf; - } - - if (!viewable || - leaf->modified_since(cursor.modifications)) { - if (viewable) { - stats.num_refresh_modified_viewable_parent++; - } - - cursor.modifications = leaf->modifications; - if (cursor.is_end()) { - cursor.pos = leaf->get_size(); - assert(!cursor.val); - } else { - auto i = leaf->lower_bound(cursor.get_laddr()); - cursor.pos = i.get_offset(); - cursor.val = i.get_val(); - - auto iter = LBALeafNode::iterator(leaf.get(), cursor.pos); - ceph_assert(iter.get_key() == cursor.key); - ceph_assert(iter.get_val() == cursor.val); - assert(cursor.is_viewable()); - } - } - - return refresh_lba_cursor_iertr::make_ready_future(); -} - void BtreeLBAManager::register_metrics() { LOG_PREFIX(BtreeLBAManager::register_metrics); @@ -838,26 +776,6 @@ void BtreeLBAManager::register_metrics() stats.num_alloc_extents_iter_nexts, sm::description("total number of iterator next operations during extent allocation") ), - sm::make_counter( - "refresh_parent_total", - stats.num_refresh_parent_total, - sm::description("total number of refreshed cursors") - ), - sm::make_counter( - "refresh_invalid_parent", - stats.num_refresh_invalid_parent, - sm::description("total number of refreshed cursors with invalid parents") - ), - sm::make_counter( - "refresh_unviewable_parent", - stats.num_refresh_unviewable_parent, - sm::description("total number of refreshed cursors with unviewable parents") - ), - sm::make_counter( - "refresh_modified_viewable_parent", - stats.num_refresh_modified_viewable_parent, - sm::description("total number of refreshed cursors with viewable but modified parents") - ), } ); } @@ -1104,3 +1022,5 @@ BtreeLBAManager::_update_mapping( } } + +} diff --git a/src/crimson/os/seastore/lba/btree_lba_manager.h b/src/crimson/os/seastore/lba/btree_lba_manager.h index d5f2adb8129..a3df2b30401 100644 --- a/src/crimson/os/seastore/lba/btree_lba_manager.h +++ b/src/crimson/os/seastore/lba/btree_lba_manager.h @@ -28,6 +28,7 @@ class LogicalCachedExtent; } namespace crimson::os::seastore::lba { +class BtreeLBAManager; using LBABtree = FixedKVBtree< laddr_t, lba_map_val_t, LBAInternalNode, @@ -274,10 +275,6 @@ private: struct { uint64_t num_alloc_extents = 0; uint64_t num_alloc_extents_iter_nexts = 0; - uint64_t num_refresh_parent_total = 0; - uint64_t num_refresh_invalid_parent = 0; - uint64_t num_refresh_unviewable_parent = 0; - uint64_t num_refresh_modified_viewable_parent = 0; } stats; struct alloc_mapping_info_t { @@ -525,13 +522,6 @@ private: Transaction &t, laddr_t addr, extent_len_t len); - - using refresh_lba_cursor_iertr = base_iertr; - using refresh_lba_cursor_ret = refresh_lba_cursor_iertr::future<>; - refresh_lba_cursor_ret refresh_lba_cursor( - op_context_t c, - LBABtree &btree, - LBACursor &cursor); }; using BtreeLBAManagerRef = std::unique_ptr; diff --git a/src/crimson/os/seastore/lba_mapping.cc b/src/crimson/os/seastore/lba_mapping.cc index f475fd47d7b..63acafbb262 100644 --- a/src/crimson/os/seastore/lba_mapping.cc +++ b/src/crimson/os/seastore/lba_mapping.cc @@ -2,6 +2,7 @@ // vim: ts=8 sw=2 smarttab #include "crimson/os/seastore/lba_mapping.h" +#include "crimson/os/seastore/lba/btree_lba_manager.h" namespace crimson::os::seastore { @@ -76,4 +77,29 @@ bool LBAMapping::is_data_stable() const { direct_cursor->key); } +LBAMapping::refresh_iertr::future LBAMapping::refresh() +{ + if (is_viewable()) { + return refresh_iertr::make_ready_future(*this); + } + return seastar::do_with( + direct_cursor, + indirect_cursor, + [](auto &direct_cursor, auto &indirect_cursor) { + return seastar::futurize_invoke([&direct_cursor] { + if (direct_cursor) { + return direct_cursor->refresh(); + } + return refresh_iertr::now(); + }).si_then([&indirect_cursor] { + if (indirect_cursor) { + return indirect_cursor->refresh(); + } + return refresh_iertr::now(); + }).si_then([&direct_cursor, &indirect_cursor] { + return LBAMapping(direct_cursor, indirect_cursor); + }); + }); +} + } // namespace crimson::os::seastore diff --git a/src/crimson/os/seastore/lba_mapping.h b/src/crimson/os/seastore/lba_mapping.h index a9342bfd4b1..beed476d888 100644 --- a/src/crimson/os/seastore/lba_mapping.h +++ b/src/crimson/os/seastore/lba_mapping.h @@ -130,6 +130,10 @@ public: }; return LBAMapping(dup_iter(direct_cursor), dup_iter(indirect_cursor)); } + + using refresh_iertr = LBACursor::base_iertr; + refresh_iertr::future refresh(); + private: friend lba::BtreeLBAManager; friend class TransactionManager; diff --git a/src/crimson/os/seastore/transaction.h b/src/crimson/os/seastore/transaction.h index 37f947c7341..d1817b4e273 100644 --- a/src/crimson/os/seastore/transaction.h +++ b/src/crimson/os/seastore/transaction.h @@ -81,6 +81,24 @@ struct rewrite_stats_t { } }; +struct btree_cursor_stats_t { + uint64_t num_refresh_parent_total = 0; + uint64_t num_refresh_invalid_parent = 0; + uint64_t num_refresh_unviewable_parent = 0; + uint64_t num_refresh_modified_viewable_parent = 0; + + void apply(btree_cursor_stats_t &stats) { + num_refresh_parent_total += + stats.num_refresh_parent_total; + num_refresh_invalid_parent += + stats.num_refresh_invalid_parent; + num_refresh_unviewable_parent += + stats.num_refresh_unviewable_parent; + num_refresh_modified_viewable_parent += + stats.num_refresh_modified_viewable_parent; + } +}; + struct rbm_pending_ool_t { bool is_conflicted = false; std::list pending_extents; @@ -603,6 +621,7 @@ public: return cache_hint; } + btree_cursor_stats_t cursor_stats; private: friend class Cache; friend Ref make_test_transaction(); -- 2.39.5