From 9d0463438755224c61f66a89e81690f11ef2c9be Mon Sep 17 00:00:00 2001 From: Niko Nastonen Date: Tue, 4 Aug 2026 08:20:53 -0700 Subject: [PATCH 1/2] SDSTOR-24093: Duplicate inserts in wb_cache --- src/lib/index/wb_cache.cpp | 20 +++++++++++++++----- src/lib/index/wb_cache.hpp | 1 + 2 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/lib/index/wb_cache.cpp b/src/lib/index/wb_cache.cpp index afa1cc958..972378a09 100644 --- a/src/lib/index/wb_cache.cpp +++ b/src/lib/index/wb_cache.cpp @@ -565,6 +565,7 @@ void IndexWBCache::recover(sisl::byte_view sb) { auto cpg = cp_mgr().cp_guard(); auto icp_ctx = r_cast< IndexCPContext* >(cpg.context(cp_consumer_t::INDEX_SVC)); std::map< BlkId, IndexBufferPtr > bufs = icp_ctx->recover(std::move(sb)); + bool allocator_state_changed{false}; LOGINFOMOD(wbcache, "Detected unclean shutdown, prior cp={} had to flush {} nodes, recovering... ", icp_ctx->id(), bufs.size()); @@ -657,7 +658,8 @@ void IndexWBCache::recover(sisl::byte_view sb) { was_node_committed(buf->m_up_buffer)); buf->m_node_freed = false; r_cast< persistent_hdr_t* >(buf->m_bytes)->node_deleted = false; - m_vdev->commit_blk(buf->m_blkid); + auto alloc_status = m_vdev->commit_blk(buf->m_blkid); + allocator_state_changed |= (alloc_status == BlkAllocStatus::SUCCESS); if (buf->m_node_level) { potential_parent_recovered_bufs.insert(buf); } prune_from_up_buffer(buf); } @@ -668,7 +670,8 @@ void IndexWBCache::recover(sisl::byte_view sb) { // Both current and up buffer is committed, we can safely commit the current block LOGTRACEMOD(wbcache, "New buffer {} and the up buffer {} are committed", buf->to_string(), buf->m_up_buffer->to_string()); - m_vdev->commit_blk(buf->m_blkid); + auto alloc_status = m_vdev->commit_blk(buf->m_blkid); + allocator_state_changed |= (alloc_status == BlkAllocStatus::SUCCESS); pending_bufs.push_back(buf->m_up_buffer); } else { // Up buffer is not committed, we need to repair it first @@ -766,6 +769,8 @@ void IndexWBCache::recover(sisl::byte_view sb) { } } } + + m_force_vdev_flush.store(allocator_state_changed, std::memory_order_release); m_in_recovery = false; m_vdev->recovery_completed(); } @@ -847,9 +852,14 @@ folly::Future< bool > IndexWBCache::async_cp_flush(IndexCPContext* cp_ctx) { // cp_ctx->to_string_dot(filename); // #endif if (!cp_ctx->any_dirty_buffers()) { - if (cp_ctx->id() == 0) { - // For the first CP, we need to flush the journal buffer to the meta blk - LOGINFO("First time boot cp, we shall flush the vdev to ensure all cp information is created"); + // For the first CP, we need to flush the journal buffer to the meta blk. + // Also after recovery, bitmap could be fixed with no dirty index buffers. + const bool force_vdev_flush = + (cp_ctx->id() == 0) || + m_force_vdev_flush.exchange(false, std::memory_order_acq_rel); + + if (force_vdev_flush) { + LOGINFO("Flush the vdev to ensure all cp information is created"); m_vdev->cp_flush(cp_ctx); } else { CP_PERIODIC_LOG(DEBUG, unmove(cp_ctx->id()), "Btree does not have any dirty buffers to flush"); diff --git a/src/lib/index/wb_cache.hpp b/src/lib/index/wb_cache.hpp index bf04dbc67..0c64b104e 100644 --- a/src/lib/index/wb_cache.hpp +++ b/src/lib/index/wb_cache.hpp @@ -42,6 +42,7 @@ class IndexWBCache : public IndexWBCacheBase { void* m_meta_blk; bool m_in_recovery{false}; std::unordered_set< uint32_t > m_updated_ordinals; + std::atomic m_force_vdev_flush{false}; public: IndexWBCache(const std::shared_ptr< VirtualDev >& vdev, std::pair< meta_blk*, sisl::byte_view > sb, From 0c2ee4b9202ffe2e2bcc2e0609eb75f166c8c98c Mon Sep 17 00:00:00 2001 From: Niko Nastonen Date: Mon, 10 Aug 2026 02:50:13 -0700 Subject: [PATCH 2/2] Addressed Yao's comments --- conanfile.py | 2 +- src/lib/index/wb_cache.cpp | 25 +++++++------------------ src/lib/index/wb_cache.hpp | 1 - 3 files changed, 8 insertions(+), 20 deletions(-) diff --git a/conanfile.py b/conanfile.py index 17eec5915..545d174c0 100644 --- a/conanfile.py +++ b/conanfile.py @@ -9,7 +9,7 @@ class HomestoreConan(ConanFile): name = "homestore" - version = "7.5.15" + version = "7.5.16" homepage = "https://github.com/eBay/Homestore" description = "HomeStore Storage Engine" diff --git a/src/lib/index/wb_cache.cpp b/src/lib/index/wb_cache.cpp index 972378a09..7660b5db3 100644 --- a/src/lib/index/wb_cache.cpp +++ b/src/lib/index/wb_cache.cpp @@ -565,7 +565,6 @@ void IndexWBCache::recover(sisl::byte_view sb) { auto cpg = cp_mgr().cp_guard(); auto icp_ctx = r_cast< IndexCPContext* >(cpg.context(cp_consumer_t::INDEX_SVC)); std::map< BlkId, IndexBufferPtr > bufs = icp_ctx->recover(std::move(sb)); - bool allocator_state_changed{false}; LOGINFOMOD(wbcache, "Detected unclean shutdown, prior cp={} had to flush {} nodes, recovering... ", icp_ctx->id(), bufs.size()); @@ -659,7 +658,7 @@ void IndexWBCache::recover(sisl::byte_view sb) { buf->m_node_freed = false; r_cast< persistent_hdr_t* >(buf->m_bytes)->node_deleted = false; auto alloc_status = m_vdev->commit_blk(buf->m_blkid); - allocator_state_changed |= (alloc_status == BlkAllocStatus::SUCCESS); + HS_REL_ASSERT_EQ(alloc_status, BlkAllocStatus::SUCCESS, "Unsuccessful commit_blk() in recover()"); if (buf->m_node_level) { potential_parent_recovered_bufs.insert(buf); } prune_from_up_buffer(buf); } @@ -671,7 +670,7 @@ void IndexWBCache::recover(sisl::byte_view sb) { LOGTRACEMOD(wbcache, "New buffer {} and the up buffer {} are committed", buf->to_string(), buf->m_up_buffer->to_string()); auto alloc_status = m_vdev->commit_blk(buf->m_blkid); - allocator_state_changed |= (alloc_status == BlkAllocStatus::SUCCESS); + HS_REL_ASSERT_EQ(alloc_status, BlkAllocStatus::SUCCESS, "Unsuccessful commit_blk() in recover()"); pending_bufs.push_back(buf->m_up_buffer); } else { // Up buffer is not committed, we need to repair it first @@ -769,8 +768,6 @@ void IndexWBCache::recover(sisl::byte_view sb) { } } } - - m_force_vdev_flush.store(allocator_state_changed, std::memory_order_release); m_in_recovery = false; m_vdev->recovery_completed(); } @@ -852,19 +849,11 @@ folly::Future< bool > IndexWBCache::async_cp_flush(IndexCPContext* cp_ctx) { // cp_ctx->to_string_dot(filename); // #endif if (!cp_ctx->any_dirty_buffers()) { - // For the first CP, we need to flush the journal buffer to the meta blk. - // Also after recovery, bitmap could be fixed with no dirty index buffers. - const bool force_vdev_flush = - (cp_ctx->id() == 0) || - m_force_vdev_flush.exchange(false, std::memory_order_acq_rel); - - if (force_vdev_flush) { - LOGINFO("Flush the vdev to ensure all cp information is created"); - m_vdev->cp_flush(cp_ctx); - } else { - CP_PERIODIC_LOG(DEBUG, unmove(cp_ctx->id()), "Btree does not have any dirty buffers to flush"); - } - cp_ctx->complete(true); + LOGINFO("Flush the vdev to ensure all cp information is created"); + // Always try to flush, will be a no-op when not needed + m_vdev->cp_flush(cp_ctx); + + cp_ctx->complete(true); return folly::makeFuture< bool >(true); // nothing to flush } diff --git a/src/lib/index/wb_cache.hpp b/src/lib/index/wb_cache.hpp index 0c64b104e..bf04dbc67 100644 --- a/src/lib/index/wb_cache.hpp +++ b/src/lib/index/wb_cache.hpp @@ -42,7 +42,6 @@ class IndexWBCache : public IndexWBCacheBase { void* m_meta_blk; bool m_in_recovery{false}; std::unordered_set< uint32_t > m_updated_ordinals; - std::atomic m_force_vdev_flush{false}; public: IndexWBCache(const std::shared_ptr< VirtualDev >& vdev, std::pair< meta_blk*, sisl::byte_view > sb,