From b6d4631e03804dc0ed81e796e1c6150a538d9dc2 Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Fri, 6 Aug 2021 14:14:23 +0800 Subject: [PATCH] crimson/os/seatore: drop the temporary InterruptedTransactionManager Signed-off-by: Yingxin Cheng --- src/crimson/os/seastore/transaction_manager.h | 129 ------------------ .../seastore/test_transaction_manager.cc | 31 +++-- .../seastore/transaction_manager_test_state.h | 3 - 3 files changed, 17 insertions(+), 146 deletions(-) diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index df21268819e..0a8036967dd 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -548,133 +548,4 @@ public: }; using TransactionManagerRef = std::unique_ptr; -#define FORWARD(METHOD) \ - template \ - auto METHOD(Args&&... args) const { \ - return tm->METHOD(std::forward(args)...); \ - } - -#define PARAM_FORWARD(METHOD) \ - template \ - auto METHOD(Args&&... args) const { \ - return tm->METHOD(std::forward(args)...); \ - } - -#define INT_FORWARD(METHOD) \ - template \ - auto METHOD(Transaction &t, Args&&... args) const { \ - return with_trans_intr( \ - t, \ - [this](auto&&... args) { \ - return tm->METHOD(std::forward(args)...); \ - }, \ - std::forward(args)...); \ - } - -#define PARAM_INT_FORWARD(METHOD) \ - template \ - auto METHOD(Transaction &t, Args&&... args) const { \ - return with_trans_intr( \ - t, \ - [this](auto&&... args) { \ - return tm->METHOD(std::forward(args)...); \ - }, \ - std::forward(args)...); \ - } - -/// Temporary translator to non-interruptible futures -class InterruptedTransactionManager { - TransactionManager *tm = nullptr; -public: - InterruptedTransactionManager() = default; - InterruptedTransactionManager(const InterruptedTransactionManager &) = default; - InterruptedTransactionManager(InterruptedTransactionManager &&) = default; - InterruptedTransactionManager(TransactionManager &tm) : tm(&tm) {} - - InterruptedTransactionManager &operator=( - const InterruptedTransactionManager &) = default; - InterruptedTransactionManager &operator=( - InterruptedTransactionManager &&) = default; - - TransactionManager &get_tm() const { return *tm; } - - FORWARD(mkfs) - FORWARD(mount) - FORWARD(close) - FORWARD(create_transaction) - FORWARD(create_weak_transaction) - FORWARD(reset_transaction_preserve_handle) - INT_FORWARD(get_pin) - INT_FORWARD(get_pins) - PARAM_INT_FORWARD(pin_to_extent) - PARAM_INT_FORWARD(read_extent) - FORWARD(get_mutable_extent) - INT_FORWARD(inc_ref) - INT_FORWARD(dec_ref) - PARAM_INT_FORWARD(alloc_extent) - INT_FORWARD(reserve_region) - INT_FORWARD(find_hole) - PARAM_INT_FORWARD(alloc_extents) - INT_FORWARD(submit_transaction) - - INT_FORWARD(read_root_meta) - INT_FORWARD(update_root_meta) - INT_FORWARD(read_onode_root) - FORWARD(write_onode_root) - INT_FORWARD(read_collection_root) - FORWARD(write_collection_root) - FORWARD(get_block_size) - FORWARD(store_stat) - - FORWARD(get_segment_cleaner) - FORWARD(get_lba_manager) - - void reset() { tm = nullptr; } -}; - -class InterruptedTMRef { - std::unique_ptr ref; - std::optional itm; -public: - InterruptedTMRef() {} - - template - InterruptedTMRef(T&&... args) - : ref(std::make_unique(std::forward(args)...)), - itm(*ref) {} - - InterruptedTMRef(std::unique_ptr tm) - : ref(std::move(tm)), itm(*ref) {} - - InterruptedTMRef(InterruptedTMRef &&itmr) - : ref(std::move(itmr.ref)), itm(*ref) {} - - InterruptedTMRef &operator=(std::unique_ptr tm) { - this->~InterruptedTMRef(); - new (this) InterruptedTMRef(std::move(tm)); - return *this; - } - - InterruptedTMRef &operator=(InterruptedTMRef &&rhs) { - this->~InterruptedTMRef(); - new (this) InterruptedTMRef(std::move(rhs)); - return *this; - } - - void reset() { - itm = std::nullopt; - ref.reset(); - } - - auto &operator*() const { - return *itm; - } - - auto operator->() const { - return &*itm; - } -}; - - - } diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 523934ed2ed..c3e59117b1c 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -361,10 +361,9 @@ struct transaction_manager_test_t : laddr_t hint, extent_len_t len, char contents) { - auto extent = itm.alloc_extent( - *(t.t), - hint, - len).unsafe_get0(); + auto extent = with_trans_intr(*(t.t), [&](auto& trans) { + return tm->alloc_extent(trans, hint, len); + }).unsafe_get0(); extent->set_contents(contents); EXPECT_FALSE(test_mappings.contains(extent->get_laddr(), t.mapping_delta)); EXPECT_EQ(len, extent->get_length()); @@ -425,9 +424,9 @@ 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 = itm.read_extent( - *t.t, addr, len - ).unsafe_get0(); + auto ext = with_trans_intr(*(t.t), [&](auto& trans) { + return tm->read_extent(trans, addr, len); + }).unsafe_get0(); EXPECT_EQ(addr, ext->get_laddr()); return ext; } @@ -441,9 +440,9 @@ struct transaction_manager_test_t : using ertr = with_trans_ertr; using ret = ertr::future; - auto ext = itm.read_extent( - *t.t, addr, len - ).safe_then([](auto ext) -> ret { + auto ext = with_trans_intr(*(t.t), [&](auto& trans) { + return tm->read_extent(trans, addr, len); + }).safe_then([](auto ext) -> ret { return ertr::make_ready_future(ext); }).handle_error( [](const crimson::ct_error::eagain &e) { @@ -468,7 +467,7 @@ struct transaction_manager_test_t : test_mappings.get(ref->get_laddr(), t.mapping_delta).desc.len == ref->get_length()); - auto ext = itm.get_mutable_extent(*t.t, ref)->cast(); + auto ext = tm->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); @@ -490,7 +489,9 @@ 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 = itm.inc_ref(*t.t, offset).unsafe_get0(); + auto refcnt = with_trans_intr(*(t.t), [&](auto& trans) { + return tm->inc_ref(trans, offset); + }).unsafe_get0(); auto check_refcnt = test_mappings.inc_ref(offset, t.mapping_delta); EXPECT_EQ(refcnt, check_refcnt); } @@ -499,7 +500,9 @@ 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 = itm.dec_ref(*t.t, offset).unsafe_get0(); + auto refcnt = with_trans_intr(*(t.t), [&](auto& trans) { + return tm->dec_ref(trans, offset); + }).unsafe_get0(); auto check_refcnt = test_mappings.dec_ref(offset, t.mapping_delta); EXPECT_EQ(refcnt, check_refcnt); if (refcnt == 0) @@ -534,7 +537,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 = itm.submit_transaction(*t.t + bool success = submit_transaction_fut(*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 ef343f5f464..d95fa45e164 100644 --- a/src/test/crimson/seastore/transaction_manager_test_state.h +++ b/src/test/crimson/seastore/transaction_manager_test_state.h @@ -101,7 +101,6 @@ auto get_seastore(SegmentManagerRef sm) { class TMTestState : public EphemeralTestState { protected: TransactionManagerRef tm; - InterruptedTransactionManager itm; LBAManager *lba_manager; SegmentCleaner *segment_cleaner; @@ -109,7 +108,6 @@ 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(); } @@ -117,7 +115,6 @@ protected: virtual void _destroy() { segment_cleaner = nullptr; lba_manager = nullptr; - itm.reset(); tm.reset(); } -- 2.39.5