From: Samuel Just Date: Sat, 19 Jun 2021 07:43:27 +0000 (-0700) Subject: crimson/os/seastore/transaction_manager: pass t by ref to submit_transaction X-Git-Tag: v17.1.0~1567^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=3ef4040c4199fdd20f5f5c390d4ec75cb336d0a0;p=ceph.git crimson/os/seastore/transaction_manager: pass t by ref to submit_transaction Signed-off-by: Samuel Just --- diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index cf0b68516a6ab..20f36667dde90 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -98,7 +98,7 @@ seastar::future<> SeaStore::mkfs(uuid_d new_osd_fsid) *t, coll_root); return transaction_manager->submit_transaction( - std::move(t)); + *t); }); }); }).safe_then([this] { @@ -637,7 +637,7 @@ seastar::future<> SeaStore::do_transaction( // destruction in debug mode, which need to be done before calling // submit_transaction(). ctx.onodes.clear(); - return transaction_manager->submit_transaction(std::move(ctx.transaction)); + return transaction_manager->submit_transaction(*ctx.transaction); }).safe_then([&ctx]() { for (auto i : { ctx.ext_transaction.get_on_applied(), @@ -1067,7 +1067,7 @@ seastar::future<> SeaStore::write_meta(const std::string& key, return transaction_manager->update_root_meta( *t, key, value ).safe_then([this, &t] { - return transaction_manager->submit_transaction(std::move(t)); + return transaction_manager->submit_transaction(*t); }); }); }).handle_error( diff --git a/src/crimson/os/seastore/segment_cleaner.cc b/src/crimson/os/seastore/segment_cleaner.cc index 59d4610c30580..b0815d029c60f 100644 --- a/src/crimson/os/seastore/segment_cleaner.cc +++ b/src/crimson/os/seastore/segment_cleaner.cc @@ -288,9 +288,9 @@ SegmentCleaner::gc_trim_journal_ret SegmentCleaner::gc_trim_journal() [this](auto &tref) { return with_trans_intr(*tref, [this, &tref](auto &t) { return rewrite_dirty(t, get_dirty_tail() - ).si_then([this, &tref] { + ).si_then([this, &t] { return ecb->submit_transaction_direct( - std::move(tref)); + t); }); }); }); @@ -363,11 +363,11 @@ SegmentCleaner::gc_reclaim_space_ret SegmentCleaner::gc_reclaim_space() } }); } - ).si_then([this, &tref] { + ).si_then([this, &t] { if (scan_cursor->is_complete()) { - tref->mark_segment_to_release(scan_cursor->get_offset().segment); + t.mark_segment_to_release(scan_cursor->get_offset().segment); } - return ecb->submit_transaction_direct(std::move(tref)); + return ecb->submit_transaction_direct(t); }); }); }); diff --git a/src/crimson/os/seastore/segment_cleaner.h b/src/crimson/os/seastore/segment_cleaner.h index 914c6597f1f36..256225d7f5400 100644 --- a/src/crimson/os/seastore/segment_cleaner.h +++ b/src/crimson/os/seastore/segment_cleaner.h @@ -337,7 +337,7 @@ public: using submit_transaction_direct_ret = submit_transaction_direct_iertr::future<>; virtual submit_transaction_direct_ret submit_transaction_direct( - TransactionRef t) = 0; + Transaction &t) = 0; }; private: diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 1c540e8761176..0b0216546a069 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -50,7 +50,7 @@ TransactionManager::mkfs_ertr::future<> TransactionManager::mkfs() return with_trans_intr( *transaction, [this, FNAME, &transaction](auto&) { - return submit_transaction_direct(std::move(transaction)); + return submit_transaction_direct(*transaction); } ).handle_error( crimson::ct_error::eagain::handle([] { @@ -219,27 +219,25 @@ TransactionManager::refs_ret TransactionManager::dec_ref( TransactionManager::submit_transaction_iertr::future<> TransactionManager::submit_transaction( - TransactionRef t) + Transaction &t) { LOG_PREFIX(TransactionManager::submit_transaction); - DEBUGT("about to await throttle", *t); - auto &tref = *t; + DEBUGT("about to await throttle", t); return trans_intr::make_interruptible( - tref.handle.enter(write_pipeline.wait_throttle) + t.handle.enter(write_pipeline.wait_throttle) ).then_interruptible([this] { return trans_intr::make_interruptible(segment_cleaner->await_hard_limits()); - }).then_interruptible([this, t=std::move(t)]() mutable { - return submit_transaction_direct(std::move(t)); + }).then_interruptible([this, &t]() { + return submit_transaction_direct(t); }); } TransactionManager::submit_transaction_direct_ret TransactionManager::submit_transaction_direct( - TransactionRef t) + Transaction &tref) { LOG_PREFIX(TransactionManager::submit_transaction_direct); - DEBUGT("about to prepare", *t); - auto &tref = *t; + DEBUGT("about to prepare", tref); return trans_intr::make_interruptible( tref.handle.enter(write_pipeline.prepare) ).then_interruptible([this, FNAME, &tref]() mutable @@ -274,8 +272,8 @@ TransactionManager::submit_transaction_direct( crimson::ct_error::all_same_way([](auto e) { ceph_assert(0 == "Hit error submitting to journal"); })); - }).finally([t=std::move(t)]() mutable { - t->handle.exit(); + }).finally([&tref]() { + tref.handle.exit(); }); } diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 8c3ac173fce07..f0e588a3078e3 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -378,12 +378,12 @@ public: * Atomically submits transaction to persistence */ using submit_transaction_iertr = base_iertr; - submit_transaction_iertr::future<> submit_transaction(TransactionRef); + submit_transaction_iertr::future<> submit_transaction(Transaction &); /// SegmentCleaner::ExtentCallbackInterface using SegmentCleaner::ExtentCallbackInterface::submit_transaction_direct_ret; submit_transaction_direct_ret submit_transaction_direct( - TransactionRef t) final; + Transaction &t) final; using SegmentCleaner::ExtentCallbackInterface::get_next_dirty_extents_ret; get_next_dirty_extents_ret get_next_dirty_extents( @@ -623,15 +623,7 @@ public: INT_FORWARD(reserve_region) INT_FORWARD(find_hole) PARAM_INT_FORWARD(alloc_extents) - - - auto submit_transaction(TransactionRef t) const { - return with_trans_intr( - *t, - [this, t=std::move(t)](auto &) mutable { - return tm.submit_transaction(std::move(t)); - }); - } + INT_FORWARD(submit_transaction) INT_FORWARD(read_root_meta) INT_FORWARD(update_root_meta) diff --git a/src/crimson/tools/store_nbd/tm_driver.cc b/src/crimson/tools/store_nbd/tm_driver.cc index 8cabeecb045dc..64271bc0ee204 100644 --- a/src/crimson/tools/store_nbd/tm_driver.cc +++ b/src/crimson/tools/store_nbd/tm_driver.cc @@ -43,7 +43,7 @@ seastar::future<> TMDriver::write( assert(ext->get_bptr().length() == ptr.length()); ext->get_bptr().swap(ptr); logger().debug("submitting transaction"); - return tm->submit_transaction(std::move(t)); + return tm->submit_transaction(*t); }); }); }).handle_error( diff --git a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc index 9a314dcb5f94f..362edac7e17d9 100644 --- a/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc +++ b/src/test/crimson/seastore/onode_tree/test_fltree_onode_manager.cc @@ -89,7 +89,7 @@ struct fltree_onode_manager_test_t [this](auto &t) { return manager->mkfs(*t ).safe_then([this, &t] { - return tm->submit_transaction(std::move(t)); + return tm->submit_transaction(*t); }); }); }).safe_then([this] { @@ -104,7 +104,7 @@ struct fltree_onode_manager_test_t void with_transaction(F&& f) { auto t = tm->create_transaction(); std::invoke(f, *t); - tm->submit_transaction(std::move(t)).unsafe_get0(); + submit_transaction(std::move(t)); segment_cleaner->run_until_halt().get0(); } diff --git a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc index 010c023016304..0792e9575c640 100644 --- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc +++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc @@ -1536,7 +1536,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) { auto t = tm->create_transaction(); tree->bootstrap(*t).unsafe_get(); - tm->submit_transaction(std::move(t)).unsafe_get(); + submit_transaction(std::move(t)); segment_cleaner->run_until_halt().get0(); } @@ -1544,7 +1544,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) { auto t = tm->create_transaction(); tree->insert(*t).unsafe_get(); - tm->submit_transaction(std::move(t)).unsafe_get(); + submit_transaction(std::move(t)); segment_cleaner->run_until_halt().get0(); } { @@ -1567,7 +1567,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) { auto t = tm->create_transaction(); tree->erase(*t, kvs.size() / 4 * 3).unsafe_get(); - tm->submit_transaction(std::move(t)).unsafe_get(); + submit_transaction(std::move(t)); segment_cleaner->run_until_halt().get0(); } { @@ -1589,7 +1589,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) { auto t = tm->create_transaction(); tree->erase(*t, kvs.size()).unsafe_get(); - tm->submit_transaction(std::move(t)).unsafe_get(); + submit_transaction(std::move(t)); segment_cleaner->run_until_halt().get0(); } { @@ -1638,11 +1638,14 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain) ++num_ops; repeat_eagain([this, &tree, &num_ops_eagain] { ++num_ops_eagain; - auto t = tm->create_transaction(); - return tree->bootstrap(*t - ).safe_then([this, t = std::move(t)] () mutable { - return tm->submit_transaction(std::move(t)); - }); + return seastar::do_with( + tm->create_transaction(), + [this, &tree](auto &t) { + return tree->bootstrap(*t + ).safe_then([this, &t] { + return tm->submit_transaction(*t); + }); + }); }).unsafe_get0(); segment_cleaner->run_until_halt().get0(); @@ -1654,12 +1657,15 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain) ++num_ops; repeat_eagain([this, &tree, &num_ops_eagain, &iter] { ++num_ops_eagain; - auto t = tm->create_transaction(); - return tree->insert_one(*t, iter - ).safe_then([this, t = std::move(t)] (auto cursor) mutable { - cursor.invalidate(); - return tm->submit_transaction(std::move(t)); - }); + return seastar::do_with( + tm->create_transaction(), + [this, &tree, &iter](auto &t) { + return tree->insert_one(*t, iter + ).safe_then([this, &t](auto cursor) { + cursor.invalidate(); + return tm->submit_transaction(*t); + }); + }); }).unsafe_get0(); segment_cleaner->run_until_halt().get0(); ++iter; @@ -1698,11 +1704,14 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain) ++num_ops; repeat_eagain([this, &tree, &num_ops_eagain, &iter] { ++num_ops_eagain; - auto t = tm->create_transaction(); - return tree->erase_one(*t, iter - ).safe_then([this, t = std::move(t)] () mutable { - return tm->submit_transaction(std::move(t)); - }); + return seastar::do_with( + tm->create_transaction(), + [this, &tree, &iter](auto &t) { + return tree->erase_one(*t, iter + ).safe_then([this, &t] () mutable { + return tm->submit_transaction(*t); + }); + }); }).unsafe_get0(); segment_cleaner->run_until_halt().get0(); ++iter; diff --git a/src/test/crimson/seastore/test_collection_manager.cc b/src/test/crimson/seastore/test_collection_manager.cc index 511868444e6c6..8fd31880b3724 100644 --- a/src/test/crimson/seastore/test_collection_manager.cc +++ b/src/test/crimson/seastore/test_collection_manager.cc @@ -69,11 +69,6 @@ struct collection_manager_test_t : auto t = tm->create_transaction(); checking_mappings(coll_root, *t); } - - void submit_transaction(TransactionRef &&t) { - tm->submit_transaction(std::move(t)).unsafe_get0(); - segment_cleaner->run_until_halt().get0(); - } }; TEST_F(collection_manager_test_t, basic) diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc index a5777bd4f1c79..c38ed6ccf3c4f 100644 --- a/src/test/crimson/seastore/test_object_data_handler.cc +++ b/src/test/crimson/seastore/test_object_data_handler.cc @@ -44,7 +44,7 @@ struct object_data_handler_test_t: object_data_handler_test_t() {} auto submit_transaction(TransactionRef &&t) { - return tm->submit_transaction(std::move(t) + return tm->submit_transaction(*t ).safe_then([this] { return segment_cleaner->run_until_halt(); }); diff --git a/src/test/crimson/seastore/test_omap_manager.cc b/src/test/crimson/seastore/test_omap_manager.cc index 8b4889c847067..570b8b1b816a3 100644 --- a/src/test/crimson/seastore/test_omap_manager.cc +++ b/src/test/crimson/seastore/test_omap_manager.cc @@ -193,11 +193,6 @@ struct omap_manager_test_t : omap_manager = omap_manager::create_omap_manager(*tm); logger().debug("{}: end", __func__); } - - void submit_transaction(TransactionRef &&t) { - tm->submit_transaction(std::move(t)).unsafe_get0(); - segment_cleaner->run_until_halt().get0(); - } }; TEST_F(omap_manager_test_t, basic) diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 8e95c0e068491..5ee55acd15c8d 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -536,7 +536,7 @@ struct transaction_manager_test_t : bool try_submit_transaction(test_transaction_t t) { using ertr = with_trans_ertr; using ret = ertr::future; - bool success = tm->submit_transaction(std::move(t.t) + bool success = tm->submit_transaction(*t.t ).safe_then([]() -> ret { return ertr::make_ready_future(true); }).handle_error( diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h index 4323124f650b6..f839939ed9c5f 100644 --- a/src/test/crimson/seastore/transaction_manager_test_state.h +++ b/src/test/crimson/seastore/transaction_manager_test_state.h @@ -144,6 +144,11 @@ protected: crimson::ct_error::assert_all{"Error in teardown"} ); } + + void submit_transaction(TransactionRef t) { + tm->submit_transaction(*t).unsafe_get0(); + segment_cleaner->run_until_halt().get0(); + } }; class TestSegmentManagerWrapper final : public SegmentManager { diff --git a/src/tools/crimson/perf_staged_fltree.cc b/src/tools/crimson/perf_staged_fltree.cc index 88a98ef956566..3f5f98fc28aae 100644 --- a/src/tools/crimson/perf_staged_fltree.cc +++ b/src/tools/crimson/perf_staged_fltree.cc @@ -36,15 +36,13 @@ class PerfTree : public TMTestState { { auto t = tm->create_transaction(); tree->bootstrap(*t).unsafe_get(); - tm->submit_transaction(std::move(t)).unsafe_get(); - segment_cleaner->run_until_halt().get0(); + submit_transaction(std::move(t)); } { auto t = tm->create_transaction(); tree->insert(*t).unsafe_get(); auto start_time = mono_clock::now(); - tm->submit_transaction(std::move(t)).unsafe_get(); - segment_cleaner->run_until_halt().get0(); + submit_transaction(std::move(t)); std::chrono::duration duration = mono_clock::now() - start_time; logger().warn("submit_transaction() done! {}s", duration.count()); } @@ -57,8 +55,7 @@ class PerfTree : public TMTestState { { auto t = tm->create_transaction(); tree->erase(*t, kvs.size() * erase_ratio).unsafe_get(); - tm->submit_transaction(std::move(t)).unsafe_get(); - segment_cleaner->run_until_halt().get0(); + submit_transaction(std::move(t)); } { auto t = tm->create_transaction();