From e36b56e9d548c671fbb735ffbf00637c0ac9c36c Mon Sep 17 00:00:00 2001 From: yawzhang Date: Thu, 13 Mar 2025 16:55:38 +0800 Subject: [PATCH] [TODO] can be used if there is a concurrency issues between gc and flush In previous implementation, there may be a concurrency issue between 'permanent_destroy' and 'cp_flush/flush_durable_commit_lsn': if m_rd_sb is destroyed at T1 then is written at T2, a NPE will occur. This pr is a fix way to avoid this issue by adding m_sb_mtx for destroy. --- src/lib/replication/repl_dev/raft_repl_dev.cpp | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/lib/replication/repl_dev/raft_repl_dev.cpp b/src/lib/replication/repl_dev/raft_repl_dev.cpp index 4b4a22213..dd231ae89 100644 --- a/src/lib/replication/repl_dev/raft_repl_dev.cpp +++ b/src/lib/replication/repl_dev/raft_repl_dev.cpp @@ -1291,6 +1291,7 @@ void RaftReplDev::permanent_destroy() { 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. + std::unique_lock lg{m_sb_mtx}; m_rd_sb.destroy(); } @@ -1409,24 +1410,18 @@ nuraft::cb_func::ReturnCode RaftReplDev::raft_event(nuraft::cb_func::Type type, } void RaftReplDev::flush_durable_commit_lsn() { + auto const lsn = m_commit_upto_lsn.load(); + std::unique_lock lg{m_sb_mtx}; if (is_destroyed()) { - RD_LOGI("Raft repl dev is destroyed, ignore flush durable commmit lsn"); + RD_LOGD("Raft repl dev is destroyed, ignore flush durable commmit lsn"); return; } - - auto const lsn = m_commit_upto_lsn.load(); - std::unique_lock lg{m_sb_mtx}; m_rd_sb->durable_commit_lsn = lsn; m_rd_sb.write(); } /////////////////////////////////// Private metohds //////////////////////////////////// void RaftReplDev::cp_flush(CP* cp, cshared< ReplDevCPContext > ctx) { - if (is_destroyed()) { - RD_LOGI("Raft repl dev is destroyed, ignore cp flush"); - return; - } - auto const lsn = ctx->cp_lsn; auto const clsn = ctx->compacted_to_lsn; auto const dsn = ctx->last_applied_dsn; @@ -1437,6 +1432,10 @@ void RaftReplDev::cp_flush(CP* cp, cshared< ReplDevCPContext > ctx) { } std::unique_lock lg{m_sb_mtx}; + if (is_destroyed()) { + RD_LOGD("Raft repl dev is destroyed, ignore cp flush"); + return; + } m_rd_sb->compact_lsn = clsn; // dc_lsn is also flushed in flush_durable_commit_lsn() // we need to take a max to avoid rolling back.