disable restart for destroy-pending repl-dev (#605)

Author: JacksonYao287

conanfile.py

@@ -9,7 +9,7 @@
 
 class HomestoreConan(ConanFile):
     name = "homestore"
-    version = "6.5.25"
+    version = "6.5.26"
 
     homepage = "https://github.com/eBay/Homestore"
     description = "HomeStore Storage Engine"

src/lib/logstore/log_dev.cpp

@@ -641,7 +641,10 @@ void LogDev::remove_log_store(logstore_id_t store_id) {
     {
         folly::SharedMutexWritePriority::WriteHolder holder(m_store_map_mtx);
         auto ret = m_id_logstore_map.erase(store_id);
-        HS_REL_ASSERT((ret == 1), "try to remove invalid store_id {}-{}", m_logdev_id, store_id);
+        if (ret == 0) {
+            LOGWARN("try to remove invalid store_id {}-{}", m_logdev_id, store_id);
+            return;
+        }
     }
     unreserve_store_id(store_id);
 }

src/lib/logstore/log_store_service.cpp

@@ -286,7 +286,10 @@ void LogStoreService::remove_log_store(logdev_id_t logdev_id, logstore_id_t stor
     folly::SharedMutexWritePriority::WriteHolder holder(m_logdev_map_mtx);
     COUNTER_INCREMENT(m_metrics, logstores_count, 1);
     const auto it = m_id_logdev_map.find(logdev_id);
-    HS_REL_ASSERT((it != m_id_logdev_map.end()), "logdev id {} doesnt exist", logdev_id);
+    if (it == m_id_logdev_map.end()) {
+        HS_LOG(WARN, logstore, "logdev id {} doesnt exist", logdev_id);
+        return;
+    }
     it->second->remove_log_store(store_id);
     COUNTER_DECREMENT(m_metrics, logstores_count, 1);
 }

src/lib/replication/repl_dev/raft_repl_dev.cpp

@@ -1197,11 +1197,13 @@ std::shared_ptr< nuraft::state_machine > RaftReplDev::get_state_machine() { retu
 
 void RaftReplDev::permanent_destroy() {
     RD_LOGI("Permanent destroy for raft repl dev group_id={}", group_id_str());
-    m_rd_sb.destroy();
     m_raft_config_sb.destroy();
     m_data_journal->remove_store();
     logstore_service().destroy_log_dev(m_data_journal->logdev_id());
     m_stage.update([](auto* stage) { *stage = repl_dev_stage_t::PERMANENT_DESTROYED; });
+    // we should destroy repl_dev superblk only after all the resources are cleaned up, so that is crash recovery
+    // occurs, we have a chance to find the stale repl_dev and reclaim all the stale resources.
+    m_rd_sb.destroy();
 }
 
 void RaftReplDev::leave() {

src/lib/replication/service/raft_repl_service.cpp

@@ -128,7 +128,8 @@ void RaftReplService::start() {
     // We need to first load the repl_dev with its config and then attach the raft config to that repl dev.
     for (auto const& [buf, mblk] : m_config_sb_bufs) {
         auto rdev = raft_group_config_found(buf, voidptr_cast(mblk));
-        rdev->on_restart();
+        // if repl_dev is in destroy_pending state, it will not be loaded.
+        if (rdev) rdev->on_restart();
     }
     m_config_sb_bufs.clear();
     LOGINFO("Repl devs load completed, calling upper layer on_repl_devs_init_completed");
@@ -383,7 +384,22 @@ void RaftReplService::load_repl_dev(sisl::byte_view const& buf, void* meta_cooki
     }
 
     if (rd_sb->destroy_pending == 0x1) {
-        LOGINFOMOD(replication, "ReplDev group_id={} was destroyed, skipping the load", group_id);
+        LOGINFOMOD(replication, "ReplDev group_id={} was destroyed, reclaim the stale resource", group_id);
+        // if we do not add the repl_dev to m_rd_map, it will not be permanently destroyed since gc thread finds the
+        // pending destroy repl_dev only from m_rd_map. so, we should try to reclaim all the repl_dev stale resources
+        // here.
+
+        // 1 since we permanantly destroy the repl_dev here, it will not join_raft group where raft_server will be
+        // created. hence , no need to detroy it through nuraft_mesg, where raft_server will be shutdown.
+        // 2  m_raft_config_sb will be destroyed in raft_group_config_found() method if repl_dev is is not found, so
+        // skip it.
+
+        // 3 logdev will be destroyed in delete_unopened_logdevs() if we don't open it(create repl_dev) here, so skip
+        // it.
+
+        // 4 destroy the superblk, and after this,  the repl_dev will not be loaded and found again.
+        rd_sb.destroy();
+
         return;
     }