From 43eb9902e234af37d18a85990ec87dda961ec9f8 Mon Sep 17 00:00:00 2001 From: Samuel Just Date: Tue, 29 Jun 2021 17:20:13 -0700 Subject: [PATCH] crimson/os/seastore/collection_manager: convert to use interruptible_future Signed-off-by: Samuel Just --- src/crimson/os/seastore/collection_manager.cc | 2 +- src/crimson/os/seastore/collection_manager.h | 26 ++++---- .../collection_flat_node.cc | 6 +- .../collection_manager/collection_flat_node.h | 12 ++-- .../flat_collection_manager.cc | 30 ++++----- .../flat_collection_manager.h | 8 +-- src/crimson/os/seastore/seastore.cc | 66 +++++++++++-------- .../seastore/test_collection_manager.cc | 66 ++++++++++++------- 8 files changed, 122 insertions(+), 94 deletions(-) diff --git a/src/crimson/os/seastore/collection_manager.cc b/src/crimson/os/seastore/collection_manager.cc index 0602f964e1e..4f5b58d01a2 100644 --- a/src/crimson/os/seastore/collection_manager.cc +++ b/src/crimson/os/seastore/collection_manager.cc @@ -7,7 +7,7 @@ namespace crimson::os::seastore::collection_manager { -CollectionManagerRef create_coll_manager(InterruptedTransactionManager trans_manager) { +CollectionManagerRef create_coll_manager(TransactionManager &trans_manager) { return CollectionManagerRef(new FlatCollectionManager(trans_manager)); } diff --git a/src/crimson/os/seastore/collection_manager.h b/src/crimson/os/seastore/collection_manager.h index c5f708dd67f..37913abb403 100644 --- a/src/crimson/os/seastore/collection_manager.h +++ b/src/crimson/os/seastore/collection_manager.h @@ -26,19 +26,17 @@ struct coll_info_t { /// Interface for maintaining set of collections class CollectionManager { public: - using base_ertr = with_trans_ertr< - TransactionManager::read_extent_iertr>; + using base_iertr = TransactionManager::read_extent_iertr; /// Initialize collection manager instance for an empty store - using mkfs_ertr = with_trans_ertr< - TransactionManager::alloc_extent_iertr>; - using mkfs_ret = mkfs_ertr::future; + using mkfs_iertr = TransactionManager::alloc_extent_iertr; + using mkfs_ret = mkfs_iertr::future; virtual mkfs_ret mkfs( Transaction &t) = 0; /// Create collection - using create_ertr = base_ertr; - using create_ret = create_ertr::future<>; + using create_iertr = base_iertr; + using create_ret = create_iertr::future<>; virtual create_ret create( coll_root_t &root, Transaction &t, @@ -47,24 +45,24 @@ public: ) = 0; /// List collections with info - using list_ertr = base_ertr; + using list_iertr = base_iertr; using list_ret_bare = std::vector>; - using list_ret = list_ertr::future; + using list_ret = list_iertr::future; virtual list_ret list( const coll_root_t &root, Transaction &t) = 0; /// Remove cid - using remove_ertr = base_ertr; - using remove_ret = remove_ertr::future<>; + using remove_iertr = base_iertr; + using remove_ret = remove_iertr::future<>; virtual remove_ret remove( const coll_root_t &coll_root, Transaction &t, coll_t cid) = 0; /// Update info for cid - using update_ertr = base_ertr; - using update_ret = base_ertr::future<>; + using update_iertr = base_iertr; + using update_ret = base_iertr::future<>; virtual update_ret update( const coll_root_t &coll_root, Transaction &t, @@ -79,7 +77,7 @@ using CollectionManagerRef = std::unique_ptr; namespace collection_manager { /* creat CollectionMapManager for Collection */ CollectionManagerRef create_coll_manager( - InterruptedTransactionManager trans_manager); + TransactionManager &trans_manager); } diff --git a/src/crimson/os/seastore/collection_manager/collection_flat_node.cc b/src/crimson/os/seastore/collection_manager/collection_flat_node.cc index bb8754e58eb..6181582702c 100644 --- a/src/crimson/os/seastore/collection_manager/collection_flat_node.cc +++ b/src/crimson/os/seastore/collection_manager/collection_flat_node.cc @@ -53,7 +53,7 @@ CollectionNode::list() list_result.emplace_back(coll, bits); } return list_ret( - list_ertr::ready_future_marker{}, + interruptible::ready_future_marker{}, std::move(list_result)); } @@ -72,7 +72,7 @@ CollectionNode::create(coll_context_t cc, coll_t coll, unsigned bits) if (encoded_sizeof((base_coll_map_t&)decoded) > get_bptr().length()) { decoded.erase(iter); return create_ret( - create_ertr::ready_future_marker{}, + interruptible::ready_future_marker{}, create_result_t::OVERFLOW); } else { if (auto buffer = maybe_get_delta_buffer(); buffer) { @@ -80,7 +80,7 @@ CollectionNode::create(coll_context_t cc, coll_t coll, unsigned bits) } copy_to_node(); return create_ret( - create_ertr::ready_future_marker{}, + interruptible::ready_future_marker{}, create_result_t::SUCCESS); } } diff --git a/src/crimson/os/seastore/collection_manager/collection_flat_node.h b/src/crimson/os/seastore/collection_manager/collection_flat_node.h index fcd9f847b68..05f926c909d 100644 --- a/src/crimson/os/seastore/collection_manager/collection_flat_node.h +++ b/src/crimson/os/seastore/collection_manager/collection_flat_node.h @@ -9,7 +9,7 @@ namespace crimson::os::seastore::collection_manager { struct coll_context_t { - InterruptedTransactionManager tm; + TransactionManager &tm; Transaction &t; }; @@ -115,7 +115,7 @@ struct CollectionNode return is_mutation_pending() ? &delta_buffer : nullptr; } - using list_ertr = CollectionManager::list_ertr; + using list_iertr = CollectionManager::list_iertr; using list_ret = CollectionManager::list_ret; list_ret list(); @@ -124,15 +124,15 @@ struct CollectionNode SUCCESS, OVERFLOW }; - using create_ertr = CollectionManager::create_ertr; - using create_ret = create_ertr::future; + using create_iertr = CollectionManager::create_iertr; + using create_ret = create_iertr::future; create_ret create(coll_context_t cc, coll_t coll, unsigned bits); - using remove_ertr = CollectionManager::remove_ertr; + using remove_iertr = CollectionManager::remove_iertr; using remove_ret = CollectionManager::remove_ret; remove_ret remove(coll_context_t cc, coll_t coll); - using update_ertr = CollectionManager::update_ertr; + using update_iertr = CollectionManager::update_iertr; using update_ret = CollectionManager::update_ret; update_ret update(coll_context_t cc, coll_t coll, unsigned bits); diff --git a/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc b/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc index c7a58e83c12..6d762844eb9 100644 --- a/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc +++ b/src/crimson/os/seastore/collection_manager/flat_collection_manager.cc @@ -22,7 +22,7 @@ constexpr static segment_off_t MIN_FLAT_BLOCK_SIZE = 4<<10; [[maybe_unused]] constexpr static segment_off_t MAX_FLAT_BLOCK_SIZE = 4<<20; FlatCollectionManager::FlatCollectionManager( - InterruptedTransactionManager tm) + TransactionManager &tm) : tm(tm) {} FlatCollectionManager::mkfs_ret @@ -32,12 +32,12 @@ FlatCollectionManager::mkfs(Transaction &t) logger().debug("FlatCollectionManager: {}", __func__); return tm.alloc_extent( t, L_ADDR_MIN, MIN_FLAT_BLOCK_SIZE - ).safe_then([](auto&& root_extent) { + ).si_then([](auto&& root_extent) { coll_root_t coll_root = coll_root_t( root_extent->get_laddr(), MIN_FLAT_BLOCK_SIZE ); - return mkfs_ertr::make_ready_future(coll_root); + return mkfs_iertr::make_ready_future(coll_root); }); } @@ -51,8 +51,8 @@ FlatCollectionManager::get_coll_root(const coll_root_t &coll_root, Transaction & cc.t, coll_root.get_location(), coll_root.get_size() - ).safe_then([](auto&& e) { - return get_root_ertr::make_ready_future(std::move(e)); + ).si_then([](auto&& e) { + return get_root_iertr::make_ready_future(std::move(e)); }); } @@ -62,10 +62,10 @@ FlatCollectionManager::create(coll_root_t &coll_root, Transaction &t, { logger().debug("FlatCollectionManager: {}", __func__); return get_coll_root(coll_root, t - ).safe_then([=, &coll_root, &t] (auto &&extent) { + ).si_then([=, &coll_root, &t] (auto &&extent) { return extent->create( get_coll_context(t), cid, info.split_bits - ).safe_then([=, &coll_root, &t] (auto ret) { + ).si_then([=, &coll_root, &t] (auto ret) { switch (ret) { case CollectionNode::create_result_t::OVERFLOW: { logger().debug("FlatCollectionManager: {} overflow!", __func__); @@ -76,23 +76,23 @@ FlatCollectionManager::create(coll_root_t &coll_root, Transaction &t, assert(new_size < MAX_FLAT_BLOCK_SIZE); return tm.alloc_extent( t, L_ADDR_MIN, new_size - ).safe_then([=, &coll_root, &t] (auto &&root_extent) { + ).si_then([=, &coll_root, &t] (auto &&root_extent) { coll_root.update(root_extent->get_laddr(), root_extent->get_length()); root_extent->decoded = extent->decoded; root_extent->loaded = true; return root_extent->create( get_coll_context(t), cid, info.split_bits - ).safe_then([=, &t](auto result) { + ).si_then([=, &t](auto result) { assert(result == CollectionNode::create_result_t::SUCCESS); return tm.dec_ref(t, extent->get_laddr()); - }).safe_then([] (auto) { - return create_ertr::make_ready_future<>(); + }).si_then([] (auto) { + return create_iertr::make_ready_future<>(); }); }); } case CollectionNode::create_result_t::SUCCESS: { - return create_ertr::make_ready_future<>(); + return create_iertr::make_ready_future<>(); } } __builtin_unreachable(); @@ -105,7 +105,7 @@ FlatCollectionManager::list(const coll_root_t &coll_root, Transaction &t) { logger().debug("FlatCollectionManager: {}", __func__); return get_coll_root(coll_root, t) - .safe_then([] (auto extent) { + .si_then([] (auto extent) { return extent->list(); }); } @@ -116,7 +116,7 @@ FlatCollectionManager::update(const coll_root_t &coll_root, Transaction &t, { logger().debug("FlatCollectionManager: {}", __func__); return get_coll_root(coll_root, t) - .safe_then([this, &t, cid, info] (auto extent) { + .si_then([this, &t, cid, info] (auto extent) { return extent->update(get_coll_context(t), cid, info.split_bits); }); } @@ -126,7 +126,7 @@ FlatCollectionManager::remove(const coll_root_t &coll_root, Transaction &t, coll_t cid ) { logger().debug("FlatCollectionManager: {}", __func__); - return get_coll_root(coll_root, t).safe_then([this, &t, cid] (auto extent) { + return get_coll_root(coll_root, t).si_then([this, &t, cid] (auto extent) { return extent->remove(get_coll_context(t), cid); }); } diff --git a/src/crimson/os/seastore/collection_manager/flat_collection_manager.h b/src/crimson/os/seastore/collection_manager/flat_collection_manager.h index 63662e813a9..1321ec1d8ab 100644 --- a/src/crimson/os/seastore/collection_manager/flat_collection_manager.h +++ b/src/crimson/os/seastore/collection_manager/flat_collection_manager.h @@ -13,18 +13,18 @@ namespace crimson::os::seastore::collection_manager { class FlatCollectionManager : public CollectionManager { - InterruptedTransactionManager tm; + TransactionManager &tm; coll_context_t get_coll_context(Transaction &t) { return coll_context_t{tm, t}; } - using get_root_ertr = base_ertr; - using get_root_ret = get_root_ertr::future; + using get_root_iertr = base_iertr; + using get_root_ret = get_root_iertr::future; get_root_ret get_coll_root(const coll_root_t &coll_root, Transaction &t); public: - explicit FlatCollectionManager(InterruptedTransactionManager tm); + explicit FlatCollectionManager(TransactionManager &tm); mkfs_ret mkfs(Transaction &t) final; diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index 8fad148b59a..55936d13962 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -87,7 +87,11 @@ seastar::future<> SeaStore::mkfs(uuid_d new_osd_fsid) [this](auto &t) { return onode_manager->mkfs(*t ).safe_then([this, &t] { - return collection_manager->mkfs(*t); + return with_trans_intr( + *t, + [this](auto &t) { + return collection_manager->mkfs(t); + }); }).safe_then([this, &t](auto coll_root) { transaction_manager->write_collection_root( *t, @@ -172,9 +176,13 @@ seastar::future> SeaStore::list_collections() [this, &ret](auto &t) { return transaction_manager->read_collection_root(*t ).safe_then([this, &t](auto coll_root) { - return collection_manager->list( - coll_root, - *t); + return with_trans_intr( + *t, + [this, &coll_root](auto &t) { + return collection_manager->list( + coll_root, + t); + }); }).safe_then([&ret](auto colls) { ret.resize(colls.size()); std::transform( @@ -981,18 +989,21 @@ SeaStore::tm_ret SeaStore::_create_collection( return seastar::do_with( _cmroot, [=, &ctx](auto &cmroot) { - return collection_manager->create( - cmroot, + return with_trans_intr( *ctx.transaction, - cid, - bits - ).safe_then([=, &ctx, &cmroot] { - if (cmroot.must_update()) { - transaction_manager->write_collection_root( - *ctx.transaction, - cmroot); - } - }); + [=, &cmroot](auto &t) { + return collection_manager->create( + cmroot, + t, + cid, + bits); + }).safe_then([=, &ctx, &cmroot] { + if (cmroot.must_update()) { + transaction_manager->write_collection_root( + *ctx.transaction, + cmroot); + } + }); }); }).handle_error( tm_ertr::pass_further{}, @@ -1012,18 +1023,21 @@ SeaStore::tm_ret SeaStore::_remove_collection( return seastar::do_with( _cmroot, [=, &ctx](auto &cmroot) { - return collection_manager->remove( - cmroot, + return with_trans_intr( *ctx.transaction, - cid - ).safe_then([=, &ctx, &cmroot] { - // param here denotes whether it already existed, probably error - if (cmroot.must_update()) { - transaction_manager->write_collection_root( - *ctx.transaction, - cmroot); - } - }); + [=, &cmroot](auto &t) { + return collection_manager->remove( + cmroot, + t, + cid); + }).safe_then([=, &ctx, &cmroot] { + // param here denotes whether it already existed, probably error + if (cmroot.must_update()) { + transaction_manager->write_collection_root( + *ctx.transaction, + cmroot); + } + }); }); }).handle_error( tm_ertr::pass_further{}, diff --git a/src/test/crimson/seastore/test_collection_manager.cc b/src/test/crimson/seastore/test_collection_manager.cc index 8fd31880b37..3f8b520f5da 100644 --- a/src/test/crimson/seastore/test_collection_manager.cc +++ b/src/test/crimson/seastore/test_collection_manager.cc @@ -23,6 +23,21 @@ namespace { } +#define TEST_COLL_FORWARD(METHOD) \ + template \ + auto METHOD(coll_root_t &root, Transaction &t, Args&&... args) const { \ + return with_trans_intr( \ + t, \ + [this](auto &t, auto &root, auto&&... args) { \ + return collection_manager->METHOD( \ + root, \ + t, \ + std::forward(args)...); \ + }, \ + root, \ + std::forward(args)...).unsafe_get0(); \ + } + struct collection_manager_test_t : public seastar_test_suite_t, TMTestState { @@ -55,8 +70,24 @@ struct collection_manager_test_t : logger().debug("{}: end", __func__); } + auto get_root() { + auto tref = tm->create_transaction(); + auto coll_root = with_trans_intr( + *tref, + [this](auto &t) { + return collection_manager->mkfs(t); + }).unsafe_get0(); + submit_transaction(std::move(tref)); + return coll_root; + } + + TEST_COLL_FORWARD(remove) + TEST_COLL_FORWARD(list) + TEST_COLL_FORWARD(create) + TEST_COLL_FORWARD(update) + void checking_mappings(coll_root_t &coll_root, Transaction &t) { - auto coll_list = collection_manager->list(coll_root, t).unsafe_get0(); + auto coll_list = list(coll_root, t); EXPECT_EQ(test_coll_mappings.size(), coll_list.size()); for (std::pair p : test_coll_mappings) { EXPECT_NE( @@ -74,17 +105,12 @@ struct collection_manager_test_t : TEST_F(collection_manager_test_t, basic) { run_async([this] { - coll_root_t coll_root; - { - auto t = tm->create_transaction(); - coll_root = collection_manager->mkfs(*t).unsafe_get0(); - submit_transaction(std::move(t)); - } + coll_root_t coll_root = get_root(); { auto t = tm->create_transaction(); for (int i = 0; i < 20; i++) { coll_t cid(spg_t(pg_t(i+1,i+2), shard_id_t::NO_SHARD)); - collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0(); + create(coll_root, *t, cid, coll_info_t(i)); test_coll_mappings.emplace(cid, coll_info_t(i)); } checking_mappings(coll_root, *t); @@ -97,7 +123,7 @@ TEST_F(collection_manager_test_t, basic) { auto t = tm->create_transaction(); for (auto& ite : test_coll_mappings) { - collection_manager->remove(coll_root, *t, ite.first).unsafe_get0(); + remove(coll_root, *t, ite.first); test_coll_mappings.erase(ite.first); } submit_transaction(std::move(t)); @@ -105,7 +131,7 @@ TEST_F(collection_manager_test_t, basic) replay(); { auto t = tm->create_transaction(); - auto list_ret = collection_manager->list(coll_root, *t).unsafe_get0(); + auto list_ret = list(coll_root, *t); submit_transaction(std::move(t)); EXPECT_EQ(list_ret.size(), test_coll_mappings.size()); } @@ -115,18 +141,13 @@ TEST_F(collection_manager_test_t, basic) TEST_F(collection_manager_test_t, overflow) { run_async([this] { - coll_root_t coll_root; - { - auto t = tm->create_transaction(); - coll_root = collection_manager->mkfs(*t).unsafe_get0(); - submit_transaction(std::move(t)); - } + coll_root_t coll_root = get_root(); auto old_location = coll_root.get_location(); auto t = tm->create_transaction(); for (int i = 0; i < 412; i++) { coll_t cid(spg_t(pg_t(i+1,i+2), shard_id_t::NO_SHARD)); - collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0(); + create(coll_root, *t, cid, coll_info_t(i)); test_coll_mappings.emplace(cid, coll_info_t(i)); } submit_transaction(std::move(t)); @@ -141,17 +162,12 @@ TEST_F(collection_manager_test_t, overflow) TEST_F(collection_manager_test_t, update) { run_async([this] { - coll_root_t coll_root; - { - auto t = tm->create_transaction(); - coll_root = collection_manager->mkfs(*t).unsafe_get0(); - submit_transaction(std::move(t)); - } + coll_root_t coll_root = get_root(); { auto t = tm->create_transaction(); for (int i = 0; i < 2; i++) { coll_t cid(spg_t(pg_t(1,i+1), shard_id_t::NO_SHARD)); - collection_manager->create(coll_root, *t, cid, coll_info_t(i)).unsafe_get0(); + create(coll_root, *t, cid, coll_info_t(i)); test_coll_mappings.emplace(cid, coll_info_t(i)); } submit_transaction(std::move(t)); @@ -161,7 +177,7 @@ TEST_F(collection_manager_test_t, update) auto iter2 = std::next(test_coll_mappings.begin(), 1); EXPECT_NE(iter1->second.split_bits, iter2->second.split_bits); auto t = tm->create_transaction(); - collection_manager->update(coll_root, *t, iter1->first, iter2->second).unsafe_get0(); + update(coll_root, *t, iter1->first, iter2->second); submit_transaction(std::move(t)); iter1->second.split_bits = iter2->second.split_bits; }