From: Yingxin Cheng Date: Mon, 1 Aug 2022 06:10:38 +0000 (+0800) Subject: crimson/os/seastore: reuse with_transaction_intr for weak trans X-Git-Tag: v18.0.0~318^2~4 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=cd0dc13a50deb48fbf4fd3bb9963fd0566a29f3c;p=ceph.git crimson/os/seastore: reuse with_transaction_intr for weak trans Signed-off-by: Yingxin Cheng --- diff --git a/src/crimson/os/seastore/async_cleaner.h b/src/crimson/os/seastore/async_cleaner.h index 294a8e0c3c9..8e9c7b33fad 100644 --- a/src/crimson/os/seastore/async_cleaner.h +++ b/src/crimson/os/seastore/async_cleaner.h @@ -592,8 +592,10 @@ public: public: virtual ~ExtentCallbackInterface() = default; + /// Creates empty transaction + /// weak transaction should be type READ virtual TransactionRef create_transaction( - Transaction::src_t, const char*) = 0; + Transaction::src_t, const char *name, bool is_weak=false) = 0; /// Creates empty transaction with interruptible context template @@ -601,16 +603,21 @@ public: Transaction::src_t src, const char* name, Func &&f) { - return seastar::do_with( - create_transaction(src, name), - [f=std::forward(f)](auto &ref_t) mutable { - return with_trans_intr( - *ref_t, - [f=std::forward(f)](auto& t) mutable { - return f(t); - } - ); - } + return do_with_transaction_intr( + src, name, std::forward(f)); + } + + template + auto with_transaction_weak( + const char* name, + Func &&f) { + return do_with_transaction_intr( + Transaction::src_t::READ, name, std::forward(f) + ).handle_error( + crimson::ct_error::eagain::handle([] { + ceph_assert(0 == "eagain impossible"); + }), + crimson::ct_error::pass_further_all{} ); } @@ -684,6 +691,25 @@ public: virtual submit_transaction_direct_ret submit_transaction_direct( Transaction &t, std::optional seq_to_trim = std::nullopt) = 0; + + private: + template + auto do_with_transaction_intr( + Transaction::src_t src, + const char* name, + Func &&f) { + return seastar::do_with( + create_transaction(src, name, IsWeak), + [f=std::forward(f)](auto &ref_t) mutable { + return with_trans_intr( + *ref_t, + [f=std::forward(f)](auto& t) mutable { + return f(t); + } + ); + } + ); + } }; private: diff --git a/src/crimson/os/seastore/cache.h b/src/crimson/os/seastore/cache.h index d4e58346537..01e163afe35 100644 --- a/src/crimson/os/seastore/cache.h +++ b/src/crimson/os/seastore/cache.h @@ -237,6 +237,7 @@ public: ); SUBDEBUGT(seastore_t, "created name={}, source={}, is_weak={}", *ret, name, src, is_weak); + assert(!is_weak || src == Transaction::src_t::READ); return ret; } diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 81c79524a3b..7f1d8daf131 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -110,72 +110,67 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount() return journal->open_for_mount(); }).safe_then([this, FNAME](auto start_seq) { async_cleaner->set_journal_head(start_seq); - return seastar::do_with( - create_weak_transaction( - Transaction::src_t::READ, "mount"), - [this, FNAME](auto &tref) { - return with_trans_intr( - *tref, - [this, FNAME](auto &t) { - return cache->init_cached_extents(t, [this](auto &t, auto &e) { - if (is_backref_node(e->get_type())) - return backref_manager->init_cached_extent(t, e); - else - return lba_manager->init_cached_extent(t, e); - }).si_then([this, FNAME, &t] { - assert(async_cleaner->debug_check_space( - *async_cleaner->get_empty_space_tracker())); - return backref_manager->scan_mapped_space( - t, - [this, FNAME, &t]( - paddr_t addr, - extent_len_t len, - depth_t depth, - extent_types_t type) - { - TRACET( - "marking {}~{} used", - t, - addr, - len); - async_cleaner->mark_space_used( - addr, - len , - /* init_scan = */ true); - if (is_backref_node(type)) { - ceph_assert(depth); - backref_manager->cache_new_backref_extent(addr, type); - cache->update_tree_extents_num(type, 1); - return seastar::now(); - } else { - ceph_assert(!depth); - cache->update_tree_extents_num(type, 1); - return seastar::now(); - } - }).si_then([this, &t] { - LOG_PREFIX(TransactionManager::mount); - auto &backrefs = backref_manager->get_cached_backrefs(); - DEBUGT("scan backref cache", t); - for (auto &backref : backrefs) { - if (backref.laddr == L_ADDR_NULL) { - async_cleaner->mark_space_free( - backref.paddr, - backref.len, - true); - cache->update_tree_extents_num(backref.type, -1); - } else { - async_cleaner->mark_space_used( - backref.paddr, - backref.len, - true); - cache->update_tree_extents_num(backref.type, 1); - } - } - return seastar::now(); - }); - }); - }); + return with_transaction_weak( + "mount", + [this, FNAME](auto &t) + { + return cache->init_cached_extents(t, [this](auto &t, auto &e) { + if (is_backref_node(e->get_type())) { + return backref_manager->init_cached_extent(t, e); + } else { + return lba_manager->init_cached_extent(t, e); + } + }).si_then([this, FNAME, &t] { + assert(async_cleaner->debug_check_space( + *async_cleaner->get_empty_space_tracker())); + return backref_manager->scan_mapped_space( + t, + [this, FNAME, &t]( + paddr_t addr, + extent_len_t len, + depth_t depth, + extent_types_t type) { + TRACET( + "marking {}~{} used", + t, + addr, + len); + async_cleaner->mark_space_used( + addr, + len , + /* init_scan = */ true); + if (is_backref_node(type)) { + ceph_assert(depth); + backref_manager->cache_new_backref_extent(addr, type); + cache->update_tree_extents_num(type, 1); + return seastar::now(); + } else { + ceph_assert(!depth); + cache->update_tree_extents_num(type, 1); + return seastar::now(); + } + }); + }).si_then([this, FNAME, &t] { + auto &backrefs = backref_manager->get_cached_backrefs(); + DEBUGT("scan backref cache", t); + for (auto &backref : backrefs) { + if (backref.laddr == L_ADDR_NULL) { + async_cleaner->mark_space_free( + backref.paddr, + backref.len, + true); + cache->update_tree_extents_num(backref.type, -1); + } else { + async_cleaner->mark_space_used( + backref.paddr, + backref.len, + true); + cache->update_tree_extents_num(backref.type, 1); + } + } + return seastar::now(); }); + }); }).safe_then([this] { return epm->open(); }).safe_then([FNAME, this] { @@ -186,7 +181,8 @@ TransactionManager::mount_ertr::future<> TransactionManager::mount() crimson::ct_error::all_same_way([] { ceph_assert(0 == "unhandled error"); return mount_ertr::now(); - })); + }) + ); } TransactionManager::close_ertr::future<> TransactionManager::close() { diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index e123e14cf89..28cf66880c6 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -130,17 +130,12 @@ public: close_ertr::future<> close(); /// Creates empty transaction + /// weak transaction should be type READ TransactionRef create_transaction( Transaction::src_t src, - const char* name) final { - return cache->create_transaction(src, name, false); - } - - /// Creates empty weak transaction - TransactionRef create_weak_transaction( - Transaction::src_t src, - const char* name) { - return cache->create_transaction(src, name, true); + const char* name, + bool is_weak=false) final { + return cache->create_transaction(src, name, is_weak); } /// Resets transaction diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h index f288a074a0d..51765401477 100644 --- a/src/test/crimson/seastore/transaction_manager_test_state.h +++ b/src/test/crimson/seastore/transaction_manager_test_state.h @@ -246,8 +246,8 @@ protected: } auto create_weak_transaction() { - return tm->create_weak_transaction( - Transaction::src_t::READ, "test_read_weak"); + return tm->create_transaction( + Transaction::src_t::READ, "test_read_weak", true); } auto submit_transaction_fut2(Transaction& t) {