From 0a878d4394b97348121d120fd6ac7f0365c5f265 Mon Sep 17 00:00:00 2001 From: Yaming Kuang <1477567+yamingk@users.noreply.github.com> Date: Fri, 29 Sep 2023 15:39:58 -0700 Subject: [PATCH 1/2] feed num_chunks to create_vdev (#195) * feed num_chunks to create_vdev * inc con ver --- src/include/homestore/blkdata_service.hpp | 2 +- src/include/homestore/index_service.hpp | 2 +- src/include/homestore/logstore_service.hpp | 2 +- src/include/homestore/meta_service.hpp | 2 +- src/lib/blkdata_svc/blkdata_service.cpp | 4 ++-- src/lib/homestore.cpp | 12 ++++++------ src/lib/index/index_service.cpp | 4 ++-- src/lib/logstore/log_store_service.cpp | 5 +++-- src/lib/meta/meta_blk_service.cpp | 4 ++-- 9 files changed, 19 insertions(+), 18 deletions(-) diff --git a/src/include/homestore/blkdata_service.hpp b/src/include/homestore/blkdata_service.hpp index faac2e426..00e569e1f 100644 --- a/src/include/homestore/blkdata_service.hpp +++ b/src/include/homestore/blkdata_service.hpp @@ -47,7 +47,7 @@ class BlkDataService { * @param size : size of this vdev */ void create_vdev(uint64_t size, uint32_t blk_size, blk_allocator_type_t alloc_type, - chunk_selector_type_t chunk_sel_type); + chunk_selector_type_t chunk_sel_type, uint32_t num_chunks); /** * @brief : called during recovery to open existing vdev for data service diff --git a/src/include/homestore/index_service.hpp b/src/include/homestore/index_service.hpp index b4f551a8e..466c52cdb 100644 --- a/src/include/homestore/index_service.hpp +++ b/src/include/homestore/index_service.hpp @@ -51,7 +51,7 @@ class IndexService { IndexService(std::unique_ptr< IndexServiceCallbacks > cbs); // Creates the vdev that is needed to initialize the device - void create_vdev(uint64_t size); + void create_vdev(uint64_t size, uint32_t num_chunks); // Open the existing vdev which is represnted by the vdev_info_block shared< VirtualDev > open_vdev(const vdev_info& vb, bool load_existing); diff --git a/src/include/homestore/logstore_service.hpp b/src/include/homestore/logstore_service.hpp index 3f1d62958..506eff5ba 100644 --- a/src/include/homestore/logstore_service.hpp +++ b/src/include/homestore/logstore_service.hpp @@ -135,7 +135,7 @@ class LogStoreService { void device_truncate(const device_truncate_cb_t& cb = nullptr, const bool wait_till_done = false, const bool dry_run = false); - folly::Future< std::error_code > create_vdev(uint64_t size, logstore_family_id_t family); + folly::Future< std::error_code > create_vdev(uint64_t size, logstore_family_id_t family, uint32_t num_chunks); shared< VirtualDev > open_vdev(const vdev_info& vinfo, logstore_family_id_t family, bool load_existing); shared< JournalVirtualDev > get_vdev(logstore_family_id_t family) const { return (family == DATA_LOG_FAMILY_IDX) ? m_data_logdev_vdev : m_ctrl_logdev_vdev; diff --git a/src/include/homestore/meta_service.hpp b/src/include/homestore/meta_service.hpp index b6b5c9b4c..cb2c65e25 100644 --- a/src/include/homestore/meta_service.hpp +++ b/src/include/homestore/meta_service.hpp @@ -98,7 +98,7 @@ class MetaBlkService { ~MetaBlkService() = default; // Creates the vdev that is needed to initialize the device - void create_vdev(uint64_t size); + void create_vdev(uint64_t size, uint32_t num_chunks); // Open the existing vdev which is represented by the vdev_info shared< VirtualDev > open_vdev(const vdev_info& vinfo, bool load_existing); diff --git a/src/lib/blkdata_svc/blkdata_service.cpp b/src/lib/blkdata_svc/blkdata_service.cpp index 6a596eb54..8a260ecdb 100644 --- a/src/lib/blkdata_svc/blkdata_service.cpp +++ b/src/lib/blkdata_svc/blkdata_service.cpp @@ -38,7 +38,7 @@ BlkDataService::~BlkDataService() = default; // first-time boot path void BlkDataService::create_vdev(uint64_t size, uint32_t blk_size, blk_allocator_type_t alloc_type, - chunk_selector_type_t chunk_sel_type) { + chunk_selector_type_t chunk_sel_type, uint32_t num_chunks) { hs_vdev_context vdev_ctx; vdev_ctx.type = hs_vdev_type_t::DATA_VDEV; @@ -46,7 +46,7 @@ void BlkDataService::create_vdev(uint64_t size, uint32_t blk_size, blk_allocator m_vdev = hs()->device_mgr()->create_vdev(vdev_parameters{.vdev_name = "blkdata", .vdev_size = size, - .num_chunks = 1, + .num_chunks = num_chunks, .blk_size = blk_size, .dev_type = HSDevType::Data, .alloc_type = alloc_type, diff --git a/src/lib/homestore.cpp b/src/lib/homestore.cpp index 92efbe652..699b61927 100644 --- a/src/lib/homestore.cpp +++ b/src/lib/homestore.cpp @@ -153,21 +153,21 @@ void HomeStore::format_and_start(std::map< uint32_t, hs_format_params >&& format if (fparams.size_pct == 0) { continue; } if ((svc_type & HS_SERVICE::META) && has_meta_service()) { - m_meta_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Fast)); + m_meta_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Fast), fparams.num_chunks); } else if ((svc_type & HS_SERVICE::LOG_REPLICATED) && has_log_service()) { futs.emplace_back(m_log_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Fast), - LogStoreService::DATA_LOG_FAMILY_IDX)); + LogStoreService::DATA_LOG_FAMILY_IDX, fparams.num_chunks)); } else if ((svc_type & HS_SERVICE::LOG_LOCAL) && has_log_service()) { futs.emplace_back(m_log_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Fast), - LogStoreService::CTRL_LOG_FAMILY_IDX)); + LogStoreService::CTRL_LOG_FAMILY_IDX, fparams.num_chunks)); } else if ((svc_type & HS_SERVICE::DATA) && has_data_service()) { m_data_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Data), fparams.block_size, - fparams.alloc_type, fparams.chunk_sel_type); + fparams.alloc_type, fparams.chunk_sel_type, fparams.num_chunks); } else if ((svc_type & HS_SERVICE::INDEX) && has_index_service()) { - m_index_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Fast)); + m_index_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Fast), fparams.num_chunks); } else if ((svc_type & HS_SERVICE::REPLICATION) && has_repl_data_service()) { m_data_service->create_vdev(pct_to_size(fparams.size_pct, HSDevType::Data), fparams.block_size, - fparams.alloc_type, fparams.chunk_sel_type); + fparams.alloc_type, fparams.chunk_sel_type, fparams.num_chunks); } } diff --git a/src/lib/index/index_service.cpp b/src/lib/index/index_service.cpp index 4769a6d67..602026114 100644 --- a/src/lib/index/index_service.cpp +++ b/src/lib/index/index_service.cpp @@ -35,14 +35,14 @@ IndexService::IndexService(std::unique_ptr< IndexServiceCallbacks > cbs) : m_svc nullptr); } -void IndexService::create_vdev(uint64_t size) { +void IndexService::create_vdev(uint64_t size, uint32_t num_chunks) { auto const atomic_page_size = hs()->device_mgr()->atomic_page_size(HSDevType::Fast); hs_vdev_context vdev_ctx; vdev_ctx.type = hs_vdev_type_t::INDEX_VDEV; hs()->device_mgr()->create_vdev(vdev_parameters{.vdev_name = "index", .vdev_size = size, - .num_chunks = 1, + .num_chunks = num_chunks, .blk_size = atomic_page_size, .dev_type = HSDevType::Fast, .alloc_type = blk_allocator_type_t::fixed, diff --git a/src/lib/logstore/log_store_service.cpp b/src/lib/logstore/log_store_service.cpp index 02f223131..931e11b44 100644 --- a/src/lib/logstore/log_store_service.cpp +++ b/src/lib/logstore/log_store_service.cpp @@ -42,7 +42,8 @@ LogStoreService::LogStoreService() : m_logstore_families{std::make_unique< LogStoreFamily >(DATA_LOG_FAMILY_IDX), std::make_unique< LogStoreFamily >(CTRL_LOG_FAMILY_IDX)} {} -folly::Future< std::error_code > LogStoreService::create_vdev(uint64_t size, logstore_family_id_t family) { +folly::Future< std::error_code > LogStoreService::create_vdev(uint64_t size, logstore_family_id_t family, + uint32_t num_chunks) { const auto atomic_page_size = hs()->device_mgr()->atomic_page_size(HSDevType::Fast); hs_vdev_context hs_ctx; @@ -62,7 +63,7 @@ folly::Future< std::error_code > LogStoreService::create_vdev(uint64_t size, log auto vdev = hs()->device_mgr()->create_vdev(vdev_parameters{.vdev_name = name, .vdev_size = size, - .num_chunks = 1, + .num_chunks = num_chunks, .blk_size = atomic_page_size, .dev_type = HSDevType::Fast, .alloc_type = blk_allocator_type_t::none, diff --git a/src/lib/meta/meta_blk_service.cpp b/src/lib/meta/meta_blk_service.cpp index 85b39fef9..510033dfa 100644 --- a/src/lib/meta/meta_blk_service.cpp +++ b/src/lib/meta/meta_blk_service.cpp @@ -47,7 +47,7 @@ MetaBlkService& meta_service() { return hs()->meta_service(); } MetaBlkService::MetaBlkService(const char* name) : m_metrics{name} { m_last_mblk_id = std::make_unique< BlkId >(); } -void MetaBlkService::create_vdev(uint64_t size) { +void MetaBlkService::create_vdev(uint64_t size, uint32_t num_chunks) { const auto phys_page_size = hs()->device_mgr()->optimal_page_size(HSDevType::Fast); meta_vdev_context meta_ctx; @@ -55,7 +55,7 @@ void MetaBlkService::create_vdev(uint64_t size) { hs()->device_mgr()->create_vdev(vdev_parameters{.vdev_name = "meta", .vdev_size = size, - .num_chunks = 1, + .num_chunks = num_chunks, .blk_size = phys_page_size, .dev_type = HSDevType::Fast, .alloc_type = blk_allocator_type_t::varsize, From 119e67ef2973d1e0264c4c38feb905e1837cbec5 Mon Sep 17 00:00:00 2001 From: Sanal Date: Wed, 4 Oct 2023 17:12:59 -0700 Subject: [PATCH 2/2] Adjust num chunks requested in device manager. (#197) Fix timer related asan issue. Disable index btree remove api's as its conflict with cp flush. --- conanfile.py | 2 +- src/lib/checkpoint/cp_mgr.cpp | 7 ++++--- src/lib/device/device_manager.cpp | 13 +++++++++++++ src/lib/device/physical_dev.cpp | 2 +- src/lib/logstore/log_dev.cpp | 5 +++-- src/tests/test_device_manager.cpp | 2 +- src/tests/test_index_btree.cpp | 2 ++ 7 files changed, 25 insertions(+), 8 deletions(-) diff --git a/conanfile.py b/conanfile.py index c0bf4d834..2ca2c687a 100644 --- a/conanfile.py +++ b/conanfile.py @@ -5,7 +5,7 @@ class HomestoreConan(ConanFile): name = "homestore" - version = "4.5.1" + version = "4.5.2" homepage = "https://github.com/eBay/Homestore" description = "HomeStore Storage Engine" diff --git a/src/lib/checkpoint/cp_mgr.cpp b/src/lib/checkpoint/cp_mgr.cpp index 55e43000e..944c90222 100644 --- a/src/lib/checkpoint/cp_mgr.cpp +++ b/src/lib/checkpoint/cp_mgr.cpp @@ -71,6 +71,10 @@ void CPManager::create_first_cp() { } void CPManager::shutdown() { + LOGINFO("Stopping cp timer"); + iomanager.cancel_timer(m_cp_timer_hdl, true); + m_cp_timer_hdl = iomgr::null_timer_handle; + auto cp = get_cur_cp(); delete (cp); rcu_xchg_pointer(&m_cur_cp, nullptr); @@ -79,9 +83,6 @@ void CPManager::shutdown() { m_wd_cp->stop(); m_wd_cp.reset(); } - - iomanager.cancel_timer(m_cp_timer_hdl, true); - m_cp_timer_hdl = iomgr::null_timer_handle; } void CPManager::register_consumer(cp_consumer_t consumer_id, std::unique_ptr< CPCallbacks > callbacks) { diff --git a/src/lib/device/device_manager.cpp b/src/lib/device/device_manager.cpp index 1b06d2fd9..7e0dd40ec 100644 --- a/src/lib/device/device_manager.cpp +++ b/src/lib/device/device_manager.cpp @@ -211,6 +211,19 @@ shared< VirtualDev > DeviceManager::create_vdev(vdev_parameters&& vparam) { LOGINFO("{} Virtual device is attempted to be created with size={}, it needs to be rounded to new_size={}", vparam.vdev_name, in_bytes(input_vdev_size), in_bytes(vparam.vdev_size)); } + + // Adjust the maximum number chunks requested. + uint32_t max_num_chunks = 0; + for (const auto& d : m_dev_infos) { + max_num_chunks += hs_super_blk::max_chunks_in_pdev(d); + } + auto input_num_chunks = vparam.num_chunks; + vparam.num_chunks = std::min(vparam.num_chunks, max_num_chunks); + if (input_num_chunks != vparam.num_chunks) { + LOGINFO("{} Virtual device is attempted to be created with num_chunks={}, it needs to be adjust to " + "new_num_chunks={}", + vparam.vdev_name, in_bytes(input_num_chunks), in_bytes(vparam.num_chunks)); + } uint32_t chunk_size = vparam.vdev_size / vparam.num_chunks; LOGINFO( diff --git a/src/lib/device/physical_dev.cpp b/src/lib/device/physical_dev.cpp index 122f735d8..33f243824 100644 --- a/src/lib/device/physical_dev.cpp +++ b/src/lib/device/physical_dev.cpp @@ -427,7 +427,7 @@ void PhysicalDev::free_chunk_info(chunk_info* cinfo) { } ChunkInterval PhysicalDev::find_next_chunk_area(uint64_t size) const { - auto ins_ival = ChunkInterval::right_open(data_start_offset(), size); + auto ins_ival = ChunkInterval::right_open(data_start_offset(), data_start_offset() + size); for (auto& exist_ival : m_chunk_data_area) { if (ins_ival.upper() <= exist_ival.lower()) { break; } ins_ival = ChunkInterval::right_open(exist_ival.upper(), exist_ival.upper() + size); diff --git a/src/lib/logstore/log_dev.cpp b/src/lib/logstore/log_dev.cpp index 3dae46bbd..b8ea1e2ef 100644 --- a/src/lib/logstore/log_dev.cpp +++ b/src/lib/logstore/log_dev.cpp @@ -121,6 +121,9 @@ void LogDev::stop() { m_block_flush_q_cv.wait(lk, [&] { return m_stopped; }); } + // cancel the timer + iomanager.cancel_timer(m_flush_timer_hdl, true); + m_log_records = nullptr; m_logdev_meta.reset(); m_log_idx.store(0); @@ -137,8 +140,6 @@ void LogDev::stop() { } THIS_LOGDEV_LOG(INFO, "LogDev stopped successfully"); - // cancel the timer - iomanager.cancel_timer(m_flush_timer_hdl, true); m_hs.reset(); } diff --git a/src/tests/test_device_manager.cpp b/src/tests/test_device_manager.cpp index d54e31a65..8b1363688 100644 --- a/src/tests/test_device_manager.cpp +++ b/src/tests/test_device_manager.cpp @@ -140,7 +140,7 @@ TEST_F(DeviceMgrTest, StripedVDevCreation) { avail_size += pdev->data_size(); } - uint32_t size_pct = 4; + uint32_t size_pct = 2; uint64_t remain_size = avail_size; LOGINFO("Step 1: Creating {} vdevs with combined size as {}", in_bytes(avail_size)); diff --git a/src/tests/test_index_btree.cpp b/src/tests/test_index_btree.cpp index 4463ebbd4..5de8b3126 100644 --- a/src/tests/test_index_btree.cpp +++ b/src/tests/test_index_btree.cpp @@ -412,6 +412,7 @@ TYPED_TEST(BtreeTest, RandomInsert) { this->get_all_validate(); } +#if 0 TYPED_TEST(BtreeTest, SequentialRemove) { LOGINFO("SequentialRemove test start"); // Forward sequential insert @@ -466,6 +467,7 @@ TYPED_TEST(BtreeTest, RandomRemove) { } this->get_all_validate(); } +#endif TYPED_TEST(BtreeTest, RangeUpdate) { LOGINFO("RangeUpdate test start");