From c1451ea279caab3374492877ce2d8479370e60fe Mon Sep 17 00:00:00 2001 From: Yingxin Cheng Date: Sun, 7 Aug 2022 16:05:42 +0800 Subject: [PATCH] crimson/os/seastore: construct TransactionManager classes after device mount To construct TransactionManager after all the devices are discoverred. Also, it makes the following cleanups possible: * Cleanup SeaStore and TransactionManager factory methods. * Decouple TransactionManager from SegmentManagerGroup. * Drop the unnecessary tm_make_config_t. * Drop the unnecessary add_device() methods. Signed-off-by: Yingxin Cheng --- .../os/seastore/extent_placement_manager.h | 4 - .../journal/circular_bounded_journal.h | 4 +- src/crimson/os/seastore/seastore.cc | 92 +++++++++++-------- src/crimson/os/seastore/seastore.h | 20 ++-- .../os/seastore/transaction_manager.cc | 35 +++++-- src/crimson/os/seastore/transaction_manager.h | 57 +----------- src/crimson/tools/store_nbd/tm_driver.cc | 7 +- .../onode_tree/test_fltree_onode_manager.cc | 1 - .../seastore/test_transaction_manager.cc | 4 +- .../seastore/transaction_manager_test_state.h | 50 ++++------ 10 files changed, 116 insertions(+), 158 deletions(-) diff --git a/src/crimson/os/seastore/extent_placement_manager.h b/src/crimson/os/seastore/extent_placement_manager.h index 4f3dae14a672d..61ed07d0348f7 100644 --- a/src/crimson/os/seastore/extent_placement_manager.h +++ b/src/crimson/os/seastore/extent_placement_manager.h @@ -250,10 +250,6 @@ public: return crimson::do_for_each(md_writers_by_gen, [](auto &writer) { return writer->close(); }); - }).safe_then([this] { - devices_by_id.clear(); - devices_by_id.resize(DEVICE_ID_MAX, nullptr); - primary_device = nullptr; }); } diff --git a/src/crimson/os/seastore/journal/circular_bounded_journal.h b/src/crimson/os/seastore/journal/circular_bounded_journal.h index 59bb689b9beaf..4a310806a95f9 100644 --- a/src/crimson/os/seastore/journal/circular_bounded_journal.h +++ b/src/crimson/os/seastore/journal/circular_bounded_journal.h @@ -277,9 +277,7 @@ public: rbm_abs_addr get_journal_end() const { return get_start_addr() + header.size + get_block_size(); // journal size + header length } - void add_device(RBMDevice* dev) { - device = dev; - } + private: cbj_header_t header; RBMDevice* device; diff --git a/src/crimson/os/seastore/seastore.cc b/src/crimson/os/seastore/seastore.cc index a6161b1f3ae83..53bb7e03e4ff8 100644 --- a/src/crimson/os/seastore/seastore.cc +++ b/src/crimson/os/seastore/seastore.cc @@ -117,32 +117,17 @@ SeaStore::SeaStore( const std::string& root, MDStoreRef mdstore, DeviceRef dev, - TransactionManagerRef tm, - CollectionManagerRef cm, - OnodeManagerRef om) + bool is_test) : root(root), mdstore(std::move(mdstore)), device(std::move(dev)), - transaction_manager(std::move(tm)), - collection_manager(std::move(cm)), - onode_manager(std::move(om)), max_object_size( - get_conf("seastore_default_max_object_size")) + get_conf("seastore_default_max_object_size")), + is_test(is_test) { register_metrics(); } -SeaStore::SeaStore( - const std::string& root, - DeviceRef dev, - TransactionManagerRef tm, - CollectionManagerRef cm, - OnodeManagerRef om) - : SeaStore( - root, - std::make_unique(root), - std::move(dev), std::move(tm), std::move(cm), std::move(om)) {} - SeaStore::~SeaStore() = default; void SeaStore::register_metrics() @@ -185,10 +170,8 @@ seastar::future<> SeaStore::stop() SeaStore::mount_ertr::future<> SeaStore::mount() { - secondaries.clear(); return device->mount( ).safe_then([this] { - transaction_manager->add_device(device.get(), true); auto sec_devices = device->get_secondary_devices(); return crimson::do_for_each(sec_devices, [this](auto& device_entry) { device_id_t id = device_entry.first; @@ -202,12 +185,12 @@ SeaStore::mount_ertr::future<> SeaStore::mount() ).safe_then([this, sec_dev=std::move(sec_dev), magic]() mutable { boost::ignore_unused(magic); // avoid clang warning; assert(sec_dev->get_magic() == magic); - transaction_manager->add_device(sec_dev.get(), false); secondaries.emplace_back(std::move(sec_dev)); }); }); }); }).safe_then([this] { + init_managers(); return transaction_manager->mount(); }).handle_error( crimson::ct_error::assert_all{ @@ -218,8 +201,13 @@ SeaStore::mount_ertr::future<> SeaStore::mount() seastar::future<> SeaStore::umount() { - return transaction_manager->close( - ).safe_then([this] { + return [this] { + if (transaction_manager) { + return transaction_manager->close(); + } else { + return TransactionManager::close_ertr::now(); + } + }().safe_then([this] { return crimson::do_for_each( secondaries, [](auto& sec_dev) -> SegmentManager::close_ertr::future<> @@ -228,6 +216,11 @@ seastar::future<> SeaStore::umount() }); }).safe_then([this] { return device->close(); + }).safe_then([this] { + secondaries.clear(); + transaction_manager.reset(); + collection_manager.reset(); + onode_manager.reset(); }).handle_error( crimson::ct_error::assert_all{ "Invalid error in SeaStore::umount" @@ -328,23 +321,16 @@ SeaStore::mkfs_ertr::future<> SeaStore::mkfs(uuid_d new_osd_fsid) sds} ); }).safe_then([this] { - return crimson::do_for_each(secondaries, [this](auto& sec_dev) { - return sec_dev->mount().safe_then([this, &sec_dev] { - transaction_manager->add_device(sec_dev.get(), false); - return seastar::now(); - }); + return crimson::do_for_each(secondaries, [](auto& sec_dev) { + return sec_dev->mount(); }); }); }).safe_then([this] { return device->mount(); }).safe_then([this] { - transaction_manager->add_device(device.get(), true); + init_managers(); return transaction_manager->mkfs(); }).safe_then([this] { - for (auto& sec_dev : secondaries) { - transaction_manager->add_device(sec_dev.get(), false); - } - transaction_manager->add_device(device.get(), true); return transaction_manager->mount(); }).safe_then([this] { return repeat_eagain([this] { @@ -1872,6 +1858,23 @@ uuid_d SeaStore::get_fsid() const return device->get_meta().seastore_id; } +void SeaStore::init_managers() +{ + ceph_assert(!transaction_manager); + ceph_assert(!collection_manager); + ceph_assert(!onode_manager); + std::vector sec_devices; + for (auto &dev : secondaries) { + sec_devices.emplace_back(dev.get()); + } + transaction_manager = make_transaction_manager( + device.get(), sec_devices, is_test); + collection_manager = std::make_unique( + *transaction_manager); + onode_manager = std::make_unique( + *transaction_manager); +} + seastar::future> make_seastore( const std::string &device, const ConfigValues &config) @@ -1880,19 +1883,28 @@ seastar::future> make_seastore( device ).then([&device](DeviceRef device_obj) { #ifndef NDEBUG - auto tm = make_transaction_manager( - tm_make_config_t::get_test_segmented_journal()); + bool is_test = true; #else - auto tm = make_transaction_manager(tm_make_config_t::get_default()); + bool is_test = false; #endif - auto cm = std::make_unique(*tm); + auto mdstore = std::make_unique(device); return std::make_unique( device, + std::move(mdstore), std::move(device_obj), - std::move(tm), - std::move(cm), - std::make_unique(*tm)); + is_test); }); } +std::unique_ptr make_test_seastore( + DeviceRef device, + SeaStore::MDStoreRef mdstore) +{ + return std::make_unique( + "", + std::move(mdstore), + std::move(device), + true); +} + } diff --git a/src/crimson/os/seastore/seastore.h b/src/crimson/os/seastore/seastore.h index b3abd0e3b4b19..4163deafe8823 100644 --- a/src/crimson/os/seastore/seastore.h +++ b/src/crimson/os/seastore/seastore.h @@ -82,15 +82,7 @@ public: const std::string& root, MDStoreRef mdstore, DeviceRef device, - TransactionManagerRef tm, - CollectionManagerRef cm, - OnodeManagerRef om); - SeaStore( - const std::string& root, - DeviceRef device, - TransactionManagerRef tm, - CollectionManagerRef cm, - OnodeManagerRef om); + bool is_test); ~SeaStore(); seastar::future<> stop() final; @@ -331,14 +323,18 @@ private: const std::optional &_start, OMapManager::omap_list_config_t config); + void init_managers(); + std::string root; MDStoreRef mdstore; DeviceRef device; + const uint32_t max_object_size = 0; + bool is_test; + std::vector secondaries; TransactionManagerRef transaction_manager; CollectionManagerRef collection_manager; OnodeManagerRef onode_manager; - const uint32_t max_object_size = 0; using tm_iertr = TransactionManager::base_iertr; using tm_ret = tm_iertr::future<>; @@ -447,4 +443,8 @@ private: seastar::future> make_seastore( const std::string &device, const ConfigValues &config); + +std::unique_ptr make_test_seastore( + DeviceRef device, + SeaStore::MDStoreRef mdstore); } diff --git a/src/crimson/os/seastore/transaction_manager.cc b/src/crimson/os/seastore/transaction_manager.cc index 31223dffa020e..f5dee399be7b6 100644 --- a/src/crimson/os/seastore/transaction_manager.cc +++ b/src/crimson/os/seastore/transaction_manager.cc @@ -8,6 +8,7 @@ #include "crimson/os/seastore/transaction_manager.h" #include "crimson/os/seastore/journal.h" #include "crimson/os/seastore/lba_manager/btree/lba_btree_node.h" +#include "crimson/os/seastore/random_block_manager/rbm_device.h" /* * TransactionManager logs @@ -34,8 +35,7 @@ TransactionManager::TransactionManager( lba_manager(std::move(_lba_manager)), journal(std::move(_journal)), epm(std::move(epm)), - backref_manager(std::move(backref_manager)), - sm_group(*async_cleaner->get_segment_manager_group()) + backref_manager(std::move(backref_manager)) { async_cleaner->set_extent_callback(this); journal->set_write_pipeline(&write_pipeline); @@ -170,9 +170,8 @@ TransactionManager::close_ertr::future<> TransactionManager::close() { return journal->close(); }).safe_then([this] { return epm->close(); - }).safe_then([FNAME, this] { + }).safe_then([FNAME] { INFO("completed"); - sm_group.reset(); return seastar::now(); }); } @@ -621,7 +620,10 @@ TransactionManager::get_extents_if_live_ret TransactionManager::get_extents_if_l TransactionManager::~TransactionManager() {} -TransactionManagerRef make_transaction_manager(tm_make_config_t config) +TransactionManagerRef make_transaction_manager( + Device *primary_device, + const std::vector &secondary_devices, + bool is_test) { LOG_PREFIX(make_transaction_manager); auto epm = std::make_unique(); @@ -630,9 +632,19 @@ TransactionManagerRef make_transaction_manager(tm_make_config_t config) auto sms = std::make_unique(); auto backref_manager = create_backref_manager(*cache); + epm->add_device(primary_device, true); + if (primary_device->get_device_type() == device_type_t::SEGMENTED) { + sms->add_segment_manager(static_cast(primary_device)); + } + for (auto &p_dev : secondary_devices) { + epm->add_device(p_dev, false); + ceph_assert(p_dev->get_device_type() == device_type_t::SEGMENTED); + sms->add_segment_manager(static_cast(p_dev)); + } + bool cleaner_is_detailed; AsyncCleaner::config_t cleaner_config; - if (config.is_test) { + if (is_test) { cleaner_is_detailed = true; cleaner_config = AsyncCleaner::config_t::get_test(); } else { @@ -645,15 +657,18 @@ TransactionManagerRef make_transaction_manager(tm_make_config_t config) *backref_manager, cleaner_is_detailed); + auto p_device_type = primary_device->get_device_type(); JournalRef journal; - if (config.j_type == journal_type_t::SEGMENT_JOURNAL) { + if (p_device_type == device_type_t::SEGMENTED) { journal = journal::make_segmented(*async_cleaner); } else { + ceph_assert(p_device_type == device_type_t::RANDOM_BLOCK); journal = journal::make_circularbounded( - nullptr, ""); + static_cast(primary_device), + ""); async_cleaner->set_disable_trim(true); - ERROR("disabling journal trimming since support for CircularBoundedJournal\ - hasn't been added yet"); + ERROR("disabling journal trimming since support for CircularBoundedJournal " + "hasn't been added yet"); } epm->init_ool_writers( *async_cleaner, diff --git a/src/crimson/os/seastore/transaction_manager.h b/src/crimson/os/seastore/transaction_manager.h index b3dec1d6a27df..ba0c36ed523c6 100644 --- a/src/crimson/os/seastore/transaction_manager.h +++ b/src/crimson/os/seastore/transaction_manager.h @@ -29,48 +29,10 @@ #include "crimson/os/seastore/journal.h" #include "crimson/os/seastore/extent_placement_manager.h" #include "crimson/os/seastore/device.h" -#include "crimson/os/seastore/segment_manager_group.h" namespace crimson::os::seastore { class Journal; -struct tm_make_config_t { - bool is_test; - journal_type_t j_type; - - static tm_make_config_t get_default() { - return tm_make_config_t { - false, - journal_type_t::SEGMENT_JOURNAL - }; - } - static tm_make_config_t get_test_segmented_journal() { - LOG_PREFIX(get_test_segmented_journal); - SUBWARN(seastore_tm, "test mode enabled!"); - return tm_make_config_t { - true, - journal_type_t::SEGMENT_JOURNAL - }; - } - static tm_make_config_t get_test_cb_journal() { - LOG_PREFIX(get_test_cb_journal); - SUBWARN(seastore_tm, "test mode enabled!"); - return tm_make_config_t { - true, - journal_type_t::CIRCULARBOUNDED_JOURNAL - }; - } - - tm_make_config_t(const tm_make_config_t &) = default; - tm_make_config_t &operator=(const tm_make_config_t &) = default; -private: - tm_make_config_t( - bool is_test, - journal_type_t j_type) - : is_test(is_test), j_type(j_type) - {} -}; - template auto repeat_eagain(F &&f) { return seastar::do_with( @@ -625,19 +587,6 @@ public: return async_cleaner->stat(); } - void add_device(Device* dev, bool is_primary) { - LOG_PREFIX(TransactionManager::add_device); - SUBDEBUG(seastore_tm, "adding device {}, is_primary={}", - dev->get_device_id(), is_primary); - epm->add_device(dev, is_primary); - - if (dev->get_device_type() == device_type_t::SEGMENTED) { - auto sm = dynamic_cast(dev); - ceph_assert(sm != nullptr); - sm_group.add_segment_manager(sm); - } - } - ~TransactionManager(); private: @@ -649,7 +598,6 @@ private: JournalRef journal; ExtentPlacementManagerRef epm; BackrefManagerRef backref_manager; - SegmentManagerGroup &sm_group; WritePipeline write_pipeline; @@ -680,5 +628,8 @@ public: }; using TransactionManagerRef = std::unique_ptr; -TransactionManagerRef make_transaction_manager(tm_make_config_t config); +TransactionManagerRef make_transaction_manager( + Device *primary_device, + const std::vector &secondary_devices, + bool is_test); } diff --git a/src/crimson/tools/store_nbd/tm_driver.cc b/src/crimson/tools/store_nbd/tm_driver.cc index 26d126ae3efd0..c31319a52229f 100644 --- a/src/crimson/tools/store_nbd/tm_driver.cc +++ b/src/crimson/tools/store_nbd/tm_driver.cc @@ -131,13 +131,12 @@ seastar::future TMDriver::read( void TMDriver::init() { + std::vector sec_devices; #ifndef NDEBUG - tm = make_transaction_manager( - tm_make_config_t::get_test_segmented_journal()); + tm = make_transaction_manager(device.get(), sec_devices, true); #else - tm = make_transaction_manager(tm_make_config_t::get_default()); + tm = make_transaction_manager(device.get(), sec_devices, false); #endif - tm->add_device(device.get(), true); } void TMDriver::clear() 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 0da0cce18441b..72b903e414e58 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 @@ -84,7 +84,6 @@ struct fltree_onode_manager_test_t virtual FuturizedStore::mkfs_ertr::future<> _mkfs() final { return TMTestState::_mkfs( ).safe_then([this] { - tm->add_device(segment_manager.get(), true); return tm->mount( ).safe_then([this] { return repeat_eagain([this] { diff --git a/src/test/crimson/seastore/test_transaction_manager.cc b/src/test/crimson/seastore/test_transaction_manager.cc index 2f83f190a3b8d..6cfc5b6073315 100644 --- a/src/test/crimson/seastore/test_transaction_manager.cc +++ b/src/test/crimson/seastore/test_transaction_manager.cc @@ -75,9 +75,9 @@ struct transaction_manager_test_t : seastar::future<> set_up_fut() final { std::string j_type = GetParam(); if (j_type == "segmented") { - return tm_setup(tm_make_config_t::get_test_segmented_journal()); + return tm_setup(journal_type_t::SEGMENT_JOURNAL); } else if (j_type == "circularbounded") { - return tm_setup(tm_make_config_t::get_test_cb_journal()); + return tm_setup(journal_type_t::CIRCULARBOUNDED_JOURNAL); } else { ceph_assert(0 == "no support"); } diff --git a/src/test/crimson/seastore/transaction_manager_test_state.h b/src/test/crimson/seastore/transaction_manager_test_state.h index 5176540147711..1245999a2fa1a 100644 --- a/src/test/crimson/seastore/transaction_manager_test_state.h +++ b/src/test/crimson/seastore/transaction_manager_test_state.h @@ -27,7 +27,7 @@ protected: segment_manager::EphemeralSegmentManagerRef segment_manager; std::list secondary_segment_managers; std::unique_ptr rb_device; - tm_make_config_t tm_config = tm_make_config_t::get_test_segmented_journal(); + journal_type_t journal_type; EphemeralTestState(std::size_t num_segment_managers) { assert(num_segment_managers > 0); @@ -67,15 +67,16 @@ protected: } seastar::future<> tm_setup( - tm_make_config_t config = tm_make_config_t::get_test_segmented_journal()) { + journal_type_t type = journal_type_t::SEGMENT_JOURNAL) { LOG_PREFIX(EphemeralTestState::tm_setup); SUBINFO(test, "begin with {} devices ...", get_num_devices()); - tm_config = config; + journal_type = type; + // FIXME: should not initialize segment_manager with circularbounded-journal segment_manager = segment_manager::create_test_ephemeral(); for (auto &sec_sm : secondary_segment_managers) { sec_sm = segment_manager::create_test_ephemeral(); } - if (tm_config.j_type == journal_type_t::CIRCULARBOUNDED_JOURNAL) { + if (journal_type == journal_type_t::CIRCULARBOUNDED_JOURNAL) { auto config = journal::CircularBoundedJournal::mkfs_config_t::get_default(); rb_device.reset(new random_block_device::TestMemory(config.total_size)); @@ -140,19 +141,6 @@ protected: } }; -auto get_seastore(SeaStore::MDStoreRef mdstore, SegmentManagerRef sm) { - auto tm = make_transaction_manager(tm_make_config_t::get_test_segmented_journal()); - auto cm = std::make_unique(*tm); - return std::make_unique( - "", - std::move(mdstore), - std::move(sm), - std::move(tm), - std::move(cm), - std::make_unique(*tm)); -} - - class TMTestState : public EphemeralTestState { protected: TransactionManagerRef tm; @@ -166,17 +154,17 @@ protected: TMTestState(std::size_t num_devices) : EphemeralTestState(num_devices) {} virtual void _init() override { - tm = make_transaction_manager(tm_config); - tm->add_device(segment_manager.get(), true); - if (tm_config.j_type == journal_type_t::CIRCULARBOUNDED_JOURNAL) { - tm->add_device(rb_device.get(), false); - static_cast(tm->get_journal())-> - add_device(rb_device.get()); + std::vector sec_devices; + for (auto &sec_sm : secondary_segment_managers) { + sec_devices.emplace_back(sec_sm.get()); } - if (get_num_devices() > 1) { - for (auto &sec_sm : secondary_segment_managers) { - tm->add_device(sec_sm.get(), false); - } + if (journal_type == journal_type_t::CIRCULARBOUNDED_JOURNAL) { + // FIXME: should not initialize segment_manager with circularbounded-journal + // FIXME: no secondary device in the single device test + sec_devices.emplace_back(segment_manager.get()); + tm = make_transaction_manager(rb_device.get(), sec_devices, true); + } else { + tm = make_transaction_manager(segment_manager.get(), sec_devices, true); } async_cleaner = tm->get_async_cleaner(); lba_manager = tm->get_lba_manager(); @@ -211,7 +199,7 @@ protected: } virtual FuturizedStore::mkfs_ertr::future<> _mkfs() { - if (tm_config.j_type == journal_type_t::SEGMENT_JOURNAL) { + if (journal_type == journal_type_t::SEGMENT_JOURNAL) { return tm->mkfs( ).handle_error( crimson::ct_error::assert_all{"Error in mkfs"} @@ -365,9 +353,9 @@ protected: SeaStoreTestState() : EphemeralTestState(1) {} virtual void _init() final { - seastore = get_seastore( - std::make_unique(mdstore_state.get_mdstore()), - std::make_unique(*segment_manager)); + seastore = make_test_seastore( + std::make_unique(*segment_manager), + std::make_unique(mdstore_state.get_mdstore())); } virtual void _destroy() final { -- 2.39.5