From 52e7f76094ed5f97e8fda9246ca62a0267268995 Mon Sep 17 00:00:00 2001 From: zilai Date: Tue, 5 Dec 2023 23:21:49 -0700 Subject: [PATCH] disable copy construction for homestore superblock --- src/include/homestore/index/index_table.hpp | 3 +-- src/include/homestore/index_service.hpp | 2 +- src/include/homestore/superblk_handler.hpp | 24 +++++++++++++++++++ src/lib/index/index_service.cpp | 2 +- .../replication/repl_dev/solo_repl_dev.cpp | 7 +++--- src/lib/replication/repl_dev/solo_repl_dev.h | 4 ++-- .../replication/service/repl_service_impl.cpp | 13 +++++----- .../replication/service/repl_service_impl.h | 2 +- src/tests/test_index_btree.cpp | 8 +++---- 9 files changed, 44 insertions(+), 21 deletions(-) diff --git a/src/include/homestore/index/index_table.hpp b/src/include/homestore/index/index_table.hpp index bce7e8f36..34c48593e 100644 --- a/src/include/homestore/index/index_table.hpp +++ b/src/include/homestore/index/index_table.hpp @@ -45,8 +45,7 @@ class IndexTable : public IndexTableBase, public Btree< K, V > { if (status != btree_status_t::success) { throw std::runtime_error(fmt::format("Unable to create root node")); } } - IndexTable(const superblk< index_table_sb >& sb, const BtreeConfig& cfg) : Btree< K, V >{cfg} { - m_sb = sb; + IndexTable(superblk< index_table_sb >&& sb, const BtreeConfig& cfg) : Btree< K, V >{cfg}, m_sb{std::move(sb)} { Btree< K, V >::set_root_node_info(BtreeLinkInfo{m_sb->root_node, m_sb->link_version}); } diff --git a/src/include/homestore/index_service.hpp b/src/include/homestore/index_service.hpp index 09543a5fb..a4fb5bf14 100644 --- a/src/include/homestore/index_service.hpp +++ b/src/include/homestore/index_service.hpp @@ -32,7 +32,7 @@ class VirtualDev; class IndexServiceCallbacks { public: virtual ~IndexServiceCallbacks() = default; - virtual std::shared_ptr< IndexTableBase > on_index_table_found(superblk< index_table_sb > const&) { + virtual std::shared_ptr< IndexTableBase > on_index_table_found(superblk< index_table_sb >&&) { assert(0); return nullptr; } diff --git a/src/include/homestore/superblk_handler.hpp b/src/include/homestore/superblk_handler.hpp index d0c234dd9..2c74c2059 100644 --- a/src/include/homestore/superblk_handler.hpp +++ b/src/include/homestore/superblk_handler.hpp @@ -32,6 +32,30 @@ class superblk { superblk(const std::string& meta_name = "") { set_name(meta_name); } + superblk(const superblk&) = delete; + superblk& operator=(const superblk&) = delete; + + superblk(superblk&& rhs) noexcept + : m_meta_mgr_cookie(rhs.m_meta_mgr_cookie) + , m_raw_buf(std::move(rhs.m_raw_buf)) + , m_sb(rhs.m_sb) + , m_metablk_name(std::move(rhs.m_metablk_name)) { + rhs.m_meta_mgr_cookie = nullptr; + rhs.m_sb = nullptr; + } + + superblk& operator=(superblk&& rhs) noexcept { + if (this != &rhs) { + m_meta_mgr_cookie = rhs.m_meta_mgr_cookie; + m_raw_buf = std::move(rhs.m_raw_buf); + m_sb = rhs.m_sb; + m_metablk_name = std::move(rhs.m_metablk_name); + rhs.m_meta_mgr_cookie = nullptr; + rhs.m_sb = nullptr; + } + return *this; + } + void set_name(const std::string& meta_name) { if (meta_name.empty()) { m_metablk_name = "meta_blk_" + std::to_string(next_count()); diff --git a/src/lib/index/index_service.cpp b/src/lib/index/index_service.cpp index bfce92f09..eebc63d7f 100644 --- a/src/lib/index/index_service.cpp +++ b/src/lib/index/index_service.cpp @@ -63,7 +63,7 @@ void IndexService::meta_blk_found(const sisl::byte_view& buf, void* meta_cookie) // IndexTable instance superblk< index_table_sb > sb; sb.load(buf, meta_cookie); - add_index_table(m_svc_cbs->on_index_table_found(sb)); + add_index_table(m_svc_cbs->on_index_table_found(std::move(sb))); } void IndexService::start() { diff --git a/src/lib/replication/repl_dev/solo_repl_dev.cpp b/src/lib/replication/repl_dev/solo_repl_dev.cpp index 7620c0b98..35d3e6931 100644 --- a/src/lib/replication/repl_dev/solo_repl_dev.cpp +++ b/src/lib/replication/repl_dev/solo_repl_dev.cpp @@ -5,8 +5,8 @@ #include "replication/repl_dev/solo_repl_dev.h" namespace homestore { -SoloReplDev::SoloReplDev(superblk< repl_dev_superblk > const& rd_sb, bool load_existing) : - m_rd_sb{rd_sb}, m_group_id{m_rd_sb->gid} { +SoloReplDev::SoloReplDev(superblk< repl_dev_superblk >&& rd_sb, bool load_existing) : + m_rd_sb{std::move(rd_sb)}, m_group_id{m_rd_sb->gid} { if (load_existing) { logstore_service().open_log_store(LogStoreService::DATA_LOG_FAMILY_IDX, m_rd_sb->data_journal_id, true, bind_this(SoloReplDev::on_data_journal_created, 1)); @@ -14,6 +14,7 @@ SoloReplDev::SoloReplDev(superblk< repl_dev_superblk > const& rd_sb, bool load_e m_data_journal = logstore_service().create_new_log_store(LogStoreService::DATA_LOG_FAMILY_IDX, true /* append_mode */); m_rd_sb->data_journal_id = m_data_journal->get_store_id(); + m_rd_sb.write(); } } @@ -144,4 +145,4 @@ void repl_req_ctx::alloc_journal_entry(uint32_t size) { repl_req_ctx::~repl_req_ctx() { if (journal_entry) { journal_entry->~repl_journal_entry(); } } -} // namespace homestore \ No newline at end of file +} // namespace homestore diff --git a/src/lib/replication/repl_dev/solo_repl_dev.h b/src/lib/replication/repl_dev/solo_repl_dev.h index 684378a13..1ea2367b1 100644 --- a/src/lib/replication/repl_dev/solo_repl_dev.h +++ b/src/lib/replication/repl_dev/solo_repl_dev.h @@ -70,7 +70,7 @@ class SoloReplDev : public ReplDev { std::atomic< logstore_seq_num_t > m_commit_upto{-1}; public: - SoloReplDev(superblk< repl_dev_superblk > const& rd_sb, bool load_existing); + SoloReplDev(superblk< repl_dev_superblk >&& rd_sb, bool load_existing); virtual ~SoloReplDev() = default; void async_alloc_write(sisl::blob const& header, sisl::blob const& key, sisl::sg_list const& value, @@ -96,4 +96,4 @@ class SoloReplDev : public ReplDev { void on_log_found(logstore_seq_num_t lsn, log_buffer buf, void* ctx); }; -} // namespace homestore \ No newline at end of file +} // namespace homestore diff --git a/src/lib/replication/service/repl_service_impl.cpp b/src/lib/replication/service/repl_service_impl.cpp index f93c25e8a..e439488ca 100644 --- a/src/lib/replication/service/repl_service_impl.cpp +++ b/src/lib/replication/service/repl_service_impl.cpp @@ -63,10 +63,9 @@ ReplicationServiceImpl::create_repl_dev(uuid_t group_id, std::set< std::string, rd_sb.create(sizeof(repl_dev_superblk)); rd_sb->gid = group_id; - shared< ReplDev > repl_dev = create_repl_dev_instance(rd_sb, false /* load_existing */); + shared< ReplDev > repl_dev = create_repl_dev_instance(std::move(rd_sb), false /* load_existing */); listener->set_repl_dev(repl_dev.get()); repl_dev->attach_listener(std::move(listener)); - rd_sb.write(); return make_async_success(std::move(repl_dev)); } @@ -115,7 +114,7 @@ folly::Future< ReplServiceError > ReplicationServiceImpl::replace_member(uuid_t return folly::makeFuture< ReplServiceError >(ReplServiceError::NOT_IMPLEMENTED); } -shared< ReplDev > ReplicationServiceImpl::create_repl_dev_instance(superblk< repl_dev_superblk > const& rd_sb, +shared< ReplDev > ReplicationServiceImpl::create_repl_dev_instance(superblk< repl_dev_superblk >&& rd_sb, bool load_existing) { auto it = m_rd_map.end(); bool happened = false; @@ -129,7 +128,7 @@ shared< ReplDev > ReplicationServiceImpl::create_repl_dev_instance(superblk< rep shared< ReplDev > repl_dev; if (m_repl_type == repl_impl_type::solo) { - repl_dev = std::make_shared< SoloReplDev >(rd_sb, load_existing); + repl_dev = std::make_shared< SoloReplDev >(std::move(rd_sb), load_existing); } else { HS_REL_ASSERT(false, "Repl impl type = {} is not supported yet", enum_name(m_repl_type)); } @@ -143,11 +142,11 @@ void ReplicationServiceImpl::rd_super_blk_found(sisl::byte_view const& buf, void rd_sb.load(buf, meta_cookie); HS_DBG_ASSERT_EQ(rd_sb->get_magic(), repl_dev_superblk::REPL_DEV_SB_MAGIC, "Invalid rdev metablk, magic mismatch"); HS_DBG_ASSERT_EQ(rd_sb->get_version(), repl_dev_superblk::REPL_DEV_SB_VERSION, "Invalid version of rdev metablk"); - - shared< ReplDev > repl_dev = create_repl_dev_instance(rd_sb, true /* load_existing */); + auto rd_sb_gid = rd_sb->gid; + shared< ReplDev > repl_dev = create_repl_dev_instance(std::move(rd_sb), true /* load_existing */); { std::unique_lock lg(m_rd_map_mtx); - auto it = m_pending_open.find(rd_sb->gid); + auto it = m_pending_open.find(rd_sb_gid); if (it != m_pending_open.end()) { auto& li_info = it->second; // Someone waiting for this repl dev to open, call them to attach the listener and provide the value diff --git a/src/lib/replication/service/repl_service_impl.h b/src/lib/replication/service/repl_service_impl.h index bcee4488f..3a0e9493d 100644 --- a/src/lib/replication/service/repl_service_impl.h +++ b/src/lib/replication/service/repl_service_impl.h @@ -73,7 +73,7 @@ class ReplicationServiceImpl : public ReplicationService { private: - shared< ReplDev > create_repl_dev_instance(superblk< repl_dev_superblk > const& rd_sb, bool load_existing); + shared< ReplDev > create_repl_dev_instance(superblk< repl_dev_superblk > &&rd_sb, bool load_existing); void rd_super_blk_found(sisl::byte_view const& buf, void* meta_cookie); }; diff --git a/src/tests/test_index_btree.cpp b/src/tests/test_index_btree.cpp index e110a931d..b5f9eabaf 100644 --- a/src/tests/test_index_btree.cpp +++ b/src/tests/test_index_btree.cpp @@ -109,10 +109,10 @@ struct BtreeTest : public BtreeTestHelper< TestType > { class TestIndexServiceCallbacks : public IndexServiceCallbacks { public: TestIndexServiceCallbacks(BtreeTest* test) : m_test(test) {} - std::shared_ptr< IndexTableBase > on_index_table_found(const superblk< index_table_sb >& sb) override { + std::shared_ptr< IndexTableBase > on_index_table_found(superblk< index_table_sb >&& sb) override { LOGINFO("Index table recovered"); LOGINFO("Root bnode_id {} version {}", sb->root_node, sb->link_version); - m_test->m_bt = std::make_shared< typename T::BtreeType >(sb, m_test->m_cfg); + m_test->m_bt = std::make_shared< typename T::BtreeType >(std::move(sb), m_test->m_cfg); return m_test->m_bt; } @@ -464,10 +464,10 @@ struct BtreeConcurrentTest : public BtreeTestHelper< TestType > { class TestIndexServiceCallbacks : public IndexServiceCallbacks { public: TestIndexServiceCallbacks(BtreeConcurrentTest* test) : m_test(test) {} - std::shared_ptr< IndexTableBase > on_index_table_found(const superblk< index_table_sb >& sb) override { + std::shared_ptr< IndexTableBase > on_index_table_found(superblk< index_table_sb >&& sb) override { LOGINFO("Index table recovered"); LOGINFO("Root bnode_id {} version {}", sb->root_node, sb->link_version); - m_test->m_bt = std::make_shared< typename T::BtreeType >(sb, m_test->m_cfg); + m_test->m_bt = std::make_shared< typename T::BtreeType >(std::move(sb), m_test->m_cfg); return m_test->m_bt; }