From cd78b9770afe4df6370f370e06f6c05080d9e2c6 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Tue, 29 Jun 2021 17:19:23 -0700 Subject: [PATCH] test/crimson/seastore: clean up transaction_manager based tests Permits using either vanilla TransactionManager and InterruptedTransactionManager, updates users to use submit_transaction helpers. Signed-off-by: Samuel Just --- src/crimson/os/seastore/transaction_manager.h | 20 +++++++++++++------ .../onode_tree/test_fltree_onode_manager.cc | 4 ++-- .../seastore/onode_tree/test_staged_fltree.cc | 16 +++++++-------- .../seastore/test_object_data_handler.cc | 17 +++++----------- .../crimson/seastore/test_omap_manager.cc | 4 ++-- .../seastore/test_transaction_manager.cc | 18 ++++++++--------- .../seastore/transaction_manager_test_state.h | 15 ++++++++++++-- 7 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index 38796b720cbd2..b1675ecac79c7 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -573,13 +573,13 @@ using TransactionManagerRef = std::unique_ptr; #define FORWARD(METHOD) \ template \ auto METHOD(Args&&... args) const { \ - return tm.METHOD(std::forward(args)...); \ + return tm->METHOD(std::forward(args)...); \ } #define PARAM_FORWARD(METHOD) \ template \ auto METHOD(Args&&... args) const { \ - return tm.METHOD(std::forward(args)...); \ + return tm->METHOD(std::forward(args)...); \ } #define INT_FORWARD(METHOD) \ @@ -588,7 +588,7 @@ using TransactionManagerRef = std::unique_ptr; return with_trans_intr( \ t, \ [this](auto&&... args) { \ - return tm.METHOD(std::forward(args)...); \ + return tm->METHOD(std::forward(args)...); \ }, \ std::forward(args)...); \ } @@ -599,18 +599,24 @@ using TransactionManagerRef = std::unique_ptr; return with_trans_intr( \ t, \ [this](auto&&... args) { \ - return tm.METHOD(std::forward(args)...); \ + return tm->METHOD(std::forward(args)...); \ }, \ std::forward(args)...); \ } /// Temporary translator to non-interruptible futures class InterruptedTransactionManager { - TransactionManager &tm; + TransactionManager *tm = nullptr; public: + InterruptedTransactionManager() = default; InterruptedTransactionManager(const InterruptedTransactionManager &) = default; InterruptedTransactionManager(InterruptedTransactionManager &&) = default; - InterruptedTransactionManager(TransactionManager &tm) : tm(tm) {} + InterruptedTransactionManager(TransactionManager &tm) : tm(&tm) {} + + InterruptedTransactionManager &operator=( + const InterruptedTransactionManager &) = default; + InterruptedTransactionManager &operator=( + InterruptedTransactionManager &&) = default; FORWARD(mkfs) FORWARD(mount) @@ -642,6 +648,8 @@ public: FORWARD(get_segment_cleaner) FORWARD(get_lba_manager) + + void reset() { tm = nullptr; } }; class InterruptedTMRef { 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 362edac7e17d9..bdbdd3d184500 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 @@ -71,7 +71,7 @@ struct fltree_onode_manager_test_t virtual void _init() final { TMTestState::_init(); - manager.reset(new FLTreeOnodeManager(*tm)); + manager.reset(new FLTreeOnodeManager(itm)); } virtual void _destroy() final { @@ -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(*t); + return submit_transaction_fut(*t); }); }); }).safe_then([this] { 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 0792e9575c640..79ce42aed7687 100644 --- a/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc +++ b/src/test/crimson/seastore/onode_tree/test_staged_fltree.cc @@ -1528,7 +1528,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) {8, 11, 64, 256, 301, 320}, {8, 16, 128, 512, 576, 640}, {0, 16}, {0, 10}, {0, 4}); - auto moved_nm = (TEST_SEASTORE ? NodeExtentManager::create_seastore(*tm) + auto moved_nm = (TEST_SEASTORE ? NodeExtentManager::create_seastore(itm) : NodeExtentManager::create_dummy(IS_DUMMY_SYNC)); auto p_nm = moved_nm.get(); auto tree = std::make_unique>( @@ -1554,7 +1554,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) if constexpr (TEST_SEASTORE) { logger().info("seastore replay insert begin"); restart(); - tree->reload(NodeExtentManager::create_seastore(*tm)); + tree->reload(NodeExtentManager::create_seastore(itm)); logger().info("seastore replay insert end"); } { @@ -1577,7 +1577,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) if constexpr (TEST_SEASTORE) { logger().info("seastore replay erase-1 begin"); restart(); - tree->reload(NodeExtentManager::create_seastore(*tm)); + tree->reload(NodeExtentManager::create_seastore(itm)); logger().info("seastore replay erase-1 end"); } { @@ -1599,7 +1599,7 @@ TEST_F(d_seastore_tm_test_t, 6_random_tree_insert_erase) if constexpr (TEST_SEASTORE) { logger().info("seastore replay erase-2 begin"); restart(); - tree->reload(NodeExtentManager::create_seastore(*tm)); + tree->reload(NodeExtentManager::create_seastore(itm)); logger().info("seastore replay erase-2 end"); } { @@ -1627,7 +1627,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain) {8, 16, 128, 576, 992, 1200}, {0, 8}, {0, 10}, {0, 4}); auto moved_nm = NodeExtentManager::create_seastore( - *tm, L_ADDR_MIN, EAGAIN_PROBABILITY); + itm, L_ADDR_MIN, EAGAIN_PROBABILITY); auto p_nm = static_cast*>(moved_nm.get()); auto tree = std::make_unique>( kvs, std::move(moved_nm)); @@ -1643,7 +1643,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain) [this, &tree](auto &t) { return tree->bootstrap(*t ).safe_then([this, &t] { - return tm->submit_transaction(*t); + return submit_transaction_fut(*t); }); }); }).unsafe_get0(); @@ -1663,7 +1663,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain) return tree->insert_one(*t, iter ).safe_then([this, &t](auto cursor) { cursor.invalidate(); - return tm->submit_transaction(*t); + return submit_transaction_fut(*t); }); }); }).unsafe_get0(); @@ -1709,7 +1709,7 @@ TEST_F(d_seastore_tm_test_t, 7_tree_insert_erase_eagain) [this, &tree, &iter](auto &t) { return tree->erase_one(*t, iter ).safe_then([this, &t] () mutable { - return tm->submit_transaction(*t); + return submit_transaction_fut(*t); }); }); }).unsafe_get0(); diff --git a/src/test/crimson/seastore/test_object_data_handler.cc b/src/test/crimson/seastore/test_object_data_handler.cc index c38ed6ccf3c4f..95770a3c5c2cf 100644 --- a/src/test/crimson/seastore/test_object_data_handler.cc +++ b/src/test/crimson/seastore/test_object_data_handler.cc @@ -43,13 +43,6 @@ struct object_data_handler_test_t: object_data_handler_test_t() {} - auto submit_transaction(TransactionRef &&t) { - return tm->submit_transaction(*t - ).safe_then([this] { - return segment_cleaner->run_until_halt(); - }); - } - void write(Transaction &t, objaddr_t offset, extent_len_t len, char fill) { ceph_assert(offset + len <= known_contents.length()); size = std::max(size, offset + len); @@ -65,7 +58,7 @@ struct object_data_handler_test_t: len)); return ObjectDataHandler().write( ObjectDataHandler::context_t{ - *tm, + itm, t, *onode, }, @@ -75,7 +68,7 @@ struct object_data_handler_test_t: void write(objaddr_t offset, extent_len_t len, char fill) { auto t = tm->create_transaction(); write(*t, offset, len, fill); - return submit_transaction(std::move(t)).unsafe_get0(); + return submit_transaction(std::move(t)); } void truncate(Transaction &t, objaddr_t offset) { @@ -86,7 +79,7 @@ struct object_data_handler_test_t: size - offset); ObjectDataHandler().truncate( ObjectDataHandler::context_t{ - *tm, + itm, t, *onode }, @@ -97,13 +90,13 @@ struct object_data_handler_test_t: void truncate(objaddr_t offset) { auto t = tm->create_transaction(); truncate(*t, offset); - return submit_transaction(std::move(t)).unsafe_get0(); + return submit_transaction(std::move(t)); } void read(Transaction &t, objaddr_t offset, extent_len_t len) { bufferlist bl = ObjectDataHandler().read( ObjectDataHandler::context_t{ - *tm, + itm, t, *onode }, diff --git a/src/test/crimson/seastore/test_omap_manager.cc b/src/test/crimson/seastore/test_omap_manager.cc index 570b8b1b816a3..0363890b237dc 100644 --- a/src/test/crimson/seastore/test_omap_manager.cc +++ b/src/test/crimson/seastore/test_omap_manager.cc @@ -55,7 +55,7 @@ struct omap_manager_test_t : seastar::future<> set_up_fut() final { return tm_setup().then([this] { - omap_manager = omap_manager::create_omap_manager(*tm); + omap_manager = omap_manager::create_omap_manager(itm); return seastar::now(); }); } @@ -190,7 +190,7 @@ struct omap_manager_test_t : void replay() { logger().debug("{}: begin", __func__); restart(); - omap_manager = omap_manager::create_omap_manager(*tm); + omap_manager = omap_manager::create_omap_manager(itm); logger().debug("{}: end", __func__); } }; diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 5ee55acd15c8d..0325dfc32d737 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -351,11 +351,11 @@ struct transaction_manager_test_t : }; test_transaction_t create_transaction() { - return { tm->create_transaction(), {} }; + return { itm.create_transaction(), {} }; } test_transaction_t create_weak_transaction() { - return { tm->create_weak_transaction(), {} }; + return { itm.create_weak_transaction(), {} }; } TestBlockRef alloc_extent( @@ -363,7 +363,7 @@ struct transaction_manager_test_t : laddr_t hint, extent_len_t len, char contents) { - auto extent = tm->alloc_extent( + auto extent = itm.alloc_extent( *(t.t), hint, len).unsafe_get0(); @@ -427,7 +427,7 @@ struct transaction_manager_test_t : ceph_assert(test_mappings.contains(addr, t.mapping_delta)); ceph_assert(test_mappings.get(addr, t.mapping_delta).desc.len == len); - auto ext = tm->read_extent( + auto ext = itm.read_extent( *t.t, addr, len ).unsafe_get0(); EXPECT_EQ(addr, ext->get_laddr()); @@ -443,7 +443,7 @@ struct transaction_manager_test_t : using ertr = with_trans_ertr; using ret = ertr::future; - auto ext = tm->read_extent( + auto ext = itm.read_extent( *t.t, addr, len ).safe_then([](auto ext) -> ret { return ertr::make_ready_future(ext); @@ -470,7 +470,7 @@ struct transaction_manager_test_t : test_mappings.get(ref->get_laddr(), t.mapping_delta).desc.len == ref->get_length()); - auto ext = tm->get_mutable_extent(*t.t, ref)->cast(); + auto ext = itm.get_mutable_extent(*t.t, ref)->cast(); EXPECT_EQ(ext->get_laddr(), ref->get_laddr()); EXPECT_EQ(ext->get_desc(), ref->get_desc()); mutator.mutate(*ext, gen); @@ -492,7 +492,7 @@ struct transaction_manager_test_t : ceph_assert(test_mappings.contains(offset, t.mapping_delta)); ceph_assert(test_mappings.get(offset, t.mapping_delta).refcount > 0); - auto refcnt = tm->inc_ref(*t.t, offset).unsafe_get0(); + auto refcnt = itm.inc_ref(*t.t, offset).unsafe_get0(); auto check_refcnt = test_mappings.inc_ref(offset, t.mapping_delta); EXPECT_EQ(refcnt, check_refcnt); } @@ -501,7 +501,7 @@ struct transaction_manager_test_t : ceph_assert(test_mappings.contains(offset, t.mapping_delta)); ceph_assert(test_mappings.get(offset, t.mapping_delta).refcount > 0); - auto refcnt = tm->dec_ref(*t.t, offset).unsafe_get0(); + auto refcnt = itm.dec_ref(*t.t, offset).unsafe_get0(); auto check_refcnt = test_mappings.dec_ref(offset, t.mapping_delta); EXPECT_EQ(refcnt, check_refcnt); if (refcnt == 0) @@ -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(*t.t + bool success = itm.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 f839939ed9c5f..22c9fa738e9a3 100644 --- a/src/test/crimson/seastore/transaction_manager_test_state.h +++ b/src/test/crimson/seastore/transaction_manager_test_state.h @@ -100,7 +100,8 @@ auto get_seastore(SegmentManagerRef sm) { class TMTestState : public EphemeralTestState { protected: - InterruptedTMRef tm; + TransactionManagerRef tm; + InterruptedTransactionManager itm; LBAManager *lba_manager; SegmentCleaner *segment_cleaner; @@ -108,6 +109,7 @@ protected: virtual void _init() { tm = get_transaction_manager(*segment_manager); + itm = InterruptedTransactionManager(*tm); segment_cleaner = tm->get_segment_cleaner(); lba_manager = tm->get_lba_manager(); } @@ -115,6 +117,7 @@ protected: virtual void _destroy() { segment_cleaner = nullptr; lba_manager = nullptr; + itm.reset(); tm.reset(); } @@ -145,8 +148,16 @@ protected: ); } + auto submit_transaction_fut(Transaction &t) { + return with_trans_intr( + t, + [this](auto &t) { + return tm->submit_transaction(t); + }); + } + void submit_transaction(TransactionRef t) { - tm->submit_transaction(*t).unsafe_get0(); + submit_transaction_fut(*t).unsafe_get0(); segment_cleaner->run_until_halt().get0(); } }; -- 2.39.5