diff --git a/conanfile.py b/conanfile.py index 4ba2982ed..da0de7d08 100644 --- a/conanfile.py +++ b/conanfile.py @@ -9,7 +9,7 @@ class HomestoreConan(ConanFile): name = "homestore" - version = "7.6.1" + version = "7.6.2" homepage = "https://github.com/eBay/Homestore" description = "HomeStore Storage Engine" diff --git a/src/lib/replication/service/generic_repl_svc.cpp b/src/lib/replication/service/generic_repl_svc.cpp index 4e34a94c3..9c95879ea 100644 --- a/src/lib/replication/service/generic_repl_svc.cpp +++ b/src/lib/replication/service/generic_repl_svc.cpp @@ -79,7 +79,7 @@ hs_stats GenericReplService::get_cap_stats() const { ///////////////////// SoloReplService specializations and CP Callbacks ///////////////////////////// SoloReplService::SoloReplService(cshared< ReplApplication >& repl_app) : GenericReplService{repl_app} {} -SoloReplService::~SoloReplService() {}; +SoloReplService::~SoloReplService(){}; void SoloReplService::start() { for (auto const& [buf, mblk] : m_sb_bufs) { @@ -151,23 +151,19 @@ folly::SemiFuture< ReplServiceError > SoloReplService::remove_repl_dev(group_id_ auto rdev_ptr = rdev.value(); - // 1. Firstly stop the repl dev which waits for any outstanding requests to finish + // 1. Stop: wait for outstanding requests rdev_ptr->stop(); - // 2. Destroy the repl dev which will remove the logstore and free the memory; - dp_cast< SoloReplDev >(rdev_ptr)->destroy(); - - // 3. detaches both ways: - // detach rdev from its listener and listener from rdev; - rdev_ptr->detach_listener(); + // 2. Remove from rd map first so CP flush/cleanup can no longer see this rdev. + // Taking the unique lock also waits out any in-flight iterate_repl_devs(). { - // 4. remove from rd map which finally call SoloReplDev's destructor because this is the last one holding ref to - // this instance; std::unique_lock lg(m_rd_map_mtx); m_rd_map.erase(group_id); } - // 5. now destroy the upper layer's listener instance; + // 3. Now it is safe to destroy logstore/logdev and the superblk + dp_cast< SoloReplDev >(rdev_ptr)->destroy(); + rdev_ptr->detach_listener(); m_repl_app->destroy_repl_dev_listener(group_id); return folly::makeSemiFuture(ReplServiceError::OK); diff --git a/src/lib/replication/service/raft_repl_service.h b/src/lib/replication/service/raft_repl_service.h index 1fdd62134..ed098ca2e 100644 --- a/src/lib/replication/service/raft_repl_service.h +++ b/src/lib/replication/service/raft_repl_service.h @@ -138,7 +138,7 @@ class ReplSvcCPContext : public CPContext { std::map< ReplDev*, cshared< ReplDevCPContext > > m_cp_ctx_map; public: - ReplSvcCPContext(CP* cp) : CPContext(cp) {}; + ReplSvcCPContext(CP* cp) : CPContext(cp){}; virtual ~ReplSvcCPContext() = default; int add_repl_dev_ctx(ReplDev* dev, cshared< ReplDevCPContext > dev_ctx); cshared< ReplDevCPContext > get_repl_dev_ctx(ReplDev* dev); diff --git a/src/tests/test_raft_repl_dev.cpp b/src/tests/test_raft_repl_dev.cpp index 11eccd446..c9121daa6 100644 --- a/src/tests/test_raft_repl_dev.cpp +++ b/src/tests/test_raft_repl_dev.cpp @@ -752,13 +752,13 @@ TEST_F(RaftReplDevTest, RaftLogTruncationTest) { auto pre_raft_logstore_reserve_threshold = 0; auto pre_raft_logstore_truncation_reserve_count = 0; - HS_SETTINGS_FACTORY().modifiable_settings([&pre_raft_logstore_reserve_threshold, - &pre_raft_logstore_truncation_reserve_count](auto& s) { - pre_raft_logstore_reserve_threshold = s.resource_limits.raft_logstore_reserve_threshold; - pre_raft_logstore_truncation_reserve_count = s.resource_limits.raft_logstore_truncation_reserve_count; - s.resource_limits.raft_logstore_reserve_threshold = 200; - s.resource_limits.raft_logstore_truncation_reserve_count = 1; - }); + HS_SETTINGS_FACTORY().modifiable_settings( + [&pre_raft_logstore_reserve_threshold, &pre_raft_logstore_truncation_reserve_count](auto& s) { + pre_raft_logstore_reserve_threshold = s.resource_limits.raft_logstore_reserve_threshold; + pre_raft_logstore_truncation_reserve_count = s.resource_limits.raft_logstore_truncation_reserve_count; + s.resource_limits.raft_logstore_reserve_threshold = 200; + s.resource_limits.raft_logstore_truncation_reserve_count = 1; + }); HS_SETTINGS_FACTORY().save(); uint64_t entries_per_attempt = 100; @@ -872,11 +872,11 @@ TEST_F(RaftReplDevTest, RaftLogTruncationTest) { // set the settings back and save. LOGINFO("Set raft logstore truncation settings back to previous values, reserve_threshold={}, reserve_count={}", pre_raft_logstore_reserve_threshold, pre_raft_logstore_truncation_reserve_count); - HS_SETTINGS_FACTORY().modifiable_settings([pre_raft_logstore_reserve_threshold, - pre_raft_logstore_truncation_reserve_count](auto& s) { - s.resource_limits.raft_logstore_reserve_threshold = pre_raft_logstore_reserve_threshold; - s.resource_limits.raft_logstore_truncation_reserve_count = pre_raft_logstore_truncation_reserve_count; - }); + HS_SETTINGS_FACTORY().modifiable_settings( + [pre_raft_logstore_reserve_threshold, pre_raft_logstore_truncation_reserve_count](auto& s) { + s.resource_limits.raft_logstore_reserve_threshold = pre_raft_logstore_reserve_threshold; + s.resource_limits.raft_logstore_truncation_reserve_count = pre_raft_logstore_truncation_reserve_count; + }); HS_SETTINGS_FACTORY().save(); g_helper->sync_for_cleanup_start();