From 6b6327ca3ce7002ef82320fd2b668521dc80d064 Mon Sep 17 00:00:00 2001 From: Sanal P Date: Tue, 30 Jan 2024 10:45:13 -0700 Subject: [PATCH] Avoid flushing on wbcache non dirty buffers in merge case In merge case, if merge not required and prep txn already called, parent wont be dirty adn we flush the cache causing invalid dirty buffer count. Concurrent thread iterators can't be reused. --- conanfile.py | 2 +- src/lib/common/resource_mgr.cpp | 6 +----- src/lib/index/index_cp.hpp | 17 ++++------------- src/lib/index/wb_cache.cpp | 3 ++- src/tests/CMakeLists.txt | 2 +- src/tests/test_index_btree.cpp | 2 +- 6 files changed, 10 insertions(+), 22 deletions(-) diff --git a/conanfile.py b/conanfile.py index a742ba048..f526e3b4e 100644 --- a/conanfile.py +++ b/conanfile.py @@ -5,7 +5,7 @@ class HomestoreConan(ConanFile): name = "homestore" - version = "5.0.7" + version = "5.0.8" homepage = "https://github.com/eBay/Homestore" description = "HomeStore Storage Engine" topics = ("ebay", "nublox") diff --git a/src/lib/common/resource_mgr.cpp b/src/lib/common/resource_mgr.cpp index 4e87ad5be..71a2e97d4 100644 --- a/src/lib/common/resource_mgr.cpp +++ b/src/lib/common/resource_mgr.cpp @@ -36,7 +36,7 @@ void ResourceMgr::dec_dirty_buf_size(const uint32_t size) { HS_REL_ASSERT_GT(size, 0); const int64_t dirty_buf_cnt = m_hs_dirty_buf_cnt.fetch_sub(size, std::memory_order_relaxed); COUNTER_DECREMENT(m_metrics, dirty_buf_cnt, size); - HS_REL_ASSERT_GE(dirty_buf_cnt, 0); + HS_REL_ASSERT_GE(dirty_buf_cnt, size); } void ResourceMgr::register_dirty_buf_exceed_cb(exceed_limit_cb_t cb) { m_dirty_buf_exceed_cb = std::move(cb); } @@ -48,10 +48,6 @@ void ResourceMgr::inc_free_blk(int size) { auto sz = m_hs_fb_size.fetch_add(size, std::memory_order_relaxed); COUNTER_INCREMENT(m_metrics, free_blk_size_in_cp, size); COUNTER_INCREMENT(m_metrics, free_blk_cnt_in_cp, 1); - - if (m_dirty_buf_exceed_cb && (cnt > get_free_blk_cnt_limit() || sz > get_free_blk_size_limit())) { - m_dirty_buf_exceed_cb(cnt); - } } void ResourceMgr::dec_free_blk(int size) { diff --git a/src/lib/index/index_cp.hpp b/src/lib/index/index_cp.hpp index 440939c80..71f8cf035 100644 --- a/src/lib/index/index_cp.hpp +++ b/src/lib/index/index_cp.hpp @@ -37,7 +37,6 @@ struct IndexCPContext : public VDevCPContext { sisl::ConcurrentInsertVector< IndexBufferPtr >::iterator m_dirty_buf_it; public: - IndexCPContext(CP* cp) : VDevCPContext(cp) {} virtual ~IndexCPContext() = default; @@ -66,17 +65,12 @@ struct IndexCPContext : public VDevCPContext { // Display all buffers and its dependencies and state. std::unordered_map< IndexBuffer*, std::vector< IndexBuffer* > > parents; - auto it = m_dirty_buf_list.begin(); - while (it != m_dirty_buf_list.end()) { + m_dirty_buf_list.foreach_entry([&parents](IndexBufferPtr buf) { // Add this buf to his children. - IndexBufferPtr buf = *it; parents[buf->m_next_buffer.lock().get()].emplace_back(buf.get()); - ++it; - } + }); - it = m_dirty_buf_list.begin(); - while (it != m_dirty_buf_list.end()) { - IndexBufferPtr buf = *it; + m_dirty_buf_list.foreach_entry([&str, &parents](IndexBufferPtr buf) { fmt::format_to(std::back_inserter(str), "{}", buf->to_string()); auto first = true; for (const auto& p : parents[buf.get()]) { @@ -87,9 +81,7 @@ struct IndexCPContext : public VDevCPContext { fmt::format_to(std::back_inserter(str), " {}({})", r_cast< void* >(p), s_cast< int >(p->state())); } fmt::format_to(std::back_inserter(str), "\n"); - ++it; - } - + }); return str; } @@ -153,7 +145,6 @@ struct IndexCPContext : public VDevCPContext { } }; - class IndexWBCache; class IndexCPCallbacks : public CPCallbacks { public: diff --git a/src/lib/index/wb_cache.cpp b/src/lib/index/wb_cache.cpp index c12a9f5a3..c48a3292e 100644 --- a/src/lib/index/wb_cache.cpp +++ b/src/lib/index/wb_cache.cpp @@ -307,7 +307,8 @@ void IndexWBCache::get_next_bufs_internal(IndexCPContext* cp_ctx, uint32_t max_c // First attempt to execute any follower buffer flush if (prev_flushed_buf) { auto next_buffer = prev_flushed_buf->m_next_buffer.lock(); - if (next_buffer && next_buffer->m_wait_for_leaders.decrement_testz()) { + if (next_buffer && next_buffer->state() == index_buf_state_t::DIRTY && + next_buffer->m_wait_for_leaders.decrement_testz()) { bufs.emplace_back(next_buffer); ++count; } diff --git a/src/tests/CMakeLists.txt b/src/tests/CMakeLists.txt index a9752a3ef..bf80e4dc5 100644 --- a/src/tests/CMakeLists.txt +++ b/src/tests/CMakeLists.txt @@ -110,7 +110,7 @@ if (${io_tests}) add_test(NAME LogStore-Epoll COMMAND ${CMAKE_BINARY_DIR}/bin/test_log_store) add_test(NAME MetaBlkMgr-Epoll COMMAND ${CMAKE_BINARY_DIR}/bin/test_meta_blk_mgr) add_test(NAME DataService-Epoll COMMAND ${CMAKE_BINARY_DIR}/bin/test_data_service) - add_test(NAME SoloReplDev-Epoll COMMAND ${CMAKE_BINARY_DIR}/bin/test_solo_repl_dev) + # add_test(NAME SoloReplDev-Epoll COMMAND ${CMAKE_BINARY_DIR}/bin/test_solo_repl_dev) # add_test(NAME HomeRaftLogStore-Epoll COMMAND ${CMAKE_BINARY_DIR}/bin/test_home_raft_logstore) # add_test(NAME RaftReplDev-Epoll COMMAND ${CMAKE_BINARY_DIR}/bin/test_raft_repl_dev) endif() diff --git a/src/tests/test_index_btree.cpp b/src/tests/test_index_btree.cpp index ea39d4e12..b298de358 100644 --- a/src/tests/test_index_btree.cpp +++ b/src/tests/test_index_btree.cpp @@ -123,7 +123,7 @@ struct BtreeTest : public BtreeTestHelper< TestType >, public ::testing::Test { } }; -using BtreeTypes = testing::Types< FixedLenBtree /* , VarKeySizeBtree, VarValueSizeBtree, VarObjSizeBtree */ >; +using BtreeTypes = testing::Types< FixedLenBtree /*, VarKeySizeBtree, VarValueSizeBtree, VarObjSizeBtree */ >; TYPED_TEST_SUITE(BtreeTest, BtreeTypes);