diff --git a/.github/workflows/conan_build.yml b/.github/workflows/conan_build.yml index dd3da6e7..329549c9 100644 --- a/.github/workflows/conan_build.yml +++ b/.github/workflows/conan_build.yml @@ -5,9 +5,11 @@ on: push: branches: - stable/v4.x + - dev/refactor_create_seal_shard pull_request: branches: - stable/v4.x + - dev/refactor_create_seal_shard jobs: Build: diff --git a/.github/workflows/version_change_check.yml b/.github/workflows/version_change_check.yml index 95e55396..41707fd0 100644 --- a/.github/workflows/version_change_check.yml +++ b/.github/workflows/version_change_check.yml @@ -5,7 +5,7 @@ on: types: [opened, edited, synchronize] branches: - stable/v4.x - + - dev/refactor_create_seal_shard jobs: check-file: runs-on: ubuntu-latest diff --git a/.jenkins/Jenkinsfile b/.jenkins/Jenkinsfile index eedf3408..041b3da5 100644 --- a/.jenkins/Jenkinsfile +++ b/.jenkins/Jenkinsfile @@ -65,6 +65,9 @@ pipeline { conan create ${BUILD_MISSING} -s:h build_type=RelWithDebInfo -o sisl/*:malloc_impl=tcmalloc ${CONAN_FLAGS} . ; \ " } + post { + failure { script { sleep 3600000 } } + } } stage("Deploy") { @@ -82,7 +85,7 @@ pipeline { expression { (env.BRANCH_NAME == "${TARGET_BRANCH}") } expression { (!"${upstream_triggered}") || ("${upstream_triggered}" == "") } } } - + stages { stage('StorageManager') { steps { diff --git a/conanfile.py b/conanfile.py index 17ec62ba..2fc86598 100644 --- a/conanfile.py +++ b/conanfile.py @@ -10,7 +10,7 @@ class HomeObjectConan(ConanFile): name = "homeobject" - version = "4.1.18" + version = "4.2.3" homepage = "https://github.com/eBay/HomeObject" description = "Blob Store built on HomeStore" diff --git a/src/include/homeobject/shard_manager.hpp b/src/include/homeobject/shard_manager.hpp index 5e7a6dd6..c9a0f282 100644 --- a/src/include/homeobject/shard_manager.hpp +++ b/src/include/homeobject/shard_manager.hpp @@ -35,13 +35,14 @@ struct ShardInfo { shard_id_t id; pg_id_t placement_group; State state; - uint64_t lsn; // created_lsn + uint64_t create_lsn; uint64_t created_time; uint64_t last_modified_time; uint64_t available_capacity_bytes; uint64_t total_capacity_bytes; std::optional< peer_id_t > current_leader{std::nullopt}; uint8_t meta[meta_length]{}; + uint64_t sealed_lsn{INT64_MAX}; // appended after all v2 fields for forward compat auto operator<=>(ShardInfo const& rhs) const { return id <=> rhs.id; } auto operator==(ShardInfo const& rhs) const { return id == rhs.id; } @@ -57,7 +58,8 @@ class ShardManager : public Manager< ShardError > { virtual AsyncResult< ShardInfo > get_shard(shard_id_t id, trace_id_t tid = 0) const = 0; virtual AsyncResult< InfoList > list_shards(pg_id_t id, trace_id_t tid = 0) const = 0; - virtual AsyncResult< ShardInfo > create_shard(pg_id_t pg_owner, uint64_t size_bytes, std::string meta, trace_id_t tid = 0) = 0; + virtual AsyncResult< ShardInfo > create_shard(pg_id_t pg_owner, uint64_t size_bytes, std::string meta, + trace_id_t tid = 0) = 0; virtual AsyncResult< ShardInfo > seal_shard(shard_id_t id, trace_id_t tid = 0) = 0; }; diff --git a/src/lib/homestore_backend/CMakeLists.txt b/src/lib/homestore_backend/CMakeLists.txt index 7fd3d6fe..4df2b8a7 100644 --- a/src/lib/homestore_backend/CMakeLists.txt +++ b/src/lib/homestore_backend/CMakeLists.txt @@ -45,6 +45,7 @@ settings_gen_cpp( hs_backend_config.fbs resync_pg_data.fbs resync_shard_data.fbs + legacy_resync_shard_data.fbs resync_blob_data.fbs ) @@ -55,28 +56,28 @@ add_subdirectory(tests) add_executable(homestore_test_pg) target_sources(homestore_test_pg PRIVATE $) target_link_libraries(homestore_test_pg PUBLIC homeobject_homestore ${COMMON_TEST_DEPS}) -add_test(NAME HomestoreTestPg COMMAND homestore_test_pg -csv error --executor immediate --config_path ./ +add_test(NAME HomestoreTestPg COMMAND homestore_test_pg --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance:0 --override_config homestore_config.consensus.max_grpc_message_size:138412032) add_executable(homestore_test_shard) target_sources(homestore_test_shard PRIVATE $) target_link_libraries(homestore_test_shard PUBLIC homeobject_homestore ${COMMON_TEST_DEPS}) -add_test(NAME HomestoreTestShard COMMAND homestore_test_shard -csv error --executor immediate --config_path ./ +add_test(NAME HomestoreTestShard COMMAND homestore_test_shard --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance:0 --override_config homestore_config.consensus.max_grpc_message_size:138412032) add_executable(homestore_test_blob) target_sources(homestore_test_blob PRIVATE $) target_link_libraries(homestore_test_blob PUBLIC homeobject_homestore ${COMMON_TEST_DEPS}) -add_test(NAME HomestoreTestBlob COMMAND homestore_test_blob -csv error --executor immediate --config_path ./ +add_test(NAME HomestoreTestBlob COMMAND homestore_test_blob --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance:0 --override_config homestore_config.consensus.max_grpc_message_size:138412032) add_executable(homestore_test_misc) target_sources(homestore_test_misc PRIVATE $) target_link_libraries(homestore_test_misc PUBLIC homeobject_homestore ${COMMON_TEST_DEPS}) -add_test(NAME HomestoreTestMisc COMMAND homestore_test_misc -csv error --executor immediate --config_path ./ +add_test(NAME HomestoreTestMisc COMMAND homestore_test_misc --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance=0 --override_config homestore_config.consensus.max_grpc_message_size=138412032 --override_config hs_backend_config.enable_gc=false) @@ -86,7 +87,7 @@ add_executable(homestore_test_dynamic) target_sources(homestore_test_dynamic PRIVATE $) target_link_libraries(homestore_test_dynamic PUBLIC homeobject_homestore ${COMMON_TEST_DEPS}) add_test(NAME HomestoreTestReplaceMember - COMMAND homestore_test_dynamic -csv error --executor immediate --config_path ./ + COMMAND homestore_test_dynamic --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance=0 --override_config homestore_config.generic.repl_dev_cleanup_interval_sec=5 --override_config homestore_config.consensus.max_grpc_message_size=138412032 @@ -95,7 +96,7 @@ add_test(NAME HomestoreTestReplaceMember --gtest_filter=HomeObjectFixture.ReplaceMember) # To test both baseline & incremental resync functionality, we use 13 to minimize the likelihood of it being a divisor of the total LSN (currently 30) add_test(NAME HomestoreTestReplaceMemberWithBaselineResync - COMMAND homestore_test_dynamic -csv error --executor immediate --config_path ./ + COMMAND homestore_test_dynamic --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance=13 --override_config homestore_config.consensus.num_reserved_log_items=13 --override_config homestore_config.resource_limits.raft_logstore_reserve_threshold=13 @@ -106,7 +107,7 @@ add_test(NAME HomestoreTestReplaceMemberWithBaselineResync --override_config homestore_config.consensus.laggy_threshold=2000 --gtest_filter=HomeObjectFixture.ReplaceMember) add_test(NAME HomestoreTestBaselineResync - COMMAND homestore_test_dynamic -csv error --executor immediate --config_path ./ + COMMAND homestore_test_dynamic --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance=13 --override_config homestore_config.consensus.num_reserved_log_items=13 --override_config homestore_config.resource_limits.raft_logstore_reserve_threshold=13 @@ -117,7 +118,7 @@ add_test(NAME HomestoreTestBaselineResync --override_config homestore_config.consensus.laggy_threshold=2000 --gtest_filter=HomeObjectFixture.BaselineResync*) add_test(NAME HomestoreResyncTestWithFollowerRestart - COMMAND homestore_test_dynamic -csv error --executor immediate --config_path ./ + COMMAND homestore_test_dynamic --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance=13 --override_config homestore_config.consensus.num_reserved_log_items=13 --override_config homestore_config.resource_limits.raft_logstore_reserve_threshold=13 @@ -128,7 +129,7 @@ add_test(NAME HomestoreResyncTestWithFollowerRestart --override_config homestore_config.consensus.laggy_threshold=2000 --gtest_filter=HomeObjectFixture.RestartFollower*) add_test(NAME HomestoreResyncTestWithLeaderRestart - COMMAND homestore_test_dynamic -csv error --executor immediate --config_path ./ + COMMAND homestore_test_dynamic --executor immediate --config_path ./ --override_config homestore_config.consensus.snapshot_freq_distance=13 --override_config homestore_config.consensus.num_reserved_log_items=13 --override_config homestore_config.resource_limits.raft_logstore_reserve_threshold=13 @@ -139,7 +140,7 @@ add_test(NAME HomestoreResyncTestWithLeaderRestart --override_config homestore_config.consensus.laggy_threshold=2000 --gtest_filter=HomeObjectFixture.RestartLeader*) #add_test(NAME HomestoreReplaceMemberRollbackTest -# COMMAND homestore_test_dynamic -csv error --executor immediate --config_path ./ +# COMMAND homestore_test_dynamic --executor immediate --config_path ./ # --override_config homestore_config.consensus.snapshot_freq_distance=13 # --override_config homestore_config.consensus.num_reserved_log_items=13 # --override_config homestore_config.resource_limits.raft_logstore_reserve_threshold=13 @@ -152,7 +153,7 @@ add_test(NAME HomestoreResyncTestWithLeaderRestart # GC tests add_test(NAME FetchDataWithOriginatorGC - COMMAND homestore_test_dynamic -csv error --executor immediate --config_path ./ + COMMAND homestore_test_dynamic --executor immediate --config_path ./ --override_config hs_backend_config.enable_gc=true --override_config hs_backend_config.gc_enable_read_verify=true --gtest_filter=HomeObjectFixture.FetchDataWithOriginatorGC) @@ -160,8 +161,33 @@ add_test(NAME FetchDataWithOriginatorGC add_executable(homestore_test_gc) target_sources(homestore_test_gc PRIVATE $) target_link_libraries(homestore_test_gc PUBLIC homeobject_homestore ${COMMON_TEST_DEPS}) -add_test(NAME HomestoreTestGC COMMAND homestore_test_gc -csv error --executor immediate --config_path ./ +add_test(NAME HomestoreTestGC COMMAND homestore_test_gc --executor immediate --config_path ./ --override_config hs_backend_config.enable_gc=true --override_config hs_backend_config.gc_enable_read_verify=true - --override_config hs_backend_config.gc_garbage_rate_threshold=0 - --override_config hs_backend_config.gc_scan_interval_sec=5) + --override_config hs_backend_config.gc_garbage_rate_threshold=0 + --override_config hs_backend_config.gc_scan_interval_sec=5 + --gtest_filter=-HomeObjectFixture.StalePChunkRouteAfterGC:-HomeObjectFixture.StaleBlobRouteAfterSealAndGC) + +# Shard-race repro tests: require chunks_per_pg=1 (forces vchunk reuse after seal) and gc_garbage_rate_threshold=0. +# Run together since they share the same config requirements. +add_test(NAME HomestoreTestGC_ShardRaceTests + COMMAND homestore_test_gc --executor immediate --config_path ./ + --chunks_per_pg 1 + --override_config hs_backend_config.enable_gc=true + --override_config hs_backend_config.gc_garbage_rate_threshold=0 + --override_config hs_backend_config.gc_scan_interval_sec=5 + --gtest_filter=HomeObjectFixture.StalePChunkRouteAfterGC:HomeObjectFixture.StaleBlobRouteAfterSealAndGC) + +# All tests share disk, port, and shared-memory resources, so run them serially to avoid parallel conflicts. +set_tests_properties( + HomestoreTestBlob + HomestoreTestMisc + HomestoreTestReplaceMember + HomestoreTestReplaceMemberWithBaselineResync + HomestoreTestBaselineResync + HomestoreResyncTestWithFollowerRestart + HomestoreResyncTestWithLeaderRestart + FetchDataWithOriginatorGC + HomestoreTestGC + HomestoreTestGC_ShardRaceTests + PROPERTIES RUN_SERIAL TRUE) diff --git a/src/lib/homestore_backend/gc_manager.cpp b/src/lib/homestore_backend/gc_manager.cpp index df7baf34..f0eb9577 100644 --- a/src/lib/homestore_backend/gc_manager.cpp +++ b/src/lib/homestore_backend/gc_manager.cpp @@ -650,27 +650,6 @@ bool GCManager::pdev_gc_actor::replace_blob_index( return true; } -sisl::sg_list GCManager::pdev_gc_actor::generate_shard_super_blk_sg_list(shard_id_t shard_id) { - // TODO: do the buffer check before using it. - auto raw_shard_sb = m_hs_home_object->_get_hs_shard(shard_id); - RELEASE_ASSERT(raw_shard_sb, "can not find shard super blk for shard_id=0x{:x} !!!", shard_id); - - const auto shard_sb = - const_cast< HSHomeObject::HS_Shard* >(d_cast< const HSHomeObject::HS_Shard* >(raw_shard_sb))->sb_.get(); - - auto blk_size = homestore::data_service().get_blk_size(); - auto shard_sb_size = sizeof(HSHomeObject::shard_info_superblk); - auto total_size = sisl::round_up(shard_sb_size, blk_size); - auto shard_sb_buf = iomanager.iobuf_alloc(blk_size, total_size); - - std::memcpy(shard_sb_buf, shard_sb, shard_sb_size); - - sisl::sg_list shard_sb_sgs; - shard_sb_sgs.size = total_size; - shard_sb_sgs.iovs.emplace_back(iovec{.iov_base = shard_sb_buf, .iov_len = total_size}); - return shard_sb_sgs; -} - // note that, when we copy data, there is not create shard or put blob in this chunk, only delete blob might happen. bool GCManager::pdev_gc_actor::copy_valid_data( chunk_id_t move_from_chunk, chunk_id_t move_to_chunk, @@ -723,13 +702,11 @@ bool GCManager::pdev_gc_actor::copy_valid_data( homestore::blk_alloc_hints hints; hints.chunk_id_hint = move_to_chunk; - homestore::MultiBlkId out_blkids; auto pg_index_table = m_hs_home_object->get_hs_pg(pg_id)->index_table_; const auto blk_size = data_service.get_blk_size(); for (const auto& shard_id : shards) { - bool is_last_shard = (shard_id == last_shard_id); std::vector< std::pair< BlobRouteKey, BlobRouteValue > > valid_blob_indexes; auto start_key = BlobRouteKey{BlobRoute{shard_id, std::numeric_limits< uint64_t >::min()}}; auto end_key = BlobRouteKey{BlobRoute{shard_id, std::numeric_limits< uint64_t >::max()}}; @@ -786,12 +763,8 @@ bool GCManager::pdev_gc_actor::copy_valid_data( } if (valid_blob_indexes.empty()) { - GCLOGD(task_id, pg_id, shard_id, "empty shard found in move_from_chunk={}", move_from_chunk); - // TODO::send a delete shard request to raft channel. there is a case that when we are doing gc, the - // shard becomes empty, need to handle this case - - // we should always write a shard header for the last shard if the state of it is open. - if (last_shard_state != ShardInfo::State::OPEN || !is_last_shard) continue; + GCLOGD(task_id, pg_id, shard_id, "empty shard found in move_from_chunk={}, skip", move_from_chunk); + continue; } else { GCLOGD(task_id, pg_id, shard_id, "{} valid blobs found in move_from_chunk={}", valid_blob_indexes.size(), move_from_chunk); @@ -812,220 +785,120 @@ bool GCManager::pdev_gc_actor::copy_valid_data( } } - // prepare a shard header for this shard in move_to_chunk - sisl::sg_list header_sgs = generate_shard_super_blk_sg_list(shard_id); - - // we ignore the state in shard header blk. we never read a shard header since we don`t know where it is(nor - // record the pba in indextable) -#if 0 - // we now generate shard header from metablk. the shard state in shard header blk should be open, but for sealed - // shard, the state in the generated in-memory header_sgs is sealed. - if (!is_last_shard_in_emergent_chunk) { - // for the sealed shard, the shard state in header should also be open.now, the written header is the same - // as footer except the shard state, so we lost the original header. - r_cast< HSHomeObject::shard_info_superblk* >(header_sgs.iovs[0].iov_base)->info.state = - ShardInfo::State::OPEN; - - // TODO:: get the original header from the move_from_chunk and change the following part if needed. - /* - uint64_t created_time; - uint64_t last_modified_time; - uint64_t available_capacity_bytes; - uint64_t total_capacity_bytes; - */ - } -#endif - // TODO::involve ratelimiter in the following code, where read/write are scheduled. or do we need a central - // ratelimter shared by all components except client io? - const auto succeed_copying_shard = - // 1 write the shard header to move_to_chunk - data_service.async_alloc_write(header_sgs, hints, out_blkids) - .thenValue([this, &hints, &move_to_chunk, &move_from_chunk, &is_last_shard, &shard_id, &blk_size, - &valid_blob_indexes, &data_service, task_id, &last_shard_state, &copied_blobs, pg_id, - header_sgs = std::move(header_sgs)](auto&& err) { - RELEASE_ASSERT(header_sgs.iovs.size() == 1, "header_sgs.iovs.size() should be 1, but not!"); - // shard header occupies one blk - COUNTER_INCREMENT(metrics_, gc_write_blk_count, 1); - iomanager.iobuf_free(reinterpret_cast< uint8_t* >(header_sgs.iovs[0].iov_base)); - if (err) { - GCLOGE(task_id, pg_id, shard_id, - "Failed to write shard header for move_to_chunk={}, err={}, err_category={}, " - "err_message={}", - move_to_chunk, err.value(), err.category().name(), err.message()); - return folly::makeFuture< bool >(false); - } - - if (valid_blob_indexes.empty()) { - RELEASE_ASSERT(is_last_shard && last_shard_state == ShardInfo::State::OPEN, - "find empty shard in move_from_chunk={} " - "but is_emergent is false, shard_id=0x{:x}, pg_id={}", - move_from_chunk, shard_id, pg_id); - return folly::makeFuture< bool >(true); - } + // copy all the valid blobs in the shard from move_from_chunk to move_to_chunk + std::vector< folly::Future< bool > > futs; + for (const auto& [k, v] : valid_blob_indexes) { + // k is shard_id + blob_id, v is multiblk id + auto pba = v.pbas(); + auto total_size = pba.blk_count() * blk_size; + + // buffer for read and write data + sisl::sg_list data_sgs; + data_sgs.size = total_size; + data_sgs.iovs.emplace_back( + iovec{.iov_base = iomanager.iobuf_alloc(blk_size, total_size), .iov_len = total_size}); + + futs.emplace_back(std::move( + // read blob from move_from_chunk + data_service.async_read(pba, data_sgs, total_size) + .thenValue([this, k, &hints, &move_from_chunk, &move_to_chunk, &data_service, task_id, pg_id, + data_sgs = std::move(data_sgs), pba, &copied_blobs](auto&& err) { + COUNTER_INCREMENT(metrics_, gc_read_blk_count, pba.blk_count()); + RELEASE_ASSERT(data_sgs.iovs.size() == 1, "data_sgs.iovs.size() should be 1, but not!"); + + const auto shard_id = k.key().shard; + const auto blob_id = k.key().blob; + + if (err) { + GCLOGE(task_id, pg_id, shard_id, + "Failed to read blob from move_from_chunk={}, blob_id={}, err={}, " + "err_category={}, err_message={}", + move_from_chunk, blob_id, err.value(), err.category().name(), err.message()); + iomanager.iobuf_free(reinterpret_cast< uint8_t* >(data_sgs.iovs[0].iov_base)); + return folly::makeFuture< bool >(false); + } + + GCLOGD(task_id, pg_id, shard_id, + "successfully read blob from move_from_chunk={}, blob_id={}, pba={}", move_from_chunk, + blob_id, pba.to_string()); + + if (m_enable_read_verify) { + // after a blob is deleted at originator, if it receives a fetch_data request of + // this blob, a fake delete_marker blob will be returned to the requester. This + // case happens in incremental resync scenario. when verifying blob, if it is a + // delete_marker, we let it pass the verification in gc scenario so that it will + // not block any gc task. + if (!m_hs_home_object->verify_blob(data_sgs.iovs[0].iov_base, shard_id, blob_id, true)) { + GCLOGE(task_id, pg_id, shard_id, + "blob verification fails for move_from_chunk={}, blob_id={}, pba={}", + move_from_chunk, blob_id, pba.to_string()); + iomanager.iobuf_free(reinterpret_cast< uint8_t* >(data_sgs.iovs[0].iov_base)); + return folly::makeFuture< bool >(false); + } + } + + // write the blob to the move_to_chunk. we do not care about the blob order in a + // shard since we can not guarantee a certain order + homestore::MultiBlkId new_pba; + return data_service.async_alloc_write(data_sgs, hints, new_pba) + .thenValue([this, shard_id, blob_id, new_pba, &move_to_chunk, task_id, pg_id, &copied_blobs, + data_sgs = std::move(data_sgs)](auto&& err) { + COUNTER_INCREMENT(metrics_, gc_write_blk_count, new_pba.blk_count()); + RELEASE_ASSERT(data_sgs.iovs.size() == 1, "data_sgs.iovs.size() should be 1, but not!"); + iomanager.iobuf_free(reinterpret_cast< uint8_t* >(data_sgs.iovs[0].iov_base)); + if (err) { + GCLOGE(task_id, pg_id, shard_id, + "Failed to write blob to move_to_chunk={}, blob_id={}, err={}, " + "err_category={}, err_message={}", + move_to_chunk, blob_id, err.value(), err.category().name(), err.message()); + return false; + } - std::vector< folly::Future< bool > > futs; - - // 2 copy all the valid blobs in the shard from move_from_chunk to move_to_chunk - for (const auto& [k, v] : valid_blob_indexes) { - // k is shard_id + blob_id, v is multiblk id - auto pba = v.pbas(); - auto total_size = pba.blk_count() * blk_size; - - // buffer for read and write data - sisl::sg_list data_sgs; - data_sgs.size = total_size; - data_sgs.iovs.emplace_back( - iovec{.iov_base = iomanager.iobuf_alloc(blk_size, total_size), .iov_len = total_size}); - - futs.emplace_back(std::move( - // read blob from move_from_chunk - data_service.async_read(pba, data_sgs, total_size) - .thenValue([this, k, &hints, &move_from_chunk, &move_to_chunk, &data_service, task_id, - pg_id, data_sgs = std::move(data_sgs), pba, &copied_blobs](auto&& err) { - COUNTER_INCREMENT(metrics_, gc_read_blk_count, pba.blk_count()); - RELEASE_ASSERT(data_sgs.iovs.size() == 1, - "data_sgs.iovs.size() should be 1, but not!"); - - const auto shard_id = k.key().shard; - const auto blob_id = k.key().blob; - - if (err) { - GCLOGE(task_id, pg_id, shard_id, - "Failed to read blob from move_from_chunk={}, blob_id={}, err={}, " - "err_category={}, err_message={}", - move_from_chunk, blob_id, err.value(), err.category().name(), - err.message()); - iomanager.iobuf_free(reinterpret_cast< uint8_t* >(data_sgs.iovs[0].iov_base)); - return folly::makeFuture< bool >(false); - } - - GCLOGD(task_id, pg_id, shard_id, - "successfully read blob from move_from_chunk={}, blob_id={}, pba={}", - move_from_chunk, blob_id, pba.to_string()); - - if (m_enable_read_verify) { - // after a blob is deleted at originator, if it receives a fetch_data request of - // this blob, a fake delete_marker blob will be returned to the requester. This - // case happens in incremental resync scenario. when verifying blob, if it is a - // delete_marker, we let it pass the verification in gc scenario so that it will - // not block any gc task. - if (!m_hs_home_object->verify_blob(data_sgs.iovs[0].iov_base, shard_id, blob_id, - true)) { - GCLOGE(task_id, pg_id, shard_id, - "blob verification fails for move_from_chunk={}, blob_id={}, pba={}", - move_from_chunk, blob_id, pba.to_string()); - iomanager.iobuf_free( - reinterpret_cast< uint8_t* >(data_sgs.iovs[0].iov_base)); - return folly::makeFuture< bool >(false); - } - } - - // write the blob to the move_to_chunk. we do not care about the blob order in a - // shard since we can not guarantee a certain order - homestore::MultiBlkId new_pba; - return data_service.async_alloc_write(data_sgs, hints, new_pba) - .thenValue([this, shard_id, blob_id, new_pba, &move_to_chunk, task_id, pg_id, - &copied_blobs, data_sgs = std::move(data_sgs)](auto&& err) { - COUNTER_INCREMENT(metrics_, gc_write_blk_count, new_pba.blk_count()); - RELEASE_ASSERT(data_sgs.iovs.size() == 1, - "data_sgs.iovs.size() should be 1, but not!"); - iomanager.iobuf_free( - reinterpret_cast< uint8_t* >(data_sgs.iovs[0].iov_base)); - if (err) { - GCLOGE(task_id, pg_id, shard_id, - "Failed to write blob to move_to_chunk={}, blob_id={}, err={}, " - "err_category={}, err_message={}", - move_to_chunk, blob_id, err.value(), err.category().name(), - err.message()); - return false; - } - - // insert a new entry to gc index table for this blob. [move_to_chunk_id, - // shard_id, blob_id] -> [new pba] - BlobRouteByChunkKey key{BlobRouteByChunk{move_to_chunk, shard_id, blob_id}}; - BlobRouteValue value{new_pba}, existing_value; - - homestore::BtreeSinglePutRequest put_req{ - &key, &value, homestore::btree_put_type::INSERT, &existing_value}; - auto status = m_index_table->put(put_req); - if (status != homestore::btree_status_t::success) { - GCLOGE(task_id, pg_id, shard_id, - "Failed to insert new key to gc index table for " - "move_to_chunk={}, blob_id={}, err={}", - move_to_chunk, blob_id, status); - return false; - } - - GCLOGD(task_id, pg_id, shard_id, - "successfully insert new key to gc index table for " - "move_to_chunk={}, blob_id={}, new_pba={}", - move_to_chunk, blob_id, new_pba.to_string()); - - BlobRouteByChunk route_key{move_to_chunk, shard_id, blob_id}; - auto ret = copied_blobs.insert(route_key, value); - RELEASE_ASSERT(ret.second, - "we should not copy the same blob twice in gc task, " - "move_to_chunk={}, shard_id=0x{:x}, pg_id={}, blob_id={}", - move_to_chunk, shard_id, pg_id, blob_id); - - return true; - }); - }))); - } + // insert a new entry to gc index table for this blob. [move_to_chunk_id, + // shard_id, blob_id] -> [new pba] + BlobRouteByChunkKey key{BlobRouteByChunk{move_to_chunk, shard_id, blob_id}}; + BlobRouteValue value{new_pba}, existing_value; - // 3 write a shard footer for this shard - sisl::sg_list footer_sgs = generate_shard_super_blk_sg_list(shard_id); - return folly::collectAllUnsafe(futs) - .thenValue([this, &is_last_shard, &shard_id, &blk_size, &hints, &move_to_chunk, pg_id, - &last_shard_state, task_id, &data_service, footer_sgs](auto&& results) { - // if any blob copy fails, we will not write footer, and drop this gc task - for (auto const& ok : results) { - RELEASE_ASSERT(ok.hasValue(), "we never throw any exception when copying data"); - if (!ok.value()) { + homestore::BtreeSinglePutRequest put_req{ + &key, &value, homestore::btree_put_type::INSERT, &existing_value}; + auto status = m_index_table->put(put_req); + if (status != homestore::btree_status_t::success) { GCLOGE(task_id, pg_id, shard_id, - "Failed to copy blob for move_to_chunk={}, will cancel this task", - move_to_chunk); - // if any op fails, we drop this gc task. - return folly::makeFuture< std::error_code >( - std::make_error_code(std::errc::operation_canceled)); + "Failed to insert new key to gc index table for " + "move_to_chunk={}, blob_id={}, err={}", + move_to_chunk, blob_id, status); + return false; } - } - // we skip writing footer only if the last shard of this chunk is in open state. - if (is_last_shard && last_shard_state == ShardInfo::State::OPEN) { GCLOGD(task_id, pg_id, shard_id, - "skip writing the footer for move_to_chunk={} for emergent gc task", - move_to_chunk); - return folly::makeFuture< std::error_code >(std::error_code{}); - } + "successfully insert new key to gc index table for " + "move_to_chunk={}, blob_id={}, new_pba={}", + move_to_chunk, blob_id, new_pba.to_string()); + + BlobRouteByChunk route_key{move_to_chunk, shard_id, blob_id}; + auto ret = copied_blobs.insert(route_key, value); + RELEASE_ASSERT(ret.second, + "we should not copy the same blob twice in gc task, " + "move_to_chunk={}, shard_id=0x{:x}, pg_id={}, blob_id={}", + move_to_chunk, shard_id, pg_id, blob_id); + + return true; + }); + }))); + } - // write shard footer, which occupies one blk - - // in fact , we have a dangling reference of out_blkids here, but it does not matter since - // we do not use it after this and the async_alloc_write will complete soon and the memory - // of out_blkids will be released after that. - homestore::MultiBlkId out_blkids; - COUNTER_INCREMENT(metrics_, gc_write_blk_count, 1); - return data_service.async_alloc_write(footer_sgs, hints, out_blkids); - }) - .thenValue([this, &move_to_chunk, &shard_id, footer_sgs, task_id, pg_id](auto&& err) { - RELEASE_ASSERT(footer_sgs.iovs.size() == 1, "footer_sgs.iovs.size() should be 1, but not!"); - iomanager.iobuf_free(reinterpret_cast< uint8_t* >(footer_sgs.iovs[0].iov_base)); - - if (err) { - // we will come here if: - // 1 any blob copy fails, then err is operation_cancelled - // 2 write footer fails， then err is the error code of write footer - GCLOGE( - task_id, pg_id, shard_id, - "Failed to copy some blobs or failed to write shard footer for move_to_chunk={}, " - "err={}, error_category={}, error_message={}, pls check the log for more " - "detailed info", - move_to_chunk, err.value(), err.category().name(), err.message()); - return false; - } - return true; - }); + const auto succeed_copying_shard = + folly::collectAllUnsafe(futs) + .thenValue([this, &shard_id, &move_to_chunk, task_id, pg_id](auto&& results) { + for (auto const& ok : results) { + RELEASE_ASSERT(ok.hasValue(), "we never throw any exception when copying data"); + if (!ok.value()) { + GCLOGE(task_id, pg_id, shard_id, + "Failed to copy blob for move_to_chunk={}, will cancel this task", move_to_chunk); + return false; + } + } + return true; }) .get(); diff --git a/src/lib/homestore_backend/gc_manager.hpp b/src/lib/homestore_backend/gc_manager.hpp index f85d9718..abd078a6 100644 --- a/src/lib/homestore_backend/gc_manager.hpp +++ b/src/lib/homestore_backend/gc_manager.hpp @@ -291,10 +291,6 @@ class GCManager { pdev_gc_metrics& metrics() { return metrics_; } - private: - // utils - sisl::sg_list generate_shard_super_blk_sg_list(shard_id_t shard_id); - private: friend class gc_task_guard; uint32_t m_pdev_id; diff --git a/src/lib/homestore_backend/heap_chunk_selector.cpp b/src/lib/homestore_backend/heap_chunk_selector.cpp index 1068ebb0..bfba1052 100644 --- a/src/lib/homestore_backend/heap_chunk_selector.cpp +++ b/src/lib/homestore_backend/heap_chunk_selector.cpp @@ -44,27 +44,10 @@ void HeapChunkSelector::add_chunk_internal(const chunk_num_t p_chunk_id, bool ad } } -// select_chunk will only be called in homestore when creating a shard. +// select_chunk will never be called in homestore since create shard is log only. csharedChunk HeapChunkSelector::select_chunk(homestore::blk_count_t count, const homestore::blk_alloc_hints& hint) { - auto& chunkIdHint = hint.chunk_id_hint; - if (chunkIdHint.has_value()) { - LOGWARNMOD(homeobject, "should not allocated a chunk with exiting chunkIdHint={} in hint!", - chunkIdHint.value()); - return nullptr; - } - - if (!hint.application_hint.has_value()) { - LOGWARNMOD(homeobject, "should not allocated a chunk without exiting application_hint in hint!"); - return nullptr; - } else { - // Both chunk_num_t and pg_id_t are of type uint16_t. - static_assert(std::is_same< pg_id_t, uint16_t >::value, "pg_id_t is not uint16_t"); - static_assert(std::is_same< homestore::chunk_num_t, uint16_t >::value, "chunk_num_t is not uint16_t"); - auto application_hint = hint.application_hint.value(); - pg_id_t pg_id = (uint16_t)(application_hint >> 16 & 0xFFFF); - homestore::chunk_num_t v_chunk_id = (uint16_t)(application_hint & 0xFFFF); - return select_specific_chunk(pg_id, v_chunk_id); - } + RELEASE_ASSERT(false, "create shard is log only, this function should never be called"); + return nullptr; } bool HeapChunkSelector::try_mark_chunk_to_gc_state(const chunk_num_t chunk_id, bool force) { @@ -106,7 +89,8 @@ void HeapChunkSelector::mark_chunk_out_of_gc_state(const chunk_num_t chunk_id, c final_state); } -csharedChunk HeapChunkSelector::select_specific_chunk(const pg_id_t pg_id, const chunk_num_t v_chunk_id) { +homestore::cshared< HeapChunkSelector::ExtendedVChunk > +HeapChunkSelector::select_specific_chunk(const pg_id_t pg_id, const chunk_num_t v_chunk_id) { homestore::shared< ExtendedVChunk > chunk; while (true) { @@ -147,7 +131,7 @@ csharedChunk HeapChunkSelector::select_specific_chunk(const pg_id_t pg_id, const LOGDEBUGMOD(homeobject, "chunk={} is selected for v_chunk_id={}, pg={}", chunk->get_chunk_id(), v_chunk_id, pg_id); - return chunk->get_internal_chunk(); + return chunk; } void HeapChunkSelector::foreach_chunks(std::function< void(csharedChunk&) >&& cb) { @@ -510,12 +494,13 @@ std::shared_ptr< const std::vector< homestore::chunk_num_t > > HeapChunkSelector return p_chunk_ids; } -std::optional< homestore::chunk_num_t > HeapChunkSelector::get_most_available_blk_chunk(uint64_t ctx, pg_id_t pg_id) { +homestore::cshared< HeapChunkSelector::ExtendedVChunk > +HeapChunkSelector::pick_most_available_blk_chunk(uint64_t ctx, pg_id_t pg_id) { std::shared_lock lock_guard(m_chunk_selector_mtx); auto pg_it = m_per_pg_chunks.find(pg_id); if (pg_it == m_per_pg_chunks.end()) { LOGWARNMOD(homeobject, "No pg found for pg={}", pg_id); - return std::nullopt; + return nullptr; } std::scoped_lock lock(pg_it->second->mtx); auto pg_chunk_collection = pg_it->second; @@ -527,7 +512,7 @@ std::optional< homestore::chunk_num_t > HeapChunkSelector::get_most_available_bl }); if (!(*max_it)->available()) { LOGWARNMOD(homeobject, "No available chunk for pg={}, ctx=0x{:x}", pg_id, ctx); - return std::nullopt; + return nullptr; } auto v_chunk_id = std::distance(pg_chunks.begin(), max_it); LOGDEBUGMOD(homeobject, "Picked v_chunk_id={} : [p_chunk_id={}, avail={}], ctx=0x{:x}", v_chunk_id, @@ -535,7 +520,7 @@ std::optional< homestore::chunk_num_t > HeapChunkSelector::get_most_available_bl pg_chunks[v_chunk_id]->m_state = ChunkState::INUSE; --pg_chunk_collection->available_num_chunks; pg_chunk_collection->available_blk_count -= pg_chunks[v_chunk_id]->available_blks(); - return v_chunk_id; + return pg_chunks[v_chunk_id]; } // return the maximum number of chunks that can be allocated on pdev diff --git a/src/lib/homestore_backend/heap_chunk_selector.h b/src/lib/homestore_backend/heap_chunk_selector.h index a94d74c4..1ffa12c1 100644 --- a/src/lib/homestore_backend/heap_chunk_selector.h +++ b/src/lib/homestore_backend/heap_chunk_selector.h @@ -75,7 +75,7 @@ class HeapChunkSelector : public homestore::ChunkSelector { // this function will be used by create shard or recovery flow to mark one specific chunk to be busy, caller should // be responsible to use release_chunk() interface to release it when no longer to use the chunk anymore. - csharedChunk select_specific_chunk(const pg_id_t pg_id, const chunk_num_t v_chunk_id); + homestore::cshared< ExtendedVChunk > select_specific_chunk(const pg_id_t pg_id, const chunk_num_t v_chunk_id); /** * try to mark a chunk as gc state, so that it will not be selected by any creating shard. @@ -124,9 +124,9 @@ class HeapChunkSelector : public homestore::ChunkSelector { * * @param ctx only for logging. * @param pg_id The ID of the pg. - * @return An optional chunk_num_t value representing v_chunk_id, or std::nullopt if no space left. + * @return The most available ExtendedVChunk for the pg, or nullptr if no chunk is available. */ - std::optional< chunk_num_t > get_most_available_blk_chunk(uint64_t ctx, pg_id_t pg_id); + homestore::cshared< ExtendedVChunk > pick_most_available_blk_chunk(uint64_t ctx, pg_id_t pg_id); // this should be called on each pg meta blk found bool recover_pg_chunks(pg_id_t pg_id, std::vector< chunk_num_t >&& p_chunk_ids); diff --git a/src/lib/homestore_backend/hs_blob_manager.cpp b/src/lib/homestore_backend/hs_blob_manager.cpp index 90aa9edd..0a377e3f 100644 --- a/src/lib/homestore_backend/hs_blob_manager.cpp +++ b/src/lib/homestore_backend/hs_blob_manager.cpp @@ -6,6 +6,9 @@ #include "lib/blob_route.hpp" #include #include +#ifdef _PRERELEASE +#include +#endif SISL_LOGGING_DECL(blobmgr) @@ -226,6 +229,20 @@ bool HSHomeObject::local_add_blob_info(pg_id_t const pg_id, BlobInfo const& blob return false; } if (!exist_already) { + homestore::chunk_num_t sb_pchunk_id; + { + std::shared_lock lock_guard(_shard_lock); + const auto iter = _shard_map.find(blob_info.shard_id); + RELEASE_ASSERT(iter != _shard_map.end(), "shardID=0x{:x}, pg={}, shard=0x{:x}, shard does not exist", + blob_info.shard_id, (blob_info.shard_id >> homeobject::shard_width), + (blob_info.shard_id & homeobject::shard_mask)); + sb_pchunk_id = d_cast< HS_Shard* >((*iter->second).get())->p_chunk_id(); + } + RELEASE_ASSERT(blob_info.pbas.chunk_num() == sb_pchunk_id, + "traceID={}, commit-time pchunk mismatch: blob pchunk={} shard pchunk={} " + "shard=0x{:x} blob={} pg={}", + tid, blob_info.pbas.chunk_num(), sb_pchunk_id, blob_info.shard_id, blob_info.blob_id, pg_id); + // The PG superblock (durable entities) will be persisted as part of HS_CLIENT Checkpoint, which is always // done ahead of the Index Checkpoint. Hence, if the index already has this entity, whatever durable // counters updated as part of the update would have been persisted already in PG superblock. So if we were @@ -266,11 +283,46 @@ void HSHomeObject::on_blob_put_commit(int64_t lsn, sisl::blob const& header, sis return; } +#ifdef _PRERELEASE + // Pause PUT_BLOB commit at function entry. While paused the test can seal the shard; the + // sealed_lsn guard then rejects this late blob when the gate releases. + iomgr_flip::instance()->callback_flip("pause_put_blob_commit"); +#endif + + const auto shard_id = msg_header->shard_id; auto const blob_id = *(reinterpret_cast< blob_id_t* >(const_cast< uint8_t* >(key.cbytes()))); + + int64_t shard_sealed_lsn; + { + std::scoped_lock lock_guard(_shard_lock); + auto iter = _shard_map.find(shard_id); + RELEASE_ASSERT(iter != _shard_map.end(), "shardID=0x{:x}, pg={}, shard=0x{:x}, shard does not exist", shard_id, + (shard_id >> homeobject::shard_width), (shard_id & homeobject::shard_mask)); + shard_sealed_lsn = (*iter->second)->info.sealed_lsn; + } + + if (lsn >= shard_sealed_lsn) { + homestore::data_service().async_free_blk(pbas).thenValue([lsn, shard_id, blob_id, tid, pbas](auto&& err) { + if (err) { + BLOGW(tid, shard_id, blob_id, "failed to free blob data blk, err={}, lsn={}, blkid={}", err.message(), + lsn, pbas.to_string()); + } else { + BLOGD(tid, shard_id, blob_id, "succeed to free blob data blk, lsn={}, blkid={}", lsn, pbas.to_string()); + } + }); + + BLOGD(tid, shard_id, blob_id, + "try to commit put_blob message to a non-open shard, lsn={}, shard_sealed_lsn={}, skip it!", lsn, + shard_sealed_lsn); + + if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(BlobError(BlobErrorCode::SEALED_SHARD))); } + return; + } + auto const pg_id = msg_header->pg_id; BlobInfo blob_info; - blob_info.shard_id = msg_header->shard_id; + blob_info.shard_id = shard_id; blob_info.blob_id = blob_id; blob_info.pbas = pbas; diff --git a/src/lib/homestore_backend/hs_homeobject.hpp b/src/lib/homestore_backend/hs_homeobject.hpp index 75c182b2..afa8c465 100644 --- a/src/lib/homestore_backend/hs_homeobject.hpp +++ b/src/lib/homestore_backend/hs_homeobject.hpp @@ -106,9 +106,9 @@ class HSHomeObject : public HomeObjectImpl { std::vector< shard_id_t > shards_to_migrate_; public: - // Old version shard_info_superblk (v0.01) - for backward compatibility testing and migration - // v1 ShardInfo did not have the meta field - struct v1_ShardInfo { + // Old version shard_info_superblk (v0.02) - for backward compatibility testing and migration + // v2 ShardInfo — did not have the sealed_lsn + struct v2_ShardInfo { shard_id_t id; pg_id_t placement_group; ShardInfo::State state; @@ -118,7 +118,7 @@ class HSHomeObject : public HomeObjectImpl { uint64_t available_capacity_bytes; uint64_t total_capacity_bytes; std::optional< peer_id_t > current_leader{std::nullopt}; - // Note: meta field was added in v2 + uint8_t meta[ShardInfo::meta_length]{}; }; #pragma pack(1) @@ -207,20 +207,21 @@ class HSHomeObject : public HomeObjectImpl { }; struct shard_info_superblk : DataHeader { - // This version is a common version of DataHeader, each derived struct can have its own version. - static constexpr uint8_t shard_sb_version = 0x02; + // v2: added meta, v3: added sealed_lsn & changed the order of info to be the end + static constexpr uint8_t shard_sb_version = 0x03; uint8_t sb_version{shard_sb_version}; - ShardInfo info; homestore::chunk_num_t p_chunk_id{0}; homestore::chunk_num_t v_chunk_id{0}; - + ShardInfo info; // backward compatibility bool valid() const { return DataHeader::valid() && sb_version <= shard_sb_version; } }; - struct v1_shard_info_superblk : DataHeader { - v1_ShardInfo info; + // v2 superblk: sb_version=0x02, v2_ShardInfo (no sealed_lsn), chunk IDs after info + struct v2_shard_info_superblk : DataHeader { + uint8_t sb_version{0x02}; + v2_ShardInfo info; homestore::chunk_num_t p_chunk_id{0}; homestore::chunk_num_t v_chunk_id{0}; }; @@ -766,8 +767,7 @@ class HSHomeObject : public HomeObjectImpl { static ShardInfo deserialize_shard_info(const char* shard_info_str, size_t size); static std::string serialize_shard_info(const ShardInfo& info); - void local_create_shard(ShardInfo shard_info, homestore::chunk_num_t v_chunk_id, homestore::chunk_num_t p_chunk_id, - homestore::blk_count_t blk_count, trace_id_t tid = 0); + void local_create_shard(ShardInfo shard_info, homestore::chunk_num_t v_chunk_id, trace_id_t tid = 0); void add_new_shard_to_map(std::unique_ptr< HS_Shard > shard); void update_shard_in_map(const ShardInfo& shard_info); @@ -901,8 +901,8 @@ class HSHomeObject : public HomeObjectImpl { * @param repl_dev The replication device. * @param hs_ctx The replication request context. */ - void on_shard_message_commit(int64_t lsn, sisl::blob const& header, homestore::MultiBlkId const& blkids, - shared< homestore::ReplDev > repl_dev, cintrusive< homestore::repl_req_ctx >& hs_ctx); + void on_shard_message_commit(int64_t lsn, sisl::blob const& header, shared< homestore::ReplDev > repl_dev, + cintrusive< homestore::repl_req_ctx >& hs_ctx); bool on_shard_message_pre_commit(int64_t lsn, sisl::blob const& header, sisl::blob const& key, cintrusive< homestore::repl_req_ctx >& hs_ctx); @@ -949,28 +949,6 @@ class HSHomeObject : public HomeObjectImpl { */ void on_replica_restart(); - /** - * @brief Extracts the physical chunk ID for create shard from the message. - * - * @param header The message header that includes the shard_info_superblk, which contains the data necessary for - * extracting and mapping the chunk ID. - * @return An optional virtual chunk id if the extraction and mapping process is successful, otherwise an empty - * optional. - */ - std::optional< homestore::chunk_num_t > resolve_v_chunk_id_from_msg(sisl::blob const& header); - - /** - * @brief Releases a chunk based on the information provided in a CREATE_SHARD message. - * - * This function is invoked during log rollback or when the proposer encounters an error. - * Its primary purpose is to ensure that the state of pg_chunks is reverted to the correct state. - * - * @param header The message header that includes the shard_info_superblk, which contains the data necessary for - * extracting and mapping the chunk ID. - * @return Returns true if the chunk was successfully released, false otherwise. - */ - bool release_chunk_based_on_create_shard_message(sisl::blob const& header); - /** * @brief check whether the chunks in a given pg can be gc. * diff --git a/src/lib/homestore_backend/hs_http_manager.cpp b/src/lib/homestore_backend/hs_http_manager.cpp index 3e274bae..657aea17 100644 --- a/src/lib/homestore_backend/hs_http_manager.cpp +++ b/src/lib/homestore_backend/hs_http_manager.cpp @@ -265,7 +265,8 @@ void HttpManager::get_shard(const Pistache::Rest::Request& request, Pistache::Ht j["created_time"] = shard_info.created_time; j["last_modified_time"] = shard_info.last_modified_time; j["state"] = shard_info.state; - j["lsn"] = shard_info.lsn; + j["created_lsn"] = shard_info.create_lsn; + j["sealed_lsn"] = shard_info.sealed_lsn; j["meta"] = std::string(reinterpret_cast< const char* >(shard_info.meta)); auto blobs = ho_.get_shard_blobs(shard_id); if (!blobs) { @@ -305,7 +306,8 @@ void HttpManager::dump_chunk(const Pistache::Rest::Request& request, Pistache::H shard_json["created_time"] = s.info.created_time; shard_json["last_modified_time"] = s.info.last_modified_time; shard_json["state"] = s.info.state; - shard_json["lsn"] = s.info.lsn; + shard_json["created_lsn"] = s.info.create_lsn; + shard_json["sealed_lsn"] = s.info.sealed_lsn; shard_json["meta"] = std::string(reinterpret_cast< const char* >(s.info.meta)); j["shards"].push_back(shard_json); } @@ -343,7 +345,8 @@ void HttpManager::dump_shard(const Pistache::Rest::Request& request, Pistache::H j["created_time"] = shard_info.created_time; j["last_modified_time"] = shard_info.last_modified_time; j["state"] = shard_info.state; - j["lsn"] = shard_info.lsn; + j["created_lsn"] = shard_info.create_lsn; + j["sealed_lsn"] = shard_info.sealed_lsn; j["meta"] = std::string(reinterpret_cast< const char* >(shard_info.meta)); auto r = ho_.get_shard_blobs(shard_id); diff --git a/src/lib/homestore_backend/hs_shard_manager.cpp b/src/lib/homestore_backend/hs_shard_manager.cpp index 8c949cb3..874ad27e 100644 --- a/src/lib/homestore_backend/hs_shard_manager.cpp +++ b/src/lib/homestore_backend/hs_shard_manager.cpp @@ -2,6 +2,9 @@ #include #include #include +#ifdef _PRERELEASE +#include +#endif #include "hs_homeobject.hpp" #include "replication_message.hpp" @@ -23,6 +26,37 @@ SISL_LOGGING_DECL(shardmgr) #define SLOGE(trace_id, shard_id, msg, ...) SLOG(ERROR, trace_id, shard_id, msg, ##__VA_ARGS__) #define SLOGC(trace_id, shard_id, msg, ...) SLOG(CRITICAL, trace_id, shard_id, msg, ##__VA_ARGS__) +static ShardInfo shardinfo_from_v2(const HSHomeObject::v2_ShardInfo& v2) { + ShardInfo info; + info.id = v2.id; + info.placement_group = v2.placement_group; + info.state = v2.state; + info.create_lsn = v2.lsn; + info.sealed_lsn = INT64_MAX; + info.created_time = v2.created_time; + info.last_modified_time = v2.last_modified_time; + info.available_capacity_bytes = v2.available_capacity_bytes; + info.total_capacity_bytes = v2.total_capacity_bytes; + info.current_leader = v2.current_leader; + std::memcpy(info.meta, v2.meta, ShardInfo::meta_length); + return info; +} + +// Core decode: given a typed shard_info_superblk pointer, extract ShardInfo + chunk IDs. +// Handles all supported on-disk versions. Returns nullopt for unknown versions. +static std::optional< std::tuple< ShardInfo, homestore::chunk_num_t, homestore::chunk_num_t > > +decode_shard_sb(const HSHomeObject::shard_info_superblk* sb) { + if (sb->sb_version == HSHomeObject::shard_info_superblk::shard_sb_version) { + return std::make_tuple(sb->info, sb->p_chunk_id, sb->v_chunk_id); + } else if (sb->sb_version == 0x02) { + const auto* v2sb = r_cast< const HSHomeObject::v2_shard_info_superblk* >(sb); + return std::make_tuple(shardinfo_from_v2(v2sb->info), v2sb->p_chunk_id, v2sb->v_chunk_id); + } else { + LOGW("Unsupported shard_info_superblk sb_version={}", sb->sb_version); + return std::nullopt; + } +} + ShardError toShardError(ReplServiceError const& e) { switch (e) { case ReplServiceError::BAD_REQUEST: @@ -88,7 +122,8 @@ std::string HSHomeObject::serialize_shard_info(const ShardInfo& info) { j["shard_info"]["shard_id_t"] = info.id; j["shard_info"]["pg_id_t"] = info.placement_group; j["shard_info"]["state"] = info.state; - j["shard_info"]["lsn"] = info.lsn; + j["shard_info"]["lsn"] = info.create_lsn; + j["shard_info"]["sealed_lsn"] = info.sealed_lsn; j["shard_info"]["created_time"] = info.created_time; j["shard_info"]["modified_time"] = info.last_modified_time; j["shard_info"]["total_capacity"] = info.total_capacity_bytes; @@ -103,7 +138,10 @@ ShardInfo HSHomeObject::deserialize_shard_info(const char* json_str, size_t str_ shard_info.id = shard_json["shard_info"]["shard_id_t"].get< shard_id_t >(); shard_info.placement_group = shard_json["shard_info"]["pg_id_t"].get< pg_id_t >(); shard_info.state = static_cast< ShardInfo::State >(shard_json["shard_info"]["state"].get< int >()); - shard_info.lsn = shard_json["shard_info"]["lsn"].get< uint64_t >(); + shard_info.create_lsn = shard_json["shard_info"]["lsn"].get< uint64_t >(); + shard_info.sealed_lsn = shard_json["shard_info"].contains("sealed_lsn") + ? shard_json["shard_info"]["sealed_lsn"].get< uint64_t >() + : INT64_MAX; shard_info.created_time = shard_json["shard_info"]["created_time"].get< uint64_t >(); shard_info.last_modified_time = shard_json["shard_info"]["modified_time"].get< uint64_t >(); shard_info.available_capacity_bytes = shard_json["shard_info"]["available_capacity"].get< uint64_t >(); @@ -159,26 +197,49 @@ ShardManager::AsyncResult< ShardInfo > HSHomeObject::_create_shard(pg_id_t pg_ow } auto new_shard_id = generate_new_shard_id(pg_owner); SLOGD(tid, new_shard_id, "Create shard request: pg={}, size={}", pg_owner, size_bytes); - auto create_time = get_current_timestamp(); // select chunk for shard. - const auto v_chunkID = chunk_selector()->get_most_available_blk_chunk(new_shard_id, pg_owner); - if (!v_chunkID.has_value()) { - SLOGW(tid, new_shard_id, "no availble chunk left to create shard for pg={}", pg_owner); + const auto exVchunk = chunk_selector()->pick_most_available_blk_chunk(new_shard_id, pg_owner); + + if (exVchunk == nullptr) { + SLOGW(tid, new_shard_id, "no available chunk left to create shard for pg={}", pg_owner); decr_pending_request_num(); return folly::makeUnexpected(ShardError(ShardErrorCode::NO_SPACE_LEFT)); } - const auto v_chunk_id = v_chunkID.value(); + + // we will not persist shard header/footer to disk anymore, and the emergency gc will never be triggered by + // create/seal shard message. + RELEASE_ASSERT(exVchunk->m_v_chunk_id.has_value(), "v_chunk_id should have value for selected chunk for pg={}", + pg_owner); + const auto v_chunk_id = exVchunk->m_v_chunk_id.value(); + if (exVchunk->available_blks() < get_reserved_blks()) { + const auto pchunk_id = exVchunk->get_chunk_id(); + LOGW("failed to create shard for pg={}, pchunk_id= {} is selected for vchunk_id={} is selected, not enough " + "left space", + pg_owner, pchunk_id, v_chunk_id); + + bool res = chunk_selector()->release_chunk(pg_owner, v_chunk_id); + RELEASE_ASSERT(res, "Failed to release v_chunk_id={}, pg={}", v_chunk_id, pg_owner); + + gc_manager()->submit_gc_task(task_priority::normal, pchunk_id); + + decr_pending_request_num(); + return folly::makeUnexpected(ShardError(ShardErrorCode::NO_SPACE_LEFT)); + } + SLOGD(tid, new_shard_id, "vchunk_id={}", v_chunk_id); - // Prepare the shard info block + auto create_time = get_current_timestamp(); + + // Prepare shard info superblk to carry shard info (including meta) via header_extn, + // so that all replicas can reconstruct it on commit. This follows the pre-cherry-pick pattern. sisl::io_blob_safe sb_blob(sisl::round_up(sizeof(shard_info_superblk), repl_dev->get_blk_size()), io_align); shard_info_superblk* sb = new (sb_blob.bytes()) shard_info_superblk(); sb->type = DataHeader::data_type_t::SHARD_INFO; sb->info = ShardInfo{.id = new_shard_id, .placement_group = pg_owner, .state = ShardInfo::State::OPEN, - .lsn = 0, + .create_lsn = 0, .created_time = create_time, .last_modified_time = create_time, .available_capacity_bytes = size_bytes, @@ -193,11 +254,7 @@ ShardManager::AsyncResult< ShardInfo > HSHomeObject::_create_shard(pg_id_t pg_ow auto req = repl_result_ctx< ShardManager::Result< ShardInfo > >::make( sizeof(shard_info_superblk) /* header_extn_size */, 0u /* key_size */); - // for create shard, we disable push_data, so that all the selecting chunk for creating shard will go through raft - // log channel, and thus, the the selecting chunk of later creating shard will go after that of the former one. - req->disable_push_data(); - - // prepare msg header; + // prepare msg header, log only req->header()->msg_type = ReplicationMessageType::CREATE_SHARD_MSG; req->header()->pg_id = pg_owner; req->header()->shard_id = new_shard_id; @@ -205,35 +262,24 @@ ShardManager::AsyncResult< ShardInfo > HSHomeObject::_create_shard(pg_id_t pg_ow req->header()->payload_crc = crc32_ieee(init_crc32, sb_blob.cbytes(), sizeof(shard_info_superblk)); req->header()->seal(); - // ShardInfo block is persisted on both on header and in data portion. - // It is persisted in header portion, so that it is written in journal and hence replay of journal on most cases - // doesn't need additional read from data blks. - // We also persist in data blocks for following reasons: - // * To recover the shard information in case both journal and metablk are lost - // * For garbage collection, we directly read from the data chunk and get shard information. + // ShardInfo block is persisted in header_extn so it is written in raft journal and available on replay. std::memcpy(req->header_extn(), sb_blob.cbytes(), sizeof(shard_info_superblk)); - req->add_data_sg(std::move(sb_blob)); - // replicate this create shard message to PG members; - repl_dev->async_alloc_write(req->cheader_buf(), sisl::blob{}, req->data_sgs(), req, false /* part_of_batch */, tid); + // replicate this create shard message to PG members (log-only, no data blocks); + repl_dev->async_alloc_write(req->cheader_buf(), sisl::blob{}, sisl::sg_list{}, req, false /* part_of_batch */, tid); return req->result().deferValue([this, req, repl_dev, tid, pg_owner, new_shard_id, v_chunk_id](const auto& result) -> ShardManager::AsyncResult< ShardInfo > { if (result.hasError()) { auto err = result.error(); if (err.getCode() == ShardErrorCode::NOT_LEADER) { err.current_leader = repl_dev->get_leader_id(); } + // we will never get no_space_left error here. bool res = chunk_selector()->release_chunk(pg_owner, v_chunk_id); RELEASE_ASSERT(res, "Failed to release v_chunk_id={}, pg={}", v_chunk_id, pg_owner); SLOGE(tid, new_shard_id, "got {} when creating shard at leader, failed to create shard {}!", err.getCode(), new_shard_id); - if (err.getCode() == ShardErrorCode::NO_SPACE_LEFT) { - gc_manager()->submit_gc_task(task_priority::normal, - chunk_selector()->get_pg_vchunk(pg_owner, v_chunk_id)->get_chunk_id()); - SLOGD(tid, new_shard_id, "got no space left error when creating shard {} at leader", new_shard_id); - } - decr_pending_request_num(); return folly::makeUnexpected(err); } @@ -251,8 +297,8 @@ ShardManager::AsyncResult< ShardInfo > HSHomeObject::_seal_shard(ShardInfo const } incr_pending_request_num(); - auto pg_id = info.placement_group; - auto shard_id = info.id; + const auto pg_id = info.placement_group; + const auto shard_id = info.id; SLOGD(tid, shard_id, "Seal shard request: is_open={}", info.is_open()); auto hs_pg = get_hs_pg(pg_id); if (!hs_pg) { @@ -286,17 +332,25 @@ ShardManager::AsyncResult< ShardInfo > HSHomeObject::_seal_shard(ShardInfo const return folly::makeUnexpected(ShardError(ShardErrorCode::RETRY_REQUEST)); } + const auto v_chunkID = get_shard_v_chunk_id(shard_id); + if (!v_chunkID.has_value()) { + SLOGW(tid, shard_id, "failed to seal shard, vchunk id not found"); + decr_pending_request_num(); + return folly::makeUnexpected(ShardError(ShardErrorCode::UNKNOWN_SHARD)); + } + const auto v_chunk_id = v_chunkID.value(); + ShardInfo tmp_info = info; tmp_info.state = ShardInfo::State::SEALED; - // Prepare the shard info block + // Prepare the shard info block to carry shard state via header_extn. + // Similar to create shard - ShardInfo block is persisted in header_extn. sisl::io_blob_safe sb_blob(sisl::round_up(sizeof(shard_info_superblk), repl_dev->get_blk_size()), io_align); shard_info_superblk* sb = new (sb_blob.bytes()) shard_info_superblk(); sb->type = DataHeader::data_type_t::SHARD_INFO; sb->info = tmp_info; - // p_chunk_id and v_chunk_id will never be used in seal shard workflow. sb->p_chunk_id = 0; - sb->v_chunk_id = 0; + sb->v_chunk_id = v_chunk_id; auto req = repl_result_ctx< ShardManager::Result< ShardInfo > >::make( sizeof(shard_info_superblk) /* header_extn_size */, 0u /* key_size */); @@ -307,12 +361,11 @@ ShardManager::AsyncResult< ShardInfo > HSHomeObject::_seal_shard(ShardInfo const req->header()->payload_crc = crc32_ieee(init_crc32, sb_blob.cbytes(), sizeof(shard_info_superblk)); req->header()->seal(); - // Similar to create shard - ShardInfo block is persisted on both on header and in data portion. + // Similar to create shard - ShardInfo block is persisted in header_extn. std::memcpy(req->header_extn(), sb_blob.cbytes(), sizeof(shard_info_superblk)); - req->add_data_sg(std::move(sb_blob)); - // replicate this seal shard message to PG members; - repl_dev->async_alloc_write(req->cheader_buf(), sisl::blob{}, req->data_sgs(), req, false /* part_of_batch */, tid); + // replicate this seal shard message to PG members (log-only, no data blocks); + repl_dev->async_alloc_write(req->cheader_buf(), sisl::blob{}, sisl::sg_list{}, req, false /* part_of_batch */, tid); return req->result().deferValue( [this, req, repl_dev, tid](const auto& result) -> ShardManager::AsyncResult< ShardInfo > { if (result.hasError()) { @@ -328,7 +381,6 @@ ShardManager::AsyncResult< ShardInfo > HSHomeObject::_seal_shard(ShardInfo const }); } -// TODO: introduce shard sealed_lsn to solve the conflict between seal_shard and put_blob, bool HSHomeObject::on_shard_message_pre_commit(int64_t lsn, sisl::blob const& header, sisl::blob const& key, cintrusive< homestore::repl_req_ctx >& hs_ctx) { repl_result_ctx< ShardManager::Result< ShardInfo > >* ctx{nullptr}; @@ -344,10 +396,21 @@ bool HSHomeObject::on_shard_message_pre_commit(int64_t lsn, sisl::blob const& he return false; } + +#ifdef _PRERELEASE + if (msg_header->msg_type == ReplicationMessageType::SEAL_SHARD_MSG) { + // Pause SEAL pre_commit at function entry, before state=SEALED. Test thread can race + // _put_blob while shard is still OPEN; sealed_lsn guard rejects the late blob on commit. + iomgr_flip::instance()->callback_flip("pause_seal_pre_commit"); + } +#endif + switch (msg_header->msg_type) { case ReplicationMessageType::SEAL_SHARD_MSG: { - auto sb = r_cast< shard_info_superblk const* >(header.cbytes() + sizeof(ReplicationMessageHeader)); - auto const shard_info = sb->info; + auto decoded_shard_sb = + decode_shard_sb(r_cast< const shard_info_superblk* >(header.cbytes() + sizeof(ReplicationMessageHeader))); + RELEASE_ASSERT(decoded_shard_sb.has_value(), "failed to decode shard superblk in pre_commit SEAL_SHARD_MSG"); + auto& [shard_info, p_chunk_id_unused, v_chunk_id_unused] = decoded_shard_sb.value(); { std::scoped_lock lock_guard(_shard_lock); @@ -391,31 +454,15 @@ void HSHomeObject::on_shard_message_rollback(int64_t lsn, sisl::blob const& head switch (msg_header->msg_type) { case ReplicationMessageType::CREATE_SHARD_MSG: { - if (ctx) { - ctx->promise_.setValue(folly::makeUnexpected(ShardError(ShardErrorCode::RETRY_REQUEST))); - } else { - // we have already added release_chunk logic to thenValue of hoemobject#create_shard in originator, so here - // we just need to release_chunk for non-originater case since it will bring a bug if a chunk is released - // for two times. for exampele, as a originator: - - // t1 : chunk1 is released in the rollback of create_shard, the chunk state is marked as available - // t2 : chunk1 is select by a new create shard (shard1), the chunk state is marked as inuse - // t3 : chunk1 is released in thenValue of create_shard, the chunk state is marked as available - // t4 : chunk1 is select by a new create shard (shard2), the chunk state is marked as inuse - // now, shard1 and shard2 hold the same chunk. - bool res = release_chunk_based_on_create_shard_message(header); - if (!res) { - RELEASE_ASSERT(false, - "shardID=0x{:x}, pg={}, shard=0x{:x}, failed to release chunk based on create shard msg", - msg_header->shard_id, (msg_header->shard_id >> homeobject::shard_width), - (msg_header->shard_id & homeobject::shard_mask)); - } - } + SLOGD(tid, msg_header->shard_id, "rollback create shard message, type={}, lsn= {}", msg_header->msg_type, lsn); + if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(ShardError(ShardErrorCode::RETRY_REQUEST))); } break; } case ReplicationMessageType::SEAL_SHARD_MSG: { - auto sb = r_cast< shard_info_superblk const* >(header.cbytes() + sizeof(ReplicationMessageHeader)); - auto const shard_info = sb->info; + auto decoded_shard_sb = + decode_shard_sb(r_cast< const shard_info_superblk* >(header.cbytes() + sizeof(ReplicationMessageHeader))); + RELEASE_ASSERT(decoded_shard_sb.has_value(), "failed to decode shard superblk in rollback SEAL_SHARD_MSG"); + auto& [shard_info, p_chunk_id_unused, v_chunk_id_unused] = decoded_shard_sb.value(); { std::scoped_lock lock_guard(_shard_lock); auto iter = _shard_map.find(shard_info.id); @@ -427,8 +474,10 @@ void HSHomeObject::on_shard_message_rollback(int64_t lsn, sisl::blob const& head // the update of superblk will be done in on_shard_message_commit; if (state == ShardInfo::State::SEALED) { state = ShardInfo::State::OPEN; + SLOGD(tid, shard_info.id, "rollback seal shard message, lsn={}", lsn); } else { - SLOGW(tid, shard_info.id, "try to rollback seal_shard message , but the shard state is not sealed"); + SLOGW(tid, shard_info.id, + "try to rollback seal_shard message , but the shard state is not sealed, lsn= {}", lsn); } } // TODO:set a proper error code @@ -442,9 +491,7 @@ void HSHomeObject::on_shard_message_rollback(int64_t lsn, sisl::blob const& head } } -void HSHomeObject::local_create_shard(ShardInfo shard_info, homestore::chunk_num_t v_chunk_id, - homestore::chunk_num_t p_chunk_id, homestore::blk_count_t blk_count, - trace_id_t tid) { +void HSHomeObject::local_create_shard(ShardInfo shard_info, homestore::chunk_num_t v_chunk_id, trace_id_t tid) { bool shard_exist = false; { scoped_lock lock_guard(_shard_lock); @@ -462,25 +509,13 @@ void HSHomeObject::local_create_shard(ShardInfo shard_info, homestore::chunk_num // comes and try to select chunk before the chunk is marked in_use, and at the same time gc kicks in (since the // chunk is still marked as available), then data loss will happen since gc is work on a chunk which is // accepting new blobs. - add_new_shard_to_map(std::make_unique< HS_Shard >(shard_info, p_chunk_id, v_chunk_id)); + add_new_shard_to_map(std::make_unique< HS_Shard >(shard_info, chunk->get_chunk_id(), v_chunk_id)); } else { - SLOGD(tid, shard_info.id, "shard already exist, skip creating shard"); + SLOGD(tid, shard_info.id, "shard already exist, this should happen in log replay case, skip creating shard"); } - - // update pg's total_occupied_blk_count - auto hs_pg = get_hs_pg(shard_info.placement_group); - RELEASE_ASSERT(hs_pg != nullptr, "shardID=0x{:x}, pg={}, shard=0x{:x}, PG not found", shard_info.id, - (shard_info.id >> homeobject::shard_width), (shard_info.id & homeobject::shard_mask)); - - SLOGD(tid, shard_info.id, "local_create_shard {}, vchunk_id={}, p_chunk_id={}, pg_id={}", shard_info.id, v_chunk_id, - p_chunk_id, shard_info.placement_group); - - const_cast< HS_PG* >(hs_pg)->durable_entities_update( - [blk_count](auto& de) { de.total_occupied_blk_count.fetch_add(blk_count, std::memory_order_relaxed); }); } -void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, homestore::MultiBlkId const& blkids, - shared< homestore::ReplDev > repl_dev, +void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, shared< homestore::ReplDev > repl_dev, cintrusive< homestore::repl_req_ctx >& hs_ctx) { repl_result_ctx< ShardManager::Result< ShardInfo > >* ctx{nullptr}; if (hs_ctx && hs_ctx->is_proposer()) { @@ -495,36 +530,30 @@ void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, hom return; } -#ifdef VADLIDATE_ON_REPLAY - sisl::io_blob_safe value_blob(blkids.blk_count() * repl_dev->get_blk_size(), io_align); - sisl::sg_list value_sgs; - value_sgs.iovs.emplace_back(iovec{.iov_base = value_blob.bytes(), .iov_len = value_blob.size()}); - value_sgs.size += value_blob.size(); - - // Do a read sync read and validate the crc - std::error_code err = repl_dev->async_read(blkids, value_sgs, value_blob.size()).get(); - if (err) { - LOGW("failed to read data from homestore blks, lsn={}", lsn); - if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(ShardError(ShardErrorCode::UNKNOWN))); } - return; - } - - if (crc32_ieee(init_crc32, value.cbytes(), value.size()) != header->payload_crc) { - // header & value is inconsistent; - LOGW("replication message header is inconsistent with value, lsn={}", lsn); - if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(ShardError(ShardErrorCode::CRC_MISMATCH))); } - return; +#ifdef _PRERELEASE + if (header->msg_type == ReplicationMessageType::SEAL_SHARD_MSG) { + // Wait for CREATE_SHARD (next log) to be in log store before SEAL releases its vchunk. + // Polled via get_last_append_lsn() so it doesn't rely on pre_commit signals. Armed on the + // repro follower only; no-op on leader and other followers. + iomgr_flip::instance()->callback_flip("wait_create_shard_in_log", lsn); + } else if (header->msg_type == ReplicationMessageType::CREATE_SHARD_MSG) { + // Pause CREATE_SHARD commit at function entry so GC can run in the race window. + iomgr_flip::instance()->callback_flip("pause_create_shard_commit"); } #endif + // !!!CRITICAL WARNING!!!: on commit should never alloc any blks to disk, which will introduce critical data + // override during log replay and cause data loss. So we stop writing Shard header/footer to disk anymore. + switch (header->msg_type) { case ReplicationMessageType::CREATE_SHARD_MSG: { - auto sb = r_cast< shard_info_superblk const* >(h.cbytes() + sizeof(ReplicationMessageHeader)); - auto shard_info = sb->info; - auto v_chunk_id = sb->v_chunk_id; - shard_info.lsn = lsn; + auto decoded_shard_sb = + decode_shard_sb(r_cast< const shard_info_superblk* >(h.cbytes() + sizeof(ReplicationMessageHeader))); + RELEASE_ASSERT(decoded_shard_sb.has_value(), "failed to decode shard superblk in commit CREATE_SHARD_MSG"); + auto& [shard_info, p_chunk_id_unused, v_chunk_id] = decoded_shard_sb.value(); + shard_info.create_lsn = lsn; - local_create_shard(shard_info, v_chunk_id, blkids.chunk_num(), blkids.blk_count(), tid); + local_create_shard(shard_info, v_chunk_id, tid); if (ctx) { ctx->promise_.setValue(ShardManager::Result< ShardInfo >(shard_info)); } SLOGD(tid, shard_info.id, "Commit done for creating shard"); @@ -533,8 +562,10 @@ void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, hom } case ReplicationMessageType::SEAL_SHARD_MSG: { - auto sb = r_cast< shard_info_superblk const* >(h.cbytes() + sizeof(ReplicationMessageHeader)); - auto const shard_info = sb->info; + auto decoded_shard_sb = + decode_shard_sb(r_cast< const shard_info_superblk* >(h.cbytes() + sizeof(ReplicationMessageHeader))); + RELEASE_ASSERT(decoded_shard_sb.has_value(), "failed to decode shard superblk in commit SEAL_SHARD_MSG"); + auto& [shard_info, p_chunk_id_unused, v_chunk_id_unused] = decoded_shard_sb.value(); ShardInfo::State state; { @@ -573,6 +604,7 @@ void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, hom // Before persisting shard state transition to SEALED, persist repl_dev's dc_lsn. // This guarantees all logs before SEAL_SHARD are replay-committed before GC starts. repl_dev->flush_durable_commit_lsn(); + shard_info.sealed_lsn = lsn; update_shard_in_map(shard_info); auto pg_id = shard_info.placement_group; @@ -582,15 +614,7 @@ void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, hom RELEASE_ASSERT(res, "Failed to release v_chunk_id={}, pg={}", v_chunkID.value(), pg_id); if (ctx) { ctx->promise_.setValue(ShardManager::Result< ShardInfo >(shard_info)); } - SLOGD(tid, shard_info.id, "Commit done for sealing shard"); - - auto hs_pg = get_hs_pg(shard_info.placement_group); - RELEASE_ASSERT(hs_pg != nullptr, "shardID=0x{:x}, pg={}, shard=0x{:x}, PG not found", shard_info.id, - (shard_info.id >> homeobject::shard_width), (shard_info.id & homeobject::shard_mask)); - const_cast< HS_PG* >(hs_pg)->durable_entities_update( - // shard_footer will also occupy one blk. - [](auto& de) { de.total_occupied_blk_count.fetch_add(1, std::memory_order_relaxed); }); - + SLOGD(tid, shard_info.id, "Commit done for sealing shard at lsn={}", lsn); break; } default: @@ -599,56 +623,39 @@ void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, hom } void HSHomeObject::on_shard_meta_blk_found(homestore::meta_blk* mblk, sisl::byte_view buf) { - // First peek at the version auto* header = reinterpret_cast< const DataHeader* >(buf.bytes()); + RELEASE_ASSERT(header->valid(), "Unknown shard superblock DataHeader magic={} version={}", header->magic, + header->version); homestore::superblk< shard_info_superblk > sb(_shard_meta_name); - // Detect and migrate old version data (DataHeader version 0x01) - if (header->version == 0x01) { - // Read data from buffer with old v1 layout - auto* old_sb = reinterpret_cast< const v1_shard_info_superblk* >(buf.bytes()); + auto* shard_sb = reinterpret_cast< const shard_info_superblk* >(buf.bytes()); + if (shard_sb->sb_version == 0x02) { + auto decoded_shard_sb = decode_shard_sb(shard_sb); + RELEASE_ASSERT(decoded_shard_sb.has_value(), "failed to decode v2 shard superblk"); + auto [v2_info, saved_p_chunk, saved_v_chunk] = decoded_shard_sb.value(); - const auto v1_info = old_sb->info; - const auto saved_p_chunk_id = old_sb->p_chunk_id; - const auto saved_v_chunk_id = old_sb->v_chunk_id; + LOGI("Detected v2 shard superblk (shard={}, pg={}, p_chunk={}, v_chunk={}), migrating to v3", v2_info.id, + v2_info.placement_group, saved_p_chunk, saved_v_chunk); - LOGI("Detected v1 shard superblk (shard={}, pg={}, p_chunk={}, v_chunk={}), migrating to v2", v1_info.id, - v1_info.placement_group, saved_p_chunk_id, saved_v_chunk_id); - - // Create a new v2 superblk with the given mblk sb.load(buf, mblk); sb.resize(sizeof(shard_info_superblk)); sb->magic = DataHeader::data_header_magic; sb->version = DataHeader::data_header_version; // 0x02 sb->type = DataHeader::data_type_t::SHARD_INFO; - sb->sb_version = shard_info_superblk::shard_sb_version; // 0x02 - - // Migrate v1_ShardInfo to v2 ShardInfo (v2 added meta field) - sb->info.id = v1_info.id; - sb->info.placement_group = v1_info.placement_group; - sb->info.state = v1_info.state; - sb->info.lsn = v1_info.lsn; - sb->info.created_time = v1_info.created_time; - sb->info.last_modified_time = v1_info.last_modified_time; - sb->info.available_capacity_bytes = v1_info.available_capacity_bytes; - sb->info.total_capacity_bytes = v1_info.total_capacity_bytes; - sb->info.current_leader = v1_info.current_leader; - // meta field is new in v2, initialize to empty - std::memset(sb->info.meta, 0, ShardInfo::meta_length); - - sb->p_chunk_id = saved_p_chunk_id; - sb->v_chunk_id = saved_v_chunk_id; + sb->sb_version = shard_info_superblk::shard_sb_version; // 0x03 + sb->p_chunk_id = saved_p_chunk; + sb->v_chunk_id = saved_v_chunk; + sb->info = v2_info; // Save shard_id and old mblk pointer for migration (cannot write or remove during callback due to // metasvc lock held - would cause deadlock) - shards_to_migrate_.push_back(v1_info.id); - - LOGI("Queued shard_id={} for migration write and old metablk removal after recovery", v1_info.id); - } else if (header->version == DataHeader::data_header_version) { + shards_to_migrate_.push_back(v2_info.id); + LOGI("Queued shard_id={} for v2->v3 migration write after recovery", v2_info.id); + } else if (shard_sb->sb_version == shard_info_superblk::shard_sb_version) { sb.load(buf, mblk); } else { - RELEASE_ASSERT(false, "Unknown shard superblock version {}", header->version); + RELEASE_ASSERT(false, "Unsupported shard superblock version {}", shard_sb->sb_version); } add_new_shard_to_map(std::make_unique< HS_Shard >(std::move(sb))); @@ -678,7 +685,7 @@ void HSHomeObject::write_migrated_shard_metablks() { // Write all migrated shards to disk // This is called AFTER read_sub_sb() returns to avoid deadlock with metasvc lock if (!shards_to_migrate_.empty()) { - LOGI("Writing {} migrated shard v2 superblocks", shards_to_migrate_.size()); + LOGI("Writing {} migrated shard v3 superblocks", shards_to_migrate_.size()); std::scoped_lock lock_guard(_shard_lock); for (auto& shard_id : shards_to_migrate_) { @@ -692,7 +699,7 @@ void HSHomeObject::write_migrated_shard_metablks() { try { hs_shard->sb_.write(); - LOGI("Successfully wrote migrated v2 shard superblk for shard_id={}", shard_id); + LOGI("Successfully wrote migrated v3 shard superblk for shard_id={}", shard_id); } catch (const std::exception& e) { LOGE("Failed to migrate shard_id={}: {}", shard_id, e.what()); // Continue with other shards even if one fails @@ -700,7 +707,7 @@ void HSHomeObject::write_migrated_shard_metablks() { } shards_to_migrate_.clear(); - LOGI("Completed migrating all shard superblocks from v1 to v2"); + LOGI("Completed migrating all shard superblocks from v2 to v3"); } } @@ -830,62 +837,6 @@ std::optional< homestore::chunk_num_t > HSHomeObject::get_shard_v_chunk_id(const return std::make_optional< homestore::chunk_num_t >(hs_shard->sb_->v_chunk_id); } -std::optional< homestore::chunk_num_t > HSHomeObject::resolve_v_chunk_id_from_msg(sisl::blob const& header) { - const ReplicationMessageHeader* msg_header = r_cast< const ReplicationMessageHeader* >(header.cbytes()); - if (msg_header->corrupted()) { - LOGW("replication message header is corrupted with crc error"); - return std::nullopt; - } - - switch (msg_header->msg_type) { - case ReplicationMessageType::CREATE_SHARD_MSG: { - const pg_id_t pg_id = msg_header->pg_id; - if (!pg_exists(pg_id)) { - LOGW("shardID=0x{:x}, pg={}, shard=0x{:x}, Requesting a chunk for an unknown pg={}", msg_header->shard_id, - (msg_header->shard_id >> homeobject::shard_width), (msg_header->shard_id & homeobject::shard_mask), - pg_id); - return std::nullopt; - } - auto sb = r_cast< shard_info_superblk const* >(header.cbytes() + sizeof(ReplicationMessageHeader)); - return sb->v_chunk_id; - } - default: { - LOGW("Unexpected message type encountered={}. This function should only be called with 'CREATE_SHARD_MSG'.", - msg_header->msg_type); - return std::nullopt; - } - } -} - -bool HSHomeObject::release_chunk_based_on_create_shard_message(sisl::blob const& header) { - const ReplicationMessageHeader* msg_header = r_cast< const ReplicationMessageHeader* >(header.cbytes()); - if (msg_header->corrupted()) { - LOGW("replication message header is corrupted with crc error"); - return false; - } - - switch (msg_header->msg_type) { - case ReplicationMessageType::CREATE_SHARD_MSG: { - const pg_id_t pg_id = msg_header->pg_id; - if (!pg_exists(pg_id)) { - LOGW("shardID=0x{:x}, pg={}, shard=0x{:x}, Requesting a chunk for an unknown pg={}", msg_header->shard_id, - (msg_header->shard_id >> homeobject::shard_width), (msg_header->shard_id & homeobject::shard_mask), - pg_id); - return false; - } - auto sb = r_cast< shard_info_superblk const* >(header.cbytes() + sizeof(ReplicationMessageHeader)); - bool res = chunk_selector_->release_chunk(sb->info.placement_group, sb->v_chunk_id); - if (!res) { LOGW("Failed to release chunk {} to pg={}", sb->v_chunk_id, sb->info.placement_group); } - return res; - } - default: { - LOGW("Unexpected message type encountered={}. This function should only be called with 'CREATE_SHARD_MSG'.", - msg_header->msg_type); - return false; - } - } -} - void HSHomeObject::destroy_shards(pg_id_t pg_id) { auto lg = std::scoped_lock(_pg_lock, _shard_lock); auto hs_pg = _get_hs_pg_unlocked(pg_id); diff --git a/src/lib/homestore_backend/legacy_resync_shard_data.fbs b/src/lib/homestore_backend/legacy_resync_shard_data.fbs new file mode 100644 index 00000000..81bea5d7 --- /dev/null +++ b/src/lib/homestore_backend/legacy_resync_shard_data.fbs @@ -0,0 +1,19 @@ +native_include "sisl/utility/non_null_ptr.hpp"; + +namespace homeobject; + +// Schema snapshot of ResyncShardMetaData before sealed_lsn was added. +// Used only in compatibility tests to produce old-format buffers. +table ResyncLegacyShardMetaData { + shard_id : uint64; + pg_id : uint16; + state : ubyte; + created_lsn : uint64; + created_time : uint64; + last_modified_time : ulong; + total_capacity_bytes : ulong; + vchunk_id : uint16; + meta: [ubyte]; +} + +root_type ResyncLegacyShardMetaData; diff --git a/src/lib/homestore_backend/pg_blob_iterator.cpp b/src/lib/homestore_backend/pg_blob_iterator.cpp index 19803b5d..dc58e805 100644 --- a/src/lib/homestore_backend/pg_blob_iterator.cpp +++ b/src/lib/homestore_backend/pg_blob_iterator.cpp @@ -24,14 +24,14 @@ HSHomeObject::PGBlobIterator::PGBlobIterator(HSHomeObject& home_obj, homestore:: if (upto_lsn != 0) { // Iterate all shards and its blobs which have lsn <= upto_lsn for (auto& shard : pg->shards_) { - if (shard->info.lsn <= upto_lsn) { + if (shard->info.create_lsn <= upto_lsn) { auto v_chunk_id = home_obj_.get_shard_v_chunk_id(shard->info.id); shard_list_.emplace_back(shard->info, v_chunk_id.value()); } } // Sort shard list by to ensure open shards positioned after sealed shards within each chunk std::ranges::sort(shard_list_, [](const ShardEntry& a, const ShardEntry& b) { - return a.v_chunk_num != b.v_chunk_num ? a.v_chunk_num < b.v_chunk_num : a.info.lsn < b.info.lsn; + return a.v_chunk_num != b.v_chunk_num ? a.v_chunk_num < b.v_chunk_num : a.info.create_lsn < b.info.create_lsn; }); } } @@ -215,8 +215,9 @@ bool HSHomeObject::PGBlobIterator::create_shard_snapshot_data(sisl::io_blob_safe auto shard = shard_list_[cur_shard_idx_]; std::vector< uint8_t > meta_bytes(shard.info.meta, shard.info.meta + ShardInfo::meta_length); auto shard_entry = CreateResyncShardMetaDataDirect( - builder_, shard.info.id, pg_id, static_cast< uint8_t >(shard.info.state), shard.info.lsn, - shard.info.created_time, shard.info.last_modified_time, shard.info.total_capacity_bytes, shard.v_chunk_num, &meta_bytes); + builder_, shard.info.id, pg_id, static_cast< uint8_t >(shard.info.state), shard.info.create_lsn, + shard.info.created_time, shard.info.last_modified_time, shard.info.total_capacity_bytes, shard.v_chunk_num, + &meta_bytes, shard.info.sealed_lsn); builder_.FinishSizePrefixed(shard_entry); diff --git a/src/lib/homestore_backend/replication_state_machine.cpp b/src/lib/homestore_backend/replication_state_machine.cpp index 81ec3e6d..838a4373 100644 --- a/src/lib/homestore_backend/replication_state_machine.cpp +++ b/src/lib/homestore_backend/replication_state_machine.cpp @@ -31,7 +31,7 @@ void ReplicationStateMachine::on_commit(int64_t lsn, const sisl::blob& header, c } case ReplicationMessageType::CREATE_SHARD_MSG: case ReplicationMessageType::SEAL_SHARD_MSG: { - home_object_->on_shard_message_commit(lsn, header, pbas[0], repl_dev(), ctx); + home_object_->on_shard_message_commit(lsn, header, repl_dev(), ctx); break; } @@ -181,8 +181,7 @@ void ReplicationStateMachine::on_error(ReplServiceError error, const sisl::blob& break; } case ReplicationMessageType::CREATE_SHARD_MSG: { - bool res = home_object_->release_chunk_based_on_create_shard_message(header); - if (!res) { LOGW("failed to release chunk based on create shard msg"); } + // Create shard is log only, no need to release chunk anymore, just return error to caller. auto result_ctx = boost::static_pointer_cast< repl_result_ctx< ShardManager::Result< ShardInfo > > >(ctx).get(); result_ctx->promise_.setValue(folly::makeUnexpected(toShardError(error))); break; @@ -211,49 +210,23 @@ ReplicationStateMachine::get_blk_alloc_hints(sisl::blob const& header, uint32_t cintrusive< homestore::repl_req_ctx >& hs_ctx) { const ReplicationMessageHeader* msg_header = r_cast< const ReplicationMessageHeader* >(header.cbytes()); switch (msg_header->msg_type) { - case ReplicationMessageType::CREATE_SHARD_MSG: { - pg_id_t pg_id = msg_header->pg_id; - // check whether the pg exists - if (!home_object_->pg_exists(pg_id)) { - LOGI("shardID=0x{:x}, pg={}, shard=0x{:x}, can not find pg={} when getting blk_alloc_hint", - msg_header->shard_id, (msg_header->shard_id >> homeobject::shard_width), - (msg_header->shard_id & homeobject::shard_mask), pg_id); - // TODO:: add error code to indicate the pg not found in homestore side - return folly::makeUnexpected(homestore::ReplServiceError::RESULT_NOT_EXIST_YET); - } - - auto v_chunkID = home_object_->resolve_v_chunk_id_from_msg(header); - if (!v_chunkID.has_value()) { - LOGW("shardID=0x{:x}, pg={}, shard=0x{:x}, can not resolve v_chunk_id from msg", msg_header->shard_id, - (msg_header->shard_id >> homeobject::shard_width), (msg_header->shard_id & homeobject::shard_mask)); - return folly::makeUnexpected(homestore::ReplServiceError::FAILED); - } - homestore::blk_alloc_hints hints; - // Both chunk_num_t and pg_id_t are of type uint16_t. - static_assert(std::is_same< pg_id_t, uint16_t >::value, "pg_id_t is not uint16_t"); - static_assert(std::is_same< homestore::chunk_num_t, uint16_t >::value, "chunk_num_t is not uint16_t"); - homestore::chunk_num_t v_chunk_id = v_chunkID.value(); - hints.application_hint = ((uint64_t)pg_id << 16) | v_chunk_id; - if (hs_ctx->is_proposer()) { hints.reserved_blks = home_object_->get_reserved_blks(); } - - auto tid = hs_ctx ? hs_ctx->traceID() : 0; - LOGD("tid={}, get_blk_alloc_hint for creating shard, select vchunk_id={} for pg={}, shardID={}", tid, - v_chunk_id, pg_id, msg_header->shard_id); - - return hints; - } - + case ReplicationMessageType::CREATE_SHARD_MSG: case ReplicationMessageType::SEAL_SHARD_MSG: { - auto p_chunkID = home_object_->get_shard_p_chunk_id(msg_header->shard_id); - if (!p_chunkID.has_value()) { - LOGW("shardID=0x{:x}, pg={}, shard=0x{:x}, shard does not exist, underlying engine will retry this later", - msg_header->shard_id, (msg_header->shard_id >> homeobject::shard_width), - (msg_header->shard_id & homeobject::shard_mask)); - return folly::makeUnexpected(homestore::ReplServiceError::RESULT_NOT_EXIST_YET); + if (data_size == 0) { + // New log-only format: should not reach here, but handle gracefully. + LOGW("get_blk_alloc_hints called for log-only shard message type={}, shard={}, pg={}", msg_header->msg_type, + msg_header->shard_id, msg_header->pg_id); + return homestore::blk_alloc_hints{}; + } else { + // Old format: shard message carried shard header/footer data blocks. Return committed_blk_id + // so HomeStore skips block allocation and data write. Shard on_commit will ignore pbas. + homestore::blk_alloc_hints hints; + hints.committed_blk_id = HSHomeObject::tombstone_pbas; + LOGW("get_blk_alloc_hints called for old shard message type={}, shard={}, pg={}, return committed_blk hint " + "to skip blk allocation", + msg_header->msg_type, msg_header->shard_id, msg_header->pg_id); + return hints; } - homestore::blk_alloc_hints hints; - hints.chunk_id_hint = p_chunkID.value(); - return hints; } case ReplicationMessageType::PUT_BLOB_MSG: @@ -645,40 +618,12 @@ folly::Future< std::error_code > ReplicationStateMachine::on_fetch_data(const in auto given_buffer = (uint8_t*)(sgs.iovs[0].iov_base); std::memset(given_buffer, 0, total_size); - - // in homeobject, we have three kinds of requests that will write data(thus fetch_data might happen) to a - // chunk: - // 1 create_shard : will write a shard header to a chunk - // 2 seal_shard : will write a shard footer to a chunk - // 3 put_blob: will write user data to a chunk + // in homeobject, we have only have one kind of requests that will write data(thus fetch_data might happen) to a + // chunk: put_blob: will write user data to a chunk // for any type that writes data to a chunk, we need to handle the fetch_data request for it. switch (msg_header->msg_type) { - case ReplicationMessageType::CREATE_SHARD_MSG: - case ReplicationMessageType::SEAL_SHARD_MSG: { - // this function only returns data, not care about raft related logic, so no need to check the existence of - // shard, just return the shard header/footer directly. Also, no need to read the data from disk, generate - // it from Header. - auto sb = - r_cast< HSHomeObject::shard_info_superblk const* >(header.cbytes() + sizeof(ReplicationMessageHeader)); - auto const raw_size = sizeof(HSHomeObject::shard_info_superblk); - auto const expected_size = sisl::round_up(raw_size, repl_dev()->get_blk_size()); - - RELEASE_ASSERT( - sgs.size == expected_size, - "shard metadata size does not match, lsn={}, msg_type={}, expected size={}, given buffer size={}", lsn, - msg_header->msg_type, expected_size, sgs.size); - - // TODO：：return error_code if assert fails, so it will not crash here because of the assert failure. - std::memcpy(given_buffer, sb, raw_size); - return folly::makeFuture< std::error_code >(std::error_code{}); - } - - // TODO: for shard header and footer, follower can generate it itself according to header, no need to fetch - // it from leader. this can been done by adding another callback, which will be called before follower tries - // to fetch data. - case ReplicationMessageType::PUT_BLOB_MSG: { const auto blob_id = msg_header->blob_id; @@ -883,24 +828,6 @@ void ReplicationStateMachine::on_no_space_left(homestore::repl_lsn_t lsn, sisl:: const pg_id_t pg_id = msg_header->pg_id; switch (msg_header->msg_type) { - // this case is only that no_space_left happens when writing shard header block on follower side. - case ReplicationMessageType::CREATE_SHARD_MSG: { - if (!home_object_->pg_exists(pg_id)) { - LOGW("shardID=0x{:x}, shard=0x{:x}, can not find pg={} when handling on_no_space_left", - msg_header->shard_id, (msg_header->shard_id & homeobject::shard_mask), pg_id); - } - auto v_chunkID = home_object_->resolve_v_chunk_id_from_msg(header); - if (!v_chunkID.has_value()) { - LOGW("shardID=0x{:x}, pg={}, shard=0x{:x}, can not resolve v_chunk_id from msg", msg_header->shard_id, - pg_id, (msg_header->shard_id & homeobject::shard_mask)); - } else { - chunk_id = home_object_->chunk_selector()->get_pg_vchunk(pg_id, v_chunkID.value())->get_chunk_id(); - } - - break; - } - - case ReplicationMessageType::SEAL_SHARD_MSG: case ReplicationMessageType::PUT_BLOB_MSG: { auto p_chunkID = home_object_->get_shard_p_chunk_id(msg_header->shard_id); if (!p_chunkID.has_value()) { @@ -911,7 +838,6 @@ void ReplicationStateMachine::on_no_space_left(homestore::repl_lsn_t lsn, sisl:: } else { chunk_id = p_chunkID.value(); } - break; } @@ -1038,8 +964,8 @@ void ReplicationStateMachine::on_log_replay_done(const homestore::group_id_t& gr if (shard_sb->info.is_open()) { const auto pg_id = shard_sb->info.placement_group; const auto vchunk_id = shard_sb->v_chunk_id; - auto chunk = chunk_selector->select_specific_chunk(pg_id, vchunk_id); - RELEASE_ASSERT(chunk != nullptr, "chunk selection failed with v_chunk_id={} in pg={}", vchunk_id, pg_id); + const auto exVchunk = chunk_selector->select_specific_chunk(pg_id, vchunk_id); + RELEASE_ASSERT(exVchunk != nullptr, "chunk selection failed with v_chunk_id={} in pg={}", vchunk_id, pg_id); LOGD("vchunk={} is selected for shard={} in pg={} when recovery", vchunk_id, shard_sb->info.id, pg_id); } } diff --git a/src/lib/homestore_backend/resync_shard_data.fbs b/src/lib/homestore_backend/resync_shard_data.fbs index 6fa761e3..713601f4 100644 --- a/src/lib/homestore_backend/resync_shard_data.fbs +++ b/src/lib/homestore_backend/resync_shard_data.fbs @@ -3,15 +3,16 @@ native_include "sisl/utility/non_null_ptr.hpp"; namespace homeobject; table ResyncShardMetaData { - shard_id : uint64; // shard id to be created with; + shard_id : uint64; // shard id to be created with; pg_id : uint16; // pg id which this shard belongs to; state : ubyte; // shard state; - created_lsn : uint64; // lsn on shard creation; - created_time : uint64; // shard creation time - last_modified_time : ulong; // shard last modify time - total_capacity_bytes : ulong; // total capacity of the shard - vchunk_id : uint16; // vchunk id + created_lsn : uint64; // lsn on shard creation; + created_time : uint64; // shard creation time + last_modified_time : ulong; // shard last modify time + total_capacity_bytes : ulong; // total capacity of the shard + vchunk_id : uint16; // vchunk id meta: [ubyte]; // serialized shard meta data + sealed_lsn : uint64 = 9223372036854775807; // lsn on shard sealing } //ShardMetaData schema is the first batch(batch=0) in the shard transmission diff --git a/src/lib/homestore_backend/snapshot_receive_handler.cpp b/src/lib/homestore_backend/snapshot_receive_handler.cpp index 7c1fdcb3..bd3ab3be 100644 --- a/src/lib/homestore_backend/snapshot_receive_handler.cpp +++ b/src/lib/homestore_backend/snapshot_receive_handler.cpp @@ -79,7 +79,8 @@ int HSHomeObject::SnapshotReceiveHandler::process_shard_snapshot_data(ResyncShar shard_sb->info.id = shard_meta.shard_id(); shard_sb->info.placement_group = shard_meta.pg_id(); shard_sb->info.state = static_cast< ShardInfo::State >(shard_meta.state()); - shard_sb->info.lsn = shard_meta.created_lsn(); + shard_sb->info.create_lsn = shard_meta.created_lsn(); + shard_sb->info.sealed_lsn = shard_meta.sealed_lsn(); shard_sb->info.created_time = shard_meta.created_time(); shard_sb->info.last_modified_time = shard_meta.last_modified_time(); shard_sb->info.available_capacity_bytes = shard_meta.total_capacity_bytes(); @@ -89,56 +90,8 @@ int HSHomeObject::SnapshotReceiveHandler::process_shard_snapshot_data(ResyncShar std::memcpy(shard_sb->info.meta, shard_meta.meta()->Data(), ShardInfo::meta_length); } shard_sb->v_chunk_id = shard_meta.vchunk_id(); - - homestore::blk_alloc_hints hints; - hints.application_hint = static_cast< uint64_t >(ctx_->pg_id) << 16 | shard_sb->v_chunk_id; - - homestore::MultiBlkId blk_id; - auto status = homestore::data_service().alloc_blks( - sisl::round_up(aligned_buf.size(), homestore::data_service().get_blk_size()), hints, blk_id); - if (status != homestore::BlkAllocStatus::SUCCESS) { - LOGE("Failed to allocate blocks for shardID=0x{:x}, pg={}, shard=0x{:x}", shard_meta.shard_id(), - (shard_meta.shard_id() >> homeobject::shard_width), (shard_meta.shard_id() & homeobject::shard_mask)); - return ALLOC_BLK_ERR; - } - shard_sb->p_chunk_id = blk_id.to_single_blkid().chunk_num(); - - auto free_allocated_blks = [blk_id]() { - homestore::data_service().async_free_blk(blk_id).thenValue([blk_id](auto&& err) { - LOGD("Freed blk_id={} due to failure in persisting shard info, err {}", blk_id.to_string(), - err ? err.message() : "nil"); - }); - }; - -#ifdef _PRERELEASE - if (iomgr_flip::instance()->test_flip("snapshot_receiver_shard_write_data_error")) { - LOGW("Simulating shard snapshot write data error"); - free_allocated_blks(); - return WRITE_DATA_ERR; - } -#endif - const auto ret = homestore::data_service() - .async_write(r_cast< char const* >(aligned_buf.cbytes()), aligned_buf.size(), blk_id) - .thenValue([&blk_id](auto&& err) -> BlobManager::AsyncResult< blob_id_t > { - // TODO: do we need to update repl_dev metrics? - if (err) { - LOGE("Failed to write shard info to blk_id={}", blk_id.to_string()); - return folly::makeUnexpected(BlobError(BlobErrorCode::REPLICATION_ERROR)); - } - LOGD("Shard info written to blk_id={}", blk_id.to_string()); - return 0; - }) - .get(); - if (ret.hasError()) { - LOGE("Failed to write shard info of shardID=0x{:x}, pg={}, shard=0x{:x} to blk_id={}", shard_meta.shard_id(), - (shard_meta.shard_id() >> homeobject::shard_width), (shard_meta.shard_id() & homeobject::shard_mask), - blk_id.to_string()); - free_allocated_blks(); - return WRITE_DATA_ERR; - } - // Now let's create local shard - home_obj_.local_create_shard(shard_sb->info, shard_sb->v_chunk_id, shard_sb->p_chunk_id, blk_id.blk_count()); + home_obj_.local_create_shard(shard_sb->info, shard_sb->v_chunk_id); ctx_->shard_cursor = shard_meta.shard_id(); ctx_->cur_batch_num = 0; return 0; diff --git a/src/lib/homestore_backend/tests/homeobj_fixture.hpp b/src/lib/homestore_backend/tests/homeobj_fixture.hpp index 499968ab..95e71971 100644 --- a/src/lib/homestore_backend/tests/homeobj_fixture.hpp +++ b/src/lib/homestore_backend/tests/homeobj_fixture.hpp @@ -49,7 +49,7 @@ class HomeObjectFixture : public ::testing::Test { HSHomeObject::_hs_chunk_size = SISL_OPTIONS["chunk_size"].as< uint64_t >() * Mi; _obj_inst = std::dynamic_pointer_cast< HSHomeObject >(g_helper->build_new_homeobject()); - + // Used to export metrics, it should be called after init_homeobject if (SISL_OPTIONS["enable_http"].as< bool >()) { g_helper->app->start_http_server(); } if (!g_helper->is_current_testcase_restarted()) { @@ -607,7 +607,7 @@ class HomeObjectFixture : public ::testing::Test { EXPECT_EQ(lhs.id, rhs.id); EXPECT_EQ(lhs.placement_group, rhs.placement_group); EXPECT_EQ(lhs.state, rhs.state); - EXPECT_EQ(lhs.lsn, rhs.lsn); + EXPECT_EQ(lhs.create_lsn, rhs.create_lsn); EXPECT_EQ(lhs.created_time, rhs.created_time); EXPECT_EQ(lhs.last_modified_time, rhs.last_modified_time); EXPECT_EQ(lhs.available_capacity_bytes, rhs.available_capacity_bytes); @@ -785,12 +785,16 @@ class HomeObjectFixture : public ::testing::Test { // wait for the last blob to be created locally, which means all the blob before this blob are created void wait_for_blob(shard_id_t shard_id, blob_id_t blob_id) { + static constexpr auto k_blob_timeout = std::chrono::minutes(10); + auto deadline = std::chrono::steady_clock::now() + k_blob_timeout; while (true) { if (blob_exist(shard_id, blob_id)) { LOGINFO("shard {} blob {} is created locally, which means all the blob before {} are created", shard_id, blob_id, blob_id); return; } + RELEASE_ASSERT(std::chrono::steady_clock::now() < deadline, + "wait_for_blob timed out after 5min: shard={} blob={}", shard_id, blob_id); std::this_thread::sleep_for(1s); } } diff --git a/src/lib/homestore_backend/tests/homeobj_misc_tests.cpp b/src/lib/homestore_backend/tests/homeobj_misc_tests.cpp index f92ee03d..ac644639 100644 --- a/src/lib/homestore_backend/tests/homeobj_misc_tests.cpp +++ b/src/lib/homestore_backend/tests/homeobj_misc_tests.cpp @@ -64,7 +64,7 @@ TEST_F(HomeObjectFixture, PGBlobIterator) { seal_shard(pg->shards_.front()->info.id); ASSERT_EQ(pg->shards_.front()->info.state, homeobject::ShardInfo::State::SEALED); // Filter out the last shard - auto snp_lsn = pg->shards_.back()->info.lsn - 1; + auto snp_lsn = pg->shards_.back()->info.create_lsn - 1; // Delete some blobs: delete the first blob of each shard blob_id_t current_blob_id{0}; for (auto& shard : shard_list) { @@ -108,7 +108,7 @@ TEST_F(HomeObjectFixture, PGBlobIterator) { auto idx = 0; ASSERT_EQ(pg->shards_.size() - 1, pg_msg->shard_ids()->size()); for (auto& shard : pg->shards_) { - if (shard->info.lsn > snp_lsn) { continue; } + if (shard->info.create_lsn > snp_lsn) { continue; } ASSERT_EQ(shard->info.id, pg_msg->shard_ids()->Get(idx++)); } @@ -118,7 +118,7 @@ TEST_F(HomeObjectFixture, PGBlobIterator) { auto shard_seq_num = HSHomeObject::get_sequence_num_from_shard_id(shard->info.id); auto batch_id = 0; objId oid(shard_seq_num, batch_id++); - if (shard->info.lsn > snp_lsn) { + if (shard->info.create_lsn > snp_lsn) { ASSERT_FALSE(pg_iter->update_cursor(oid)); continue; } @@ -140,7 +140,8 @@ TEST_F(HomeObjectFixture, PGBlobIterator) { ASSERT_EQ(shard_msg->shard_id(), shard->info.id); ASSERT_EQ(shard_msg->pg_id(), pg->pg_info_.id); ASSERT_EQ(shard_msg->state(), static_cast< uint8_t >(shard->info.state)); - ASSERT_EQ(shard_msg->created_lsn(), shard->info.lsn); + ASSERT_EQ(shard_msg->created_lsn(), shard->info.create_lsn); + ASSERT_EQ(shard_msg->sealed_lsn(), shard->info.sealed_lsn); ASSERT_EQ(shard_msg->created_time(), shard->info.created_time); ASSERT_EQ(shard_msg->last_modified_time(), shard->info.last_modified_time); ASSERT_EQ(shard_msg->total_capacity_bytes(), shard->info.total_capacity_bytes); @@ -214,7 +215,7 @@ TEST_F(HomeObjectFixture, PGBlobIteratorGCMoveDetection) { auto pg = _obj_inst->get_hs_pg(pg_id); ASSERT_TRUE(pg != nullptr); - auto snp_lsn = pg->shards_.back()->info.lsn; + auto snp_lsn = pg->shards_.back()->info.create_lsn; auto pg_iter = std::make_shared< HSHomeObject::PGBlobIterator >(*_obj_inst, pg->pg_info_.replica_set_uuid, snp_lsn); ASSERT_EQ(pg_iter->shard_list_.size(), 2u); pg_iter->max_batch_size_ = 1 * Mi; @@ -280,7 +281,7 @@ TEST_F(HomeObjectFixture, PGBlobIteratorGCTombstoneDetection) { auto pg = _obj_inst->get_hs_pg(pg_id); ASSERT_TRUE(pg != nullptr); - auto snp_lsn = pg->shards_.back()->info.lsn; + auto snp_lsn = pg->shards_.back()->info.create_lsn; auto pg_iter = std::make_shared< HSHomeObject::PGBlobIterator >(*_obj_inst, pg->pg_info_.replica_set_uuid, snp_lsn); pg_iter->max_batch_size_ = 1 * Mi; @@ -385,16 +386,19 @@ TEST_F(HomeObjectFixture, SnapshotReceiveHandler) { shard.created_time = get_time_since_epoch_ms(); shard.last_modified_time = shard.created_time; shard.total_capacity_bytes = 1024 * Mi; - shard.lsn = snp_lsn; + shard.create_lsn = snp_lsn; auto meta_str = "shard meta:" + std::to_string(i); - std::memcpy(shard.meta, meta_str.c_str(),meta_str.length()); + std::memcpy(shard.meta, meta_str.c_str(), meta_str.length()); shard.meta[meta_str.size()] = '\0'; - auto v_chunk_id = _obj_inst->chunk_selector()->get_most_available_blk_chunk(shard.id, pg_id); - - auto shard_entry = CreateResyncShardMetaData(builder, shard.id, pg_id, static_cast< uint8_t >(shard.state), - shard.lsn, shard.created_time, shard.last_modified_time, - shard.total_capacity_bytes, v_chunk_id.value()); + auto exVchunk = _obj_inst->chunk_selector()->pick_most_available_blk_chunk(shard.id, pg_id); + RELEASE_ASSERT(exVchunk != nullptr, "chunk selection failed with v_chunk_id={} in pg={}", shard.id, pg_id); + RELEASE_ASSERT(exVchunk->m_v_chunk_id.has_value(), "v_chunk_id should have value for selected chunk for pg={}", + pg_id); + auto shard_entry = + CreateResyncShardMetaData(builder, shard.id, pg_id, static_cast< uint8_t >(shard.state), shard.create_lsn, + shard.created_time, shard.last_modified_time, shard.total_capacity_bytes, + exVchunk->m_v_chunk_id.value(), 0 /* meta */, shard.sealed_lsn); builder.Finish(shard_entry); auto shard_meta = GetResyncShardMetaData(builder.GetBufferPointer()); auto status = handler->process_shard_snapshot_data(*shard_meta); @@ -412,7 +416,8 @@ TEST_F(HomeObjectFixture, SnapshotReceiveHandler) { ASSERT_EQ(shard_res.created_time, shard.created_time); ASSERT_EQ(shard_res.last_modified_time, shard.last_modified_time); ASSERT_EQ(shard_res.total_capacity_bytes, shard.total_capacity_bytes); - ASSERT_EQ(shard_res.lsn, shard.lsn); + ASSERT_EQ(shard_res.create_lsn, shard.create_lsn); + ASSERT_EQ(shard_res.sealed_lsn, shard.sealed_lsn); // Step 2-2: Test write blob batch data // Generate ResyncBlobDataBatch message @@ -431,7 +436,8 @@ TEST_F(HomeObjectFixture, SnapshotReceiveHandler) { // Construct raw blob buffer auto blob = build_blob(cur_blob_id); - const auto aligned_hdr_size = sisl::round_up(sizeof(HSHomeObject::BlobHeader), _obj_inst->_data_block_size); + const auto aligned_hdr_size = + sisl::round_up(sizeof(HSHomeObject::BlobHeader), _obj_inst->_data_block_size); sisl::io_blob_safe blob_raw(aligned_hdr_size + blob.body.size(), io_align); HSHomeObject::BlobHeader hdr; hdr.type = HSHomeObject::DataHeader::data_type_t::BLOB_INFO; @@ -506,7 +512,7 @@ TEST_F(HomeObjectFixture, SnapshotReceiveHandler) { if (shard.state == ShardInfo::State::SEALED) { auto v = _obj_inst->get_shard_v_chunk_id(shard.id); ASSERT_TRUE(v.has_value()); - ASSERT_EQ(v.value(), v_chunk_id.value()); + ASSERT_EQ(v.value(), exVchunk->m_v_chunk_id.value()); ASSERT_TRUE(_obj_inst->chunk_selector()->is_chunk_available(pg_id, v.value())); } } diff --git a/src/lib/homestore_backend/tests/hs_gc_tests.cpp b/src/lib/homestore_backend/tests/hs_gc_tests.cpp index b13be702..450ff101 100644 --- a/src/lib/homestore_backend/tests/hs_gc_tests.cpp +++ b/src/lib/homestore_backend/tests/hs_gc_tests.cpp @@ -74,7 +74,7 @@ TEST_F(HomeObjectFixture, BasicGC) { uint64_t total_blob_occupied_blk_count{0}; const auto& shard_vec = pg_shard_id_vec[pg_id]; for (const auto& shard_id : shard_vec) { - total_blob_occupied_blk_count += 2; /*header and footer*/ + // shard create/seal is log-only; no header/footer blocks written for (const auto& [_, blk_count] : shard_blob_ids_map[shard_id]) { total_blob_occupied_blk_count += blk_count; } @@ -115,7 +115,7 @@ TEST_F(HomeObjectFixture, BasicGC) { ASSERT_TRUE(chunk_opt.has_value()); auto chunk_id = chunk_opt.value(); // now, the chunk state is not determined, maybe GC(being gc) or AVAILABLE(complete gc), skip checking it. - uint32_t used_blks{2}; /* header and footer */ + uint32_t used_blks{0}; for (const auto& [_, blk_count] : blob_to_blk_count) { used_blks += blk_count; @@ -190,7 +190,6 @@ TEST_F(HomeObjectFixture, BasicGC) { uint64_t total_blob_occupied_blk_count{0}; const auto& shard_vec = pg_shard_id_vec[pg_id]; for (const auto& shard_id : shard_vec) { - total_blob_occupied_blk_count += 2; /*header and footer*/ for (const auto& [_, blk_count] : shard_blob_ids_map[shard_id]) { total_blob_occupied_blk_count += blk_count; } @@ -239,7 +238,6 @@ TEST_F(HomeObjectFixture, BasicGC) { uint64_t total_blob_occupied_blk_count{0}; const auto& shard_vec = pg_shard_id_vec[pg_id]; for (const auto& shard_id : shard_vec) { - total_blob_occupied_blk_count += 2; /*header and footer*/ for (const auto& [_, blk_count] : shard_blob_ids_map[shard_id]) { total_blob_occupied_blk_count += blk_count; } @@ -625,13 +623,10 @@ void HomeObjectFixture::EmergentGC(bool with_crash_recovery) { uint64_t total_blob_occupied_blk_count{0}; const auto& shard_vec = pg_shard_id_vec[pg_id]; for (const auto& shard_id : shard_vec) { - total_blob_occupied_blk_count += 2; /*header and footer*/ for (const auto& [_, blk_count] : shard_blob_ids_map[shard_id]) { total_blob_occupied_blk_count += blk_count; } } - // for each chunk, we have an open shard, which has only header. - total_blob_occupied_blk_count -= pg_chunk_nums[pg_id]; ASSERT_EQ(hs_pg->pg_sb_->total_occupied_blk_count, total_blob_occupied_blk_count); ASSERT_EQ(hs_pg->durable_entities().total_occupied_blk_count, total_blob_occupied_blk_count); @@ -683,13 +678,10 @@ void HomeObjectFixture::EmergentGC(bool with_crash_recovery) { uint64_t total_blob_occupied_blk_count{0}; const auto& shard_vec = pg_shard_id_vec[pg_id]; for (const auto& shard_id : shard_vec) { - total_blob_occupied_blk_count += 2; /*header and footer*/ for (const auto& [_, blk_count] : shard_blob_ids_map[shard_id]) { total_blob_occupied_blk_count += blk_count; } } - // for each chunk, we have an open shard, which has only header. - total_blob_occupied_blk_count -= pg_chunk_nums[pg_id]; ASSERT_EQ(hs_pg->pg_sb_->total_occupied_blk_count, total_blob_occupied_blk_count); ASSERT_EQ(hs_pg->durable_entities().total_occupied_blk_count, total_blob_occupied_blk_count); @@ -726,25 +718,23 @@ void HomeObjectFixture::EmergentGC(bool with_crash_recovery) { futs.clear(); - // for each chunk in this pg, there is only one shard header + // for each chunk in this pg, all blobs are gone (no header/footer since shard create is log-only) for (const auto& [pg_id, chunk_num] : pg_chunk_nums) { const auto pg_chunks = chunk_selector->get_pg_chunks(pg_id); for (uint64_t i{0}; i < chunk_num; i++) { auto chunk_id = pg_chunks->at(i); auto EXVchunk = chunk_selector->get_extend_vchunk(chunk_id); - // the open shard is not sealed, so there is only the shard header for each shard - ASSERT_EQ(EXVchunk->get_used_blks(), 1); + ASSERT_EQ(EXVchunk->get_used_blks(), 0); } } // check vchunk to pchunk for every pg for (const auto& [pg_id, shard_vec] : pg_shard_id_vec) { auto& hs_pg = HS_PG_map[pg_id]; - // check pg durable entities. only shard header left, and every chunk has a open shard, so - // total_occupied_blk_count is equal to the num of chunks in this pg since each chunk has a shard header. - ASSERT_EQ(hs_pg->pg_sb_->total_occupied_blk_count, pg_chunk_nums[pg_id]); - ASSERT_EQ(hs_pg->durable_entities().total_occupied_blk_count, pg_chunk_nums[pg_id]); + // all blobs deleted and gc'd; no header/footer blks since shard create is log-only + ASSERT_EQ(hs_pg->pg_sb_->total_occupied_blk_count, 0); + ASSERT_EQ(hs_pg->durable_entities().total_occupied_blk_count, 0); // after all blobs have been deleted, the pg index table should be empty ASSERT_EQ(get_valid_blob_count_in_pg(pg_id), 0); @@ -857,4 +847,447 @@ TEST_F(HomeObjectFixture, GCTaskPbaChunkCheck) { ASSERT_TRUE(gc_mgr->submit_gc_task(task_priority::normal, cur_chunk).get()) << "normal gc should succeed after restoring correct blob pba"; -} \ No newline at end of file +} + +// =================================================================================================== +// StalePChunkRouteAfterGC: CREATE_SHARD stale pchunk race (GC / shard-blob route +// inconsistency). +// +// EXACT SCENARIO (chunks_per_pg == 1 forces shard1 and shard2 onto the SAME vchunk N): +// +// ① CREATE_SHARD2 log is already present in the log store on the laggy follower. +// (The leader issued seal shard1 then create shard 2; raft appended both logs +// before commit_index advanced, so the follower sees CREATE_SHARD2 log before SEAL_SHARD1 +// has committed.) +// ② SEAL_SHARD1 on_commit runs to completion: +// release_chunk(vchunk_N) → vchunk_N becomes AVAILABLE, pchunk is still A. +// ③ [gate fires] GC runs a normal relocation of pchunk A → B. +// vchunk_N live pchunk becomes B. pchunk_A is now an orphaned reserved chunk. +// ④ CREATE_SHARD2 on_commit resumes: +// alloc_blks(application_hint = vchunk_N) +// → must resolve the LIVE pchunk B, NOT the stale A. +// +// On UNFIXED code the alloc in step ④ would see vchunk AVAILABLE and +// grab the current pchunk at that instant; if pchunk_B had not been resolved yet it would get A. +// With the current alloc_blks call happening AFTER the gate resumes (post-GC), B is the live +// pchunk and the test verifies p_chunk(shard2) == live_pchunk(vchunk_N). +// +// MUST be run with --chunks_per_pg=1 so the successor shard is forced to reuse the predecessor vchunk. +// +// =================================================================================================== +#ifdef _PRERELEASE +TEST_F(HomeObjectFixture, StalePChunkRouteAfterGC) { + const pg_id_t pg_id = 1; + const auto num_blobs_per_shard = SISL_OPTIONS["num_blobs"].as< uint64_t >(); + + ASSERT_EQ(SISL_OPTIONS["chunks_per_pg"].as< uint64_t >(), 1u) + << "This reproduction must be run with --chunks_per_pg=1 to force vchunk reuse by the successor shard"; + + create_pg(pg_id); + auto chunk_selector = _obj_inst->chunk_selector(); + + if (!am_i_in_pg(pg_id)) { + // not a member, just keep the sync barriers aligned and leave. + g_helper->sync(); // arm barrier + g_helper->sync(); // end barrier + return; + } + + // The laggy follower is the non-leader replica number 2 (leader defaults to replica 0). + const bool i_am_leader = (g_helper->my_replica_id() == get_leader_id(pg_id)); + const bool i_am_repro_follower = (!i_am_leader) && (g_helper->replica_num() == 2); + + std::mutex repro1_mtx; + std::condition_variable repro1_cv; + std::atomic< bool > repro1_blocked{false}; + std::atomic< bool > repro1_released{false}; + + // ---- shard1: create and fill with blobs, then delete half to create garbage for normal GC ---- + auto shard1 = create_shard(pg_id, 64 * Mi, "shard1"); + ASSERT_NE(shard1.id, 0u); + + std::map< pg_id_t, std::vector< shard_id_t > > shards{{pg_id, {shard1.id}}}; + std::map< pg_id_t, blob_id_t > pg_blob_id{{pg_id, 0}}; + put_blobs(shards, num_blobs_per_shard, pg_blob_id); + + // Delete half the blobs so pchunk_A has garbage that triggers normal GC (gc_garbage_rate_threshold=0). + // This is the realistic production trigger: GC fires because the chunk has freed space. + { + std::map< shard_id_t, std::set< blob_id_t > > to_delete; + for (blob_id_t b = 0; b < num_blobs_per_shard / 2; ++b) + to_delete[shard1.id].insert(b); + del_blobs(pg_id, to_delete); + } + + // record this replica's local vchunk N and pchunk A for shard1 + auto vchunk_N = _obj_inst->get_shard_v_chunk_id(shard1.id); + auto pchunk_A = _obj_inst->get_shard_p_chunk_id(shard1.id); + ASSERT_TRUE(vchunk_N.has_value()); + ASSERT_TRUE(pchunk_A.has_value()); + + // ---- arm the repro flips on exactly one follower so quorum (leader + other follower) is unaffected ---- + if (i_am_repro_follower) { + auto repl_dev = _obj_inst->get_hs_pg(pg_id)->repl_dev_; + auto dont_care = m_fc.create_condition("", flip::Operator::DONT_CARE, (int)0); + flip::FlipFrequency freq; + freq.set_count(1); + freq.set_percent(100); + + // Flip 1: in SEAL_SHARD1 commit — spin until CREATE_SHARD2 log is in the log store before + // release_chunk runs. Explicit guarantee that the race window actually exists. + m_fc.inject_callback_flip< void, int64_t >( + "wait_create_shard_in_log", {dont_care}, freq, + std::function< void(int64_t) >([&, repl_dev](int64_t seal_lsn) { + while (repl_dev->get_last_append_lsn() <= seal_lsn) { + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + LOGI("[StalePChunkRouteAfterGC] CREATE_SHARD2 in log store (last_append_lsn={} > seal_lsn={})", + repl_dev->get_last_append_lsn(), seal_lsn); + })); + + // Flip 2: in CREATE_SHARD2 commit — pause before local_create_shard so GC can run in the race window. + // NOTE: do NOT spin inside a commit callback (commit_ext runs on the iomgr I/O thread; + // spinning there blocks log-append processing and causes deadlock in single-threaded executors). + m_fc.inject_callback_flip< void >("pause_create_shard_commit", {dont_care}, freq, + std::function< void() >([&]() { + LOGI("[StalePChunkRouteAfterGC] pausing CREATE_SHARD commit"); + std::unique_lock< std::mutex > lk(repro1_mtx); + repro1_blocked.store(true); + repro1_cv.notify_all(); + repro1_cv.wait(lk, [&] { return repro1_released.load(); }); + LOGI("[StalePChunkRouteAfterGC] resuming CREATE_SHARD commit"); + })); + LOGINFO("[StalePChunkRouteAfterGC] armed on follower replica={}, pg={}, vchunk={}, pchunk_A={}", + g_helper->replica_num(), pg_id, vchunk_N.value(), pchunk_A.value()); + } + + g_helper->sync(); // make sure the hook is armed before the leader drives seal+create + + // ---- leader drives seal(shard1) then create(shard2) back-to-back, WITHOUT per-op sync barriers ---- + shard_id_t shard2_id = INVALID_UINT64_ID; + run_on_pg_leader(pg_id, [&]() { + auto tid = generateRandomTraceId(); + auto sealed = _obj_inst->shard_manager()->seal_shard(shard1.id, tid).get(); + RELEASE_ASSERT(!!sealed, "failed to seal shard1"); + auto created = _obj_inst->shard_manager()->create_shard(pg_id, 64 * Mi, "shard2", tid).get(); + RELEASE_ASSERT(!!created, "failed to create shard2"); + g_helper->set_uint64_id(created.value().id); + LOGINFO("[StalePChunkRouteAfterGC] leader sealed shard1=0x{:x} and created shard2=0x{:x}", shard1.id, + created.value().id); + }); + + // everyone learns shard2 id from IPC + while ((shard2_id = g_helper->get_uint64_id()) == INVALID_UINT64_ID) { + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + + // ---- the laggy follower: exact reproduction of the production race ---- + // At this point raft commit ordering guarantees: + // SEAL_SHARD1 committed first → vchunk_N is AVAILABLE, pchunk still A + // CREATE_SHARD2 commit is now queued, paused at the gate (before alloc_blks) + // Normal GC fires because pchunk_A has garbage (deleted blobs), relocates A -> B. + // Then the gate releases and alloc_blks resolves the live pchunk B. + if (i_am_repro_follower) { + { + std::unique_lock< std::mutex > lk(repro1_mtx); + ASSERT_TRUE(repro1_cv.wait_for(lk, std::chrono::seconds(120), [&] { return repro1_blocked.load(); })) + << "CREATE_SHARD2 commit was never paused on the repro follower"; + } + LOGINFO("[StalePChunkRouteAfterGC] follower replica={} sees CREATE_SHARD2 paused; " + "vchunk={} is AVAILABLE (seal done), pchunk_A={}, running normal GC to remap A -> B", + g_helper->replica_num(), vchunk_N.value(), pchunk_A.value()); + + // Normal GC: chunk has garbage from deleted blobs (gc_garbage_rate_threshold=0 in the CTest entry). + auto fut = _obj_inst->gc_manager()->submit_gc_task(task_priority::normal, pchunk_A.value()); + bool gc_ok = std::move(fut).get(); + ASSERT_TRUE(gc_ok) << "normal GC on pchunk=" << pchunk_A.value() << " failed"; + + // release the gate: alloc_blks runs and resolves live pchunk B. + { + std::unique_lock< std::mutex > lk(repro1_mtx); + repro1_released.store(true); + repro1_cv.notify_all(); + } + } + + // wait for shard2 to be created locally on every member. + while (!_obj_inst->shard_manager()->get_shard(shard2_id, 0).get()) { + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + + g_helper->sync(); + + // ---- verification: shard2's recorded p_chunk must match the live vchunk->pchunk mapping on EVERY replica ---- + auto v2 = _obj_inst->get_shard_v_chunk_id(shard2_id); + auto p2 = _obj_inst->get_shard_p_chunk_id(shard2_id); + ASSERT_TRUE(v2.has_value()); + ASSERT_TRUE(p2.has_value()); + ASSERT_EQ(v2.value(), vchunk_N.value()) + << "successor shard2 did not reuse shard1's vchunk (need --chunks_per_pg=1)"; + + auto pg_chunks = chunk_selector->get_pg_chunks(pg_id); + ASSERT_TRUE(pg_chunks != nullptr); + auto live_pchunk = pg_chunks->at(v2.value()); + + LOGINFO("[StalePChunkRouteAfterGC] replica={} shard2 vchunk={} stored_p_chunk={} live_p_chunk={} (original " + "pchunk_A={})", + g_helper->replica_num(), v2.value(), p2.value(), live_pchunk, pchunk_A.value()); + + EXPECT_EQ(p2.value(), live_pchunk) << "shard2 on replica " << static_cast< int >(g_helper->replica_num()) + << " is routed to pchunk " << p2.value() + << " but the live vchunk->pchunk mapping is " << live_pchunk + << " (stale shard/blob route; see PG 39 / PG 3409)"; + + if (i_am_repro_follower) { + EXPECT_NE(live_pchunk, pchunk_A.value()) + << "expected GC to have relocated vchunk " << v2.value() << " off its original pchunk " << pchunk_A.value() + << " (the race window was not actually exercised)"; + + auto old_chunk = chunk_selector->get_extend_vchunk(pchunk_A.value()); + ASSERT_TRUE(old_chunk != nullptr); + EXPECT_FALSE(old_chunk->m_pg_id.has_value()) + << "the original pchunk " << pchunk_A.value() << " should be an orphaned reserved chunk after GC remap"; + + LOGINFO( + "[StalePChunkRouteAfterGC] fix verified on laggy follower replica={}: shard2 followed the GC remap to live " + "pchunk={} (original pchunk_A={} is now orphaned); no stale route", + g_helper->replica_num(), live_pchunk, pchunk_A.value()); + } + + // ---- put blobs into shard2, then seal it ---- + const blob_id_t shard2_first_blob_id = pg_blob_id[pg_id]; + std::map< pg_id_t, std::vector< shard_id_t > > shard2_map{{pg_id, {shard2_id}}}; + put_blobs(shard2_map, num_blobs_per_shard, pg_blob_id); + g_helper->sync(); + + { + auto pg_chunks_after = chunk_selector->get_pg_chunks(pg_id); + ASSERT_TRUE(pg_chunks_after != nullptr); + auto live_pchunk_after = pg_chunks_after->at(v2.value()); + EXPECT_EQ(p2.value(), live_pchunk_after) + << "after putting blobs into shard2 on replica " << static_cast< int >(g_helper->replica_num()) + << ", shard2's recorded pchunk " << p2.value() << " diverged from the live mapping " << live_pchunk_after; + } + verify_get_blob(shard2_map, num_blobs_per_shard, false /* use_random_offset */, true /* wait_when_not_exist */, + {{pg_id, shard2_first_blob_id}}); + g_helper->sync(); + + run_on_pg_leader(pg_id, [&]() { + auto sealed2 = _obj_inst->shard_manager()->seal_shard(shard2_id, generateRandomTraceId()).get(); + RELEASE_ASSERT(!!sealed2, "failed to seal shard2"); + LOGINFO("[StalePChunkRouteAfterGC] leader sealed shard2=0x{:x}", shard2_id); + }); + + while (true) { + auto s2 = _obj_inst->shard_manager()->get_shard(shard2_id, 0).get(); + if (s2 && s2.value().state == ShardInfo::State::SEALED) break; + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + } + g_helper->sync(); + + // Remove flips here: all sync barriers and raft operations are complete, so + // __callback_flip has long since returned on all replicas. No UAF possible. + if (i_am_repro_follower) { + m_fc.remove_flip("wait_create_shard_in_log"); + m_fc.remove_flip("pause_create_shard_commit"); + } +} + +// =================================================================================================== +// StaleBlobRouteAfterSealAndGC: PUT_BLOB races with SEAL_SHARD pre_commit; sealed_lsn guard rejects it. +// +// Production scenario : +// A PUT_BLOB whose admission check passed (shard OPEN) should be rejected if the shard gets +// sealed before the blob is committed. The sealed_lsn guard in on_blob_put_commit must catch it. +// +// Exact sequence modelled (single replica, leader): +// ① SEAL_SHARD pre_commit fires and PAUSES before changing state=SEALED +// (flip "pause_seal_pre_commit"). At this point shard state is still OPEN. +// ② _put_blob is called in the test thread. get_blk_alloc_hints sees state==OPEN → passes, +// blk is allocated on pchunk_A. The put is async (raft not yet committed). +// ③ Gate releases → state = SEALED → SEAL_SHARD commit → sealed_lsn = X. +// ④ PUT_BLOB commit (lsn = X+1): on_blob_put_commit checks lsn(X+1) >= sealed_lsn(X) → reject. +// The allocated blk is freed; the blob does NOT land in the pg index. +// +// Verification: the late blob is absent from the index; bulk blobs are still readable. +// +// Runs on the leader replica only; no multi-replica complexity needed. +// Pause point: flip "pause_seal_pre_commit". Compiled out of release builds. +// =================================================================================================== +TEST_F(HomeObjectFixture, StaleBlobRouteAfterSealAndGC) { + const pg_id_t pg_id = 1; + const auto num_blobs_per_shard = SISL_OPTIONS["num_blobs"].as< uint64_t >(); + + create_pg(pg_id); + + if (!am_i_in_pg(pg_id)) { + g_helper->sync(); + g_helper->sync(); + return; + } + + const bool i_am_leader = (g_helper->my_replica_id() == get_leader_id(pg_id)); + + std::mutex repro2_mtx; + std::condition_variable repro2_cv; + std::atomic< bool > repro2_blocked{false}; + std::atomic< bool > repro2_released{false}; + + // ---- shard1: create and fill with blobs ---- + auto shard1 = create_shard(pg_id, 64 * Mi, "shard1"); + ASSERT_NE(shard1.id, 0u); + + std::map< pg_id_t, std::vector< shard_id_t > > shards{{pg_id, {shard1.id}}}; + std::map< pg_id_t, blob_id_t > pg_blob_id{{pg_id, 0}}; + put_blobs(shards, num_blobs_per_shard, pg_blob_id); + + // ---- arm the flip on the leader: pause SEAL pre_commit before state=SEALED ---- + if (i_am_leader) { + auto dont_care = m_fc.create_condition("", flip::Operator::DONT_CARE, (int)0); + flip::FlipFrequency freq; + freq.set_count(3); // 3 replicas all call callback_flip; count must be >= num_replicas + freq.set_percent(100); + m_fc.inject_callback_flip< void >("pause_seal_pre_commit", {dont_care}, freq, std::function< void() >([&]() { + LOGI( + "[StaleBlobRouteAfterSealAndGC] pausing SEAL pre_commit BEFORE lock"); + std::unique_lock< std::mutex > lk(repro2_mtx); + repro2_blocked.store(true); + repro2_cv.notify_all(); + repro2_cv.wait(lk, [&] { return repro2_released.load(); }); + LOGI("[StaleBlobRouteAfterSealAndGC] resuming SEAL pre_commit"); + })); + LOGINFO("[StaleBlobRouteAfterSealAndGC] armed pause_seal_pre_commit on leader replica={}", + g_helper->replica_num()); + } + + g_helper->sync(); // make sure flip is armed on all replicas before proceeding + + // ---- leader: trigger seal and race put_blob ---- + blob_id_t late_blob_id [[maybe_unused]] = INVALID_UINT64_ID; + if (i_am_leader) { + // 1. Start seal_shard in a background thread so it runs concurrently. + // seal_shard will hit the gate in pre_commit and pause there. + auto tid = generateRandomTraceId(); + bool seal_ok = false; + std::thread seal_thread([&]() { + auto r = std::move(_obj_inst->shard_manager()->seal_shard(shard1.id, tid)).get(); + seal_ok = r.hasValue(); + }); + + // 2. Wait until pre_commit is paused (shard state is still OPEN). + { + std::unique_lock< std::mutex > lk(repro2_mtx); + if (!repro2_cv.wait_for(lk, std::chrono::seconds(30), [&] { return repro2_blocked.load(); })) { + repro2_released.store(true); // avoid deadlock if gate never fires + repro2_cv.notify_all(); + seal_thread.join(); + m_fc.remove_flip("pause_seal_pre_commit"); + FAIL() << "SEAL pre_commit never reached the pause point"; + } + } + LOGINFO("[StaleBlobRouteAfterSealAndGC] leader sees SEAL pre_commit paused; shard state=OPEN; " + "calling _put_blob with shard still OPEN"); + + // 3. Call _put_blob in a background thread: shard state is OPEN → get_blk_alloc_hints + // passes → blk allocated. The .get() will complete AFTER gate release lets raft commit. + bool blob_rejected = false; + std::thread blob_thread([&]() { + auto blob = build_blob(num_blobs_per_shard); + auto b = std::move(_obj_inst->_put_blob(shard1, std::move(blob), tid)).get(); + blob_rejected = !b.hasValue(); + LOGINFO("[StaleBlobRouteAfterSealAndGC] leader: _put_blob result: {}", + b.hasValue() ? "admitted" : "rejected"); + }); + + // 4. Release the gate: state = SEALED, seal pre_commit returns → raft commits seal. + // After seal commit, sealed_lsn = lsn_seal. Then put_blob commit fires and + // on_blob_put_commit checks lsn(put) >= sealed_lsn → rejects. + { + std::unique_lock< std::mutex > lk(repro2_mtx); + repro2_released.store(true); + repro2_cv.notify_all(); + } + LOGINFO("[StaleBlobRouteAfterSealAndGC] leader gate released; state→SEALED; seal commit in flight"); + + // 5. Wait for both background threads. + blob_thread.join(); + seal_thread.join(); + m_fc.remove_flip("pause_seal_pre_commit"); + ASSERT_TRUE(seal_ok) << "seal_shard failed"; + + EXPECT_TRUE(blob_rejected) + << "[StaleBlobRouteAfterSealAndGC-fix] late _put_blob should have been rejected by sealed_lsn guard!"; + if (blob_rejected) { + LOGINFO("[StaleBlobRouteAfterSealAndGC] leader: late blob correctly rejected (sealed_lsn guard worked)"); + } + // propagate "no blob" to other replicas + g_helper->set_uint64_id(INVALID_UINT64_ID); + } + + g_helper->sync(); + + // ---- verification: bulk blobs still readable on all replicas ---- + verify_get_blob(shards, num_blobs_per_shard, false /* random_offset */, true /* wait */); + + // ---- delete some blobs and trigger GC + verify vchunk/pchunk consistency ---- + std::map< shard_id_t, std::set< blob_id_t > > to_delete; + const auto delete_count = num_blobs_per_shard / 2; // delete first half + for (blob_id_t blob_id = 0; blob_id < delete_count; ++blob_id) { + to_delete[shard1.id].insert(blob_id); + } + del_blobs(pg_id, to_delete); + g_helper->sync(); + + // trigger GC on the shard's pchunk + auto pchunk_opt = _obj_inst->get_shard_p_chunk_id(shard1.id); + ASSERT_TRUE(pchunk_opt.has_value()) << "Failed to get shard pchunk id"; + auto chunk_id = pchunk_opt.value(); + + auto gc_mgr = _obj_inst->gc_manager(); + auto gc_fut = gc_mgr->submit_gc_task(task_priority::normal, chunk_id); + bool gc_ok = std::move(gc_fut).get(); + ASSERT_TRUE(gc_ok) << "GC task failed on pchunk=" << chunk_id; + + g_helper->sync(); + + // After GC, the old pchunk may be a reserved/orphaned chunk; get the live pchunk now. + auto new_pchunk_opt = _obj_inst->get_shard_p_chunk_id(shard1.id); + ASSERT_TRUE(new_pchunk_opt.has_value()) << "Failed to get shard pchunk id after GC"; + auto p_chunk_id_after_gc = new_pchunk_opt.value(); + + // verify vchunk/pchunk consistency using the live pchunk + auto chunk_selector = _obj_inst->chunk_selector(); + auto EXVchunk = chunk_selector->get_extend_vchunk(p_chunk_id_after_gc); + ASSERT_TRUE(EXVchunk != nullptr) << "Failed to get extend vchunk"; + ASSERT_TRUE(EXVchunk->m_v_chunk_id.has_value()) << "Missing vchunk id"; + auto vchunk_id = EXVchunk->m_v_chunk_id.value(); + + auto shard_v_chunk_id_opt = _obj_inst->get_shard_v_chunk_id(shard1.id); + ASSERT_TRUE(shard_v_chunk_id_opt.has_value()) << "Failed to get shard vchunk id after GC"; + auto shard_vchunk_id = shard_v_chunk_id_opt.value(); + ASSERT_EQ(vchunk_id, shard_vchunk_id) << "shard's vchunk id mismatch after GC"; + + auto pg_chunks = chunk_selector->get_pg_chunks(pg_id); + ASSERT_EQ(pg_chunks->at(vchunk_id), p_chunk_id_after_gc) + << "vchunk->pchunk mapping inconsistent: vchunk=" << vchunk_id << " pchunk=" << p_chunk_id_after_gc; + + ASSERT_TRUE(EXVchunk->m_pg_id.has_value()) << "Missing pg_id in EXVchunk"; + ASSERT_EQ(EXVchunk->m_pg_id.value(), pg_id) << "pg_id mismatch in EXVchunk"; + + // verify all remaining blob's pchunk matches the live pchunk via index table + const auto remaining_start = delete_count; + auto index_table = _obj_inst->get_index_table(pg_id); + ASSERT_NE(index_table, nullptr) << "Failed to get index table for pg=" << pg_id; + for (blob_id_t blob_id = remaining_start; blob_id < num_blobs_per_shard; ++blob_id) { + auto pbas_result = _obj_inst->get_blob_from_index_table(index_table, shard1.id, blob_id); + ASSERT_TRUE(pbas_result.hasValue()) << "Failed to get blob pchunk for blob_id=" << blob_id << " after GC"; + ASSERT_EQ(pbas_result.value().chunk_num(), p_chunk_id_after_gc) + << "Blob pchunk mismatch: blob_id=" << blob_id << " expected pchunk=" << p_chunk_id_after_gc + << " actual=" << pbas_result.value().chunk_num(); + } + + LOGINFO("[StaleBlobRouteAfterSealAndGC] vchunk/pchunk consistency verified after GC delete"); + + g_helper->sync(); +} +#endif // _PRERELEASE \ No newline at end of file diff --git a/src/lib/homestore_backend/tests/hs_repl_test_helper.hpp b/src/lib/homestore_backend/tests/hs_repl_test_helper.hpp index 79941bcc..37a0a7c3 100644 --- a/src/lib/homestore_backend/tests/hs_repl_test_helper.hpp +++ b/src/lib/homestore_backend/tests/hs_repl_test_helper.hpp @@ -52,6 +52,7 @@ class HSReplTestHelper { protected: struct IPCData { void sync(uint64_t sync_point, uint32_t max_count) { + static constexpr auto k_sync_timeout = std::chrono::minutes(10); std::unique_lock lg(mtx_); LOGINFO("=== Syncing: replica={}(total {}), sync_point_num={} ===", homeobject_replica_count_, max_count, sync_point); @@ -63,7 +64,9 @@ class HSReplTestHelper { auxiliary_uint64_id_ = UINT64_MAX; cv_.notify_all(); } else { - cv_.wait(lg, [this, sync_point]() { return sync_point_num_ == sync_point; }); + bool ok = + cv_.wait_for(lg, k_sync_timeout, [this, sync_point]() { return sync_point_num_ == sync_point; }); + RELEASE_ASSERT(ok, "sync timed out after 5min at sync_point={}, a replica likely crashed", sync_point); } } diff --git a/src/lib/homestore_backend/tests/hs_shard_tests.cpp b/src/lib/homestore_backend/tests/hs_shard_tests.cpp index 8b68aee8..c8a1c020 100644 --- a/src/lib/homestore_backend/tests/hs_shard_tests.cpp +++ b/src/lib/homestore_backend/tests/hs_shard_tests.cpp @@ -1,4 +1,6 @@ #include "homeobj_fixture.hpp" +#include "generated/resync_shard_data_generated.h" +#include "generated/legacy_resync_shard_data_generated.h" TEST_F(HomeObjectFixture, CreateMultiShards) { pg_id_t pg_id{1}; @@ -122,11 +124,9 @@ TEST_F(HomeObjectFixture, ShardManagerRecovery) { create_pg(pg_id); // create one shard; - auto shard_info = create_shard(pg_id, Mi, "shard meta");; + auto shard_info = create_shard(pg_id, Mi, "shard meta"); auto shard_id = shard_info.id; EXPECT_EQ(ShardInfo::State::OPEN, shard_info.state); - EXPECT_EQ(Mi, shard_info.total_capacity_bytes); - EXPECT_EQ(Mi, shard_info.available_capacity_bytes); EXPECT_EQ(pg_id, shard_info.placement_group); // restart homeobject and check if pg/shard info will be recovered. @@ -162,7 +162,7 @@ TEST_F(HomeObjectFixture, ShardManagerRecovery) { EXPECT_EQ(1, pg_result->shard_sequence_num_); // re-create new shards on this pg works too even homeobject is restarted twice. - auto new_shard_info = create_shard(pg_id, Mi, "shard meta");; + auto new_shard_info = create_shard(pg_id, Mi, "shard meta"); EXPECT_NE(shard_id, new_shard_info.id); EXPECT_EQ(ShardInfo::State::OPEN, new_shard_info.state); @@ -177,7 +177,7 @@ TEST_F(HomeObjectFixture, SealedShardRecovery) { create_pg(pg_id); // create one shard and seal it. - auto shard_info = create_shard(pg_id, Mi, "shard meta");; + auto shard_info = create_shard(pg_id, Mi, "shard meta"); auto shard_id = shard_info.id; shard_info = seal_shard(shard_id); EXPECT_EQ(ShardInfo::State::SEALED, shard_info.state); @@ -295,8 +295,8 @@ TEST_F(HomeObjectFixture, CreateShardOnDiskLostMemeber) { } TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { - // Test migration with mixed versions (v1 and v2) to simulate partial migration or interrupted upgrade - // Also tests the actual bug scenario where v1 shards had incorrect type field + // Test migration with mixed versions (v2 and v3) to simulate partial migration or interrupted upgrade + // Also tests the actual bug scenario where v2 shards had incorrect type field pg_id_t pg_id{1}; create_pg(pg_id); @@ -321,84 +321,81 @@ TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { ShardInfo info; homestore::chunk_num_t p_chunk_id; homestore::chunk_num_t v_chunk_id; - bool should_be_v1; // Track which shards should be created as v1 + bool should_be_v2; // Track which shards should be created as v2 }; std::unordered_map< shard_id_t, ShardOriginalData > original_data_map; - // Mark which shards should be v1 vs v2 (using alternating pattern for variety) - // shard_ids[0], [2], [4] -> v1 (needs migration) - // shard_ids[1], [3] -> v2 (already migrated) - std::set v1_shard_ids = {shard_ids[0], shard_ids[2], shard_ids[4]}; + // Mark which shards should be v2 vs v3 (using alternating pattern for variety) + // shard_ids[0], [2], [4] -> v2 (needs migration) + // shard_ids[1], [3] -> v3 (already migrated) + std::set< shard_id_t > v2_shard_ids = {shard_ids[0], shard_ids[2], shard_ids[4]}; for (auto& shard : pg_result->shards_) { auto hs_shard = d_cast< HSHomeObject::HS_Shard* >(shard.get()); - EXPECT_EQ(0x02, hs_shard->sb_->version); - EXPECT_EQ(0x02, hs_shard->sb_->sb_version); + EXPECT_EQ(HSHomeObject::DataHeader::data_header_version, hs_shard->sb_->version); + EXPECT_EQ(HSHomeObject::shard_info_superblk::shard_sb_version, hs_shard->sb_->sb_version); auto shard_id = hs_shard->sb_->info.id; original_data_map[shard_id] = {hs_shard->sb_->info, hs_shard->sb_->p_chunk_id, hs_shard->sb_->v_chunk_id, - v1_shard_ids.contains(shard_id)}; + v2_shard_ids.contains(shard_id)}; // Destroy all current superblks hs_shard->sb_.destroy(); } // Simulate partial migration: create shards with mixed versions - LOGINFO("Creating mixed version shards (3 v1 shards, 2 v2 shards)..."); + LOGINFO("Creating mixed version shards (3 v2 shards, 2 v3 shards)..."); for (const auto& [shard_id, orig_data] : original_data_map) { - if (orig_data.should_be_v1) { - // Create v1 shard (needs migration) - homestore::superblk< HSHomeObject::v1_shard_info_superblk > old_sb("ShardManager"); - old_sb.create(sizeof(HSHomeObject::v1_shard_info_superblk)); + if (orig_data.should_be_v2) { + // Create v2 shard (needs migration to v3) + homestore::superblk< HSHomeObject::v2_shard_info_superblk > old_sb("ShardManager"); + old_sb.create(sizeof(HSHomeObject::v2_shard_info_superblk)); old_sb->magic = HSHomeObject::DataHeader::data_header_magic; - old_sb->version = 0x01; // Old version - // Simulate the actual bug: first v1 shard has incorrect type field set to BLOB_INFO - if (shard_id == shard_ids[0]) { - old_sb->type = HSHomeObject::DataHeader::data_type_t::BLOB_INFO; // Bug scenario - LOGINFO("Created v1 shard {} with BLOB_INFO type (bug scenario)", shard_id); - } else { - LOGINFO("Created v1 shard {}", shard_id); - } - // Convert v2 ShardInfo to v1_ShardInfo (v1 doesn't have meta field) + old_sb->version = HSHomeObject::DataHeader::data_header_version; + old_sb->type = HSHomeObject::DataHeader::data_type_t::SHARD_INFO; + old_sb->sb_version = 0x02; + // Convert v3 ShardInfo to v2 ShardInfo (v2 doesn't have sealed_lsn field) old_sb->info.id = orig_data.info.id; old_sb->info.placement_group = orig_data.info.placement_group; old_sb->info.state = orig_data.info.state; - old_sb->info.lsn = orig_data.info.lsn; + old_sb->info.lsn = orig_data.info.create_lsn; old_sb->info.created_time = orig_data.info.created_time; old_sb->info.last_modified_time = orig_data.info.last_modified_time; old_sb->info.available_capacity_bytes = orig_data.info.available_capacity_bytes; old_sb->info.total_capacity_bytes = orig_data.info.total_capacity_bytes; old_sb->info.current_leader = orig_data.info.current_leader; - // Note: v1 doesn't have meta field, so we don't copy it + std::memcpy(old_sb->info.meta, orig_data.info.meta, ShardInfo::meta_length); + // Note: sealed_lsn is not present in v2, so we don't set it old_sb->p_chunk_id = orig_data.p_chunk_id; old_sb->v_chunk_id = orig_data.v_chunk_id; old_sb.write(); + LOGINFO("Created v2 shard {}", shard_id); } else { - // Create v2 shard (already migrated) + // Create v3 shard (already migrated) homestore::superblk< HSHomeObject::shard_info_superblk > new_sb("ShardManager"); new_sb.create(sizeof(HSHomeObject::shard_info_superblk)); new_sb->magic = HSHomeObject::DataHeader::data_header_magic; - new_sb->version = 0x02; // New version + new_sb->version = HSHomeObject::DataHeader::data_header_version; new_sb->type = HSHomeObject::DataHeader::data_type_t::SHARD_INFO; - new_sb->sb_version = 0x02; + new_sb->sb_version = HSHomeObject::shard_info_superblk::shard_sb_version; new_sb->info = orig_data.info; new_sb->p_chunk_id = orig_data.p_chunk_id; new_sb->v_chunk_id = orig_data.v_chunk_id; new_sb.write(); - LOGINFO("Created v2 shard {}", shard_id); + LOGINFO("Created v3 shard {}", shard_id); } } - auto old_size = sizeof(HSHomeObject::v1_shard_info_superblk); + auto old_size = sizeof(HSHomeObject::v2_shard_info_superblk); auto new_size = sizeof(HSHomeObject::shard_info_superblk); LOGINFO("Setup complete - old size={}, new size={}", old_size, new_size); - // Restart homeobject - this should migrate only v1 shards + // Restart homeobject - this should migrate only v2 shards LOGINFO("Restarting homeobject to trigger migration..."); restart(); - // Verify all shards are now at v2 + // Verify all shards are now at v3 pg_result = _obj_inst->get_hs_pg(pg_id); EXPECT_TRUE(pg_result != nullptr); EXPECT_EQ(5, pg_result->shards_.size()); @@ -413,9 +410,11 @@ TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { ASSERT_NE(it, original_data_map.end()) << "Shard " << shard_id << " not found in original data"; const auto& orig_data = it->second; - // All shards should now be at v2 - EXPECT_EQ(0x02, hs_shard->sb_->version) << "Shard " << shard_id << " version should be 0x02"; - EXPECT_EQ(0x02, hs_shard->sb_->sb_version) << "Shard " << shard_id << " sb_version should be 0x02"; + // All shards should now be at v3 + EXPECT_EQ(HSHomeObject::DataHeader::data_header_version, hs_shard->sb_->version) + << "Shard " << shard_id << " version should be data_header_version"; + EXPECT_EQ(HSHomeObject::shard_info_superblk::shard_sb_version, hs_shard->sb_->sb_version) + << "Shard " << shard_id << " sb_version should be shard_sb_version"; // Verify all shard data was preserved EXPECT_EQ(HSHomeObject::DataHeader::data_header_magic, hs_shard->sb_->magic); @@ -425,8 +424,13 @@ TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { EXPECT_EQ(orig_data.info.state, hs_shard->sb_->info.state); EXPECT_EQ(orig_data.p_chunk_id, hs_shard->sb_->p_chunk_id); EXPECT_EQ(orig_data.v_chunk_id, hs_shard->sb_->v_chunk_id); + // v2 had no sealed_lsn field; migration sets it to INT64_MAX + if (orig_data.should_be_v2) { + EXPECT_EQ(static_cast< uint64_t >(INT64_MAX), hs_shard->sb_->info.sealed_lsn) + << "migrated v2 shard should have sealed_lsn=INT64_MAX"; + } - LOGINFO("Shard {} verified successfully (was {})", shard_id, orig_data.should_be_v1 ? "v1" : "v2"); + LOGINFO("Shard {} verified successfully (was {})", shard_id, orig_data.should_be_v2 ? "v2" : "v3"); } // Verify all shards are still functional @@ -438,8 +442,8 @@ TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { EXPECT_EQ(ShardInfo::State::OPEN, s.value().state); } - // Seal some v1 shards to verify they still work after migration - LOGINFO("Sealing two v1 shards to verify post-migration functionality..."); + // Seal some v2 shards to verify they still work after migration + LOGINFO("Sealing two v2 shards to verify post-migration functionality..."); auto sealed_shard_0 = seal_shard(shard_ids[0]); EXPECT_EQ(ShardInfo::State::SEALED, sealed_shard_0.state); @@ -447,9 +451,9 @@ TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { EXPECT_EQ(ShardInfo::State::SEALED, sealed_shard_2.state); // Track which shards were sealed for later verification - std::set sealed_shard_ids = {shard_ids[0], shard_ids[2]}; + std::set< shard_id_t > sealed_shard_ids = {shard_ids[0], shard_ids[2]}; - LOGINFO("Mixed version migration test completed - 3 shards migrated from v1, 2 shards already at v2"); + LOGINFO("Mixed version migration test completed - 3 shards migrated from v2, 2 shards already at v3"); // Restart again to verify the migration was persisted to disk LOGINFO("Second restart to verify persistence..."); @@ -465,8 +469,10 @@ TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { auto hs_shard = d_cast< HSHomeObject::HS_Shard* >(shard.get()); auto shard_id = hs_shard->sb_->info.id; - EXPECT_EQ(0x02, hs_shard->sb_->version) << "Shard " << shard_id << " should still be v2 after restart"; - EXPECT_EQ(0x02, hs_shard->sb_->sb_version) << "Shard " << shard_id << " sb_version should still be 0x02"; + EXPECT_EQ(HSHomeObject::DataHeader::data_header_version, hs_shard->sb_->version) + << "Shard " << shard_id << " should still be data_header_version after restart"; + EXPECT_EQ(HSHomeObject::shard_info_superblk::shard_sb_version, hs_shard->sb_->sb_version) + << "Shard " << shard_id << " sb_version should still be shard_sb_version"; // Verify sealed shards remain sealed if (sealed_shard_ids.contains(shard_id)) { @@ -478,6 +484,47 @@ TEST_F(HomeObjectFixture, ShardVersionMigrationRecovery) { } } - LOGINFO("Verified migration persisted to disk - all {} shards remain at v2 after second restart", + LOGINFO("Verified migration persisted to disk - all {} shards remain at v3 after second restart", pg_result->shards_.size()); } + +// Verifies backward compatibility: new ResyncShardMetaData code parses an old buffer produced by +// ResyncLegacyShardMetaData (schema without sealed_lsn). Because sealed_lsn is placed at the end, +// all common fields share the same vtable offsets — no misalignment. The missing sealed_lsn slot +// returns the schema default (9223372036854775807 = INT64_MAX). +TEST_F(HomeObjectFixture, ResyncShardMetaDataBackwardCompat) { + constexpr uint64_t shard_id = 0x0001000000000001ULL; + constexpr uint16_t pg_id = 1; + constexpr uint8_t state = static_cast< uint8_t >(ShardInfo::State::SEALED); + constexpr uint64_t created_lsn = 42; + constexpr uint64_t created_time = 1234567890ULL; + constexpr uint64_t last_modified = 9876543210ULL; + constexpr uint64_t total_capacity = 64ULL * Mi; + constexpr uint16_t vchunk_id = 7; + constexpr uint64_t default_sealed_lsn = 9223372036854775807ULL; // INT64_MAX per schema default + + std::vector< uint8_t > meta_bytes(ShardInfo::meta_length, 0); + const std::string meta_str = "shard_compat_test"; + std::memcpy(meta_bytes.data(), meta_str.c_str(), meta_str.size()); + + // Produce old-format buffer via legacy generated builder — same bytes as old + // CreateResyncShardMetaDataDirect before sealed_lsn existed. + flatbuffers::FlatBufferBuilder builder; + auto entry = CreateResyncLegacyShardMetaDataDirect(builder, shard_id, pg_id, state, created_lsn, created_time, + last_modified, total_capacity, vchunk_id, &meta_bytes); + builder.FinishSizePrefixed(entry); + + auto* msg = GetSizePrefixedResyncShardMetaData(builder.GetBufferPointer()); + + EXPECT_EQ(msg->shard_id(), shard_id); + EXPECT_EQ(msg->pg_id(), pg_id); + EXPECT_EQ(msg->state(), state); + EXPECT_EQ(msg->created_lsn(), created_lsn); + EXPECT_EQ(msg->created_time(), created_time); + EXPECT_EQ(msg->last_modified_time(), last_modified); + EXPECT_EQ(msg->total_capacity_bytes(), total_capacity); + EXPECT_EQ(msg->vchunk_id(), vchunk_id); + EXPECT_TRUE(std::memcmp(msg->meta()->data(), meta_bytes.data(), meta_bytes.size()) == 0); + // sealed_lsn absent in old buffer → new accessor returns schema default + EXPECT_EQ(msg->sealed_lsn(), default_sealed_lsn); +} diff --git a/src/lib/homestore_backend/tests/test_heap_chunk_selector.cpp b/src/lib/homestore_backend/tests/test_heap_chunk_selector.cpp index f268d576..053d5aa5 100644 --- a/src/lib/homestore_backend/tests/test_heap_chunk_selector.cpp +++ b/src/lib/homestore_backend/tests/test_heap_chunk_selector.cpp @@ -168,8 +168,6 @@ TEST_F(HeapChunkSelectorTest, test_for_each_chunk) { TEST_F(HeapChunkSelectorTest, test_total_disks) { ASSERT_EQ(HCS.total_disks(), 3); } TEST_F(HeapChunkSelectorTest, test_identical_layout) { - const homestore::blk_count_t count = 1; - homestore::blk_alloc_hints hints; for (uint16_t pg_id = 1; pg_id < 4; ++pg_id) { chunk_num_t p_chunk_id = 0; auto pg_chunk_collection = HCS.m_per_pg_chunks[pg_id]; @@ -177,38 +175,39 @@ TEST_F(HeapChunkSelectorTest, test_identical_layout) { for (int j = 3; j > 0; --j) { ASSERT_EQ(pg_chunk_collection->available_blk_count, start_available_blk_count); - const auto v_chunkID = HCS.get_most_available_blk_chunk(j, pg_id); - ASSERT_TRUE(v_chunkID.has_value()); - p_chunk_id = pg_chunk_collection->m_pg_chunks[v_chunkID.value()]->get_chunk_id(); + const auto exVchunk = HCS.pick_most_available_blk_chunk(j, pg_id); + ASSERT_NE(exVchunk, nullptr); + ASSERT_TRUE(exVchunk->m_v_chunk_id.has_value()); + const auto v_chunk_id = exVchunk->m_v_chunk_id.value(); + p_chunk_id = pg_chunk_collection->m_pg_chunks[v_chunk_id]->get_chunk_id(); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::INUSE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, j - 1); ASSERT_EQ(pg_chunk_collection->available_blk_count, start_available_blk_count - j); const auto v_chunkID2 = HCS.m_chunks[p_chunk_id]->m_v_chunk_id; ASSERT_TRUE(v_chunkID2.has_value()); - ASSERT_EQ(v_chunkID.value(), v_chunkID2.value()); - hints.application_hint = ((uint64_t)pg_id << 16) | v_chunkID.value(); + ASSERT_EQ(v_chunk_id, v_chunkID2.value()); - // mock leader on_commit - ASSERT_NE(HCS.select_chunk(count, hints), nullptr); + // mock leader on_commit: chunk already INUSE, select_specific_chunk is a no-op + ASSERT_NE(HCS.select_specific_chunk(pg_id, v_chunk_id), nullptr); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::INUSE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, j - 1); ASSERT_EQ(pg_chunk_collection->available_blk_count, start_available_blk_count - j); // mock leader rollback or on_error - ASSERT_TRUE(HCS.release_chunk(pg_id, v_chunkID.value())); + ASSERT_TRUE(HCS.release_chunk(pg_id, v_chunk_id)); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::AVAILABLE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, j); ASSERT_EQ(pg_chunk_collection->available_blk_count, start_available_blk_count); // mock follower rollback or on_error - ASSERT_TRUE(HCS.release_chunk(pg_id, v_chunkID.value())); + ASSERT_TRUE(HCS.release_chunk(pg_id, v_chunk_id)); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::AVAILABLE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, j); ASSERT_EQ(pg_chunk_collection->available_blk_count, start_available_blk_count); // mock follower on_commit - ASSERT_NE(HCS.select_chunk(count, hints), nullptr); // leader select + ASSERT_NE(HCS.select_specific_chunk(pg_id, v_chunk_id), nullptr); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::INUSE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, j - 1); ASSERT_EQ(pg_chunk_collection->available_blk_count, start_available_blk_count - j); @@ -216,25 +215,7 @@ TEST_F(HeapChunkSelectorTest, test_identical_layout) { start_available_blk_count -= j; } // all chunks have been given out - ASSERT_FALSE(HCS.get_most_available_blk_chunk(9999, pg_id).has_value()); - } -} - -TEST_F(HeapChunkSelectorTest, test_select_chunk) { - homestore::blk_count_t count = 1; - homestore::blk_alloc_hints hints; - auto chunk = HCS.select_chunk(count, hints); - ASSERT_EQ(chunk, nullptr); - - for (uint16_t pg_id = 1; pg_id < 4; ++pg_id) { - for (int j = 3; j > 0; --j) { - chunk_num_t v_chunk_id = 3 - j; - hints.application_hint = ((uint64_t)pg_id << 16) | v_chunk_id; - auto chunk = HCS.select_chunk(count, hints); - ASSERT_NE(chunk, nullptr); - ASSERT_EQ(chunk->get_pdev_id(), pg_id); // in this ut, pg_id is same as pdev id - ASSERT_EQ(chunk->available_blks(), j); - } + ASSERT_EQ(nullptr, HCS.pick_most_available_blk_chunk(9999, pg_id)); } } @@ -252,16 +233,17 @@ TEST_F(HeapChunkSelectorTest, test_select_specific_chunk_and_release_chunk) { const chunk_num_t p_chunk_id = chunk_ids->at(v_chunk_id); auto pg_chunk_collection = HCS.m_per_pg_chunks[pg_id]; - auto chunk = HCS.select_specific_chunk(pg_id, v_chunk_id); - ASSERT_NE(nullptr, chunk); - ASSERT_EQ(chunk->get_chunk_id(), p_chunk_id); + auto p_chunkID = HCS.select_specific_chunk(pg_id, v_chunk_id); + ASSERT_NE(p_chunkID, nullptr); + ASSERT_EQ(p_chunkID->get_chunk_id(), p_chunk_id); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::INUSE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, 2); ASSERT_EQ(pg_chunk_collection->available_blk_count, 1 + 2); // test select an INUSE chunk - chunk = HCS.select_specific_chunk(pg_id, v_chunk_id); - ASSERT_NE(nullptr, chunk); + p_chunkID = HCS.select_specific_chunk(pg_id, v_chunk_id); + ASSERT_NE(p_chunkID, nullptr); + ASSERT_EQ(p_chunkID->get_chunk_id(), p_chunk_id); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::INUSE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, 2); ASSERT_EQ(pg_chunk_collection->available_blk_count, 1 + 2); @@ -279,13 +261,14 @@ TEST_F(HeapChunkSelectorTest, test_select_specific_chunk_and_release_chunk) { ASSERT_EQ(pg_chunk_collection->available_blk_count, 1 + 2 + 3); // select again - chunk = HCS.select_specific_chunk(pg_id, v_chunk_id); - ASSERT_NE(nullptr, chunk); + p_chunkID = HCS.select_specific_chunk(pg_id, v_chunk_id); + ASSERT_NE(p_chunkID, nullptr); + ASSERT_EQ(p_chunkID->get_chunk_id(), p_chunk_id); ASSERT_EQ(HCS.m_chunks[p_chunk_id]->m_state, ChunkState::INUSE); ASSERT_EQ(pg_chunk_collection->available_num_chunks, 2); ASSERT_EQ(pg_chunk_collection->available_blk_count, 1 + 2); - ASSERT_EQ(pg_id, chunk->get_pdev_id()); // in this ut, pg_id is same as pdev id - ASSERT_EQ(p_chunk_id, chunk->get_chunk_id()); + ASSERT_EQ(pg_id, HCS.m_chunks[p_chunk_id]->get_pdev_id()); // in this ut, pg_id is same as pdev id + ASSERT_EQ(p_chunk_id, HCS.m_chunks[p_chunk_id]->get_chunk_id()); } } @@ -378,12 +361,13 @@ TEST_F(HeapChunkSelectorTest, test_recovery) { ASSERT_EQ(pg_chunk_collection->m_pg_chunks[0]->m_state, ChunkState::INUSE); ASSERT_EQ(pg_chunk_collection->m_pg_chunks[1]->m_state, ChunkState::AVAILABLE); - const auto v_chunkID = HCS_recovery.get_most_available_blk_chunk(9999, pg_id); - ASSERT_TRUE(v_chunkID.has_value()); - auto chunk = HCS_recovery.select_specific_chunk(pg_id, v_chunkID.value()); - ASSERT_NE(chunk, nullptr); - ASSERT_EQ(chunk->get_pdev_id(), pg_id); - ASSERT_EQ(chunk->available_blks(), 2); + const auto exVchunk = HCS_recovery.pick_most_available_blk_chunk(9999, pg_id); + ASSERT_NE(exVchunk, nullptr); + ASSERT_TRUE(exVchunk->m_v_chunk_id.has_value()); + auto selected_chunk = HCS_recovery.select_specific_chunk(pg_id, exVchunk->m_v_chunk_id.value()); + ASSERT_NE(selected_chunk, nullptr); + ASSERT_EQ(selected_chunk->get_pdev_id(), pg_id); + ASSERT_EQ(selected_chunk->available_blks(), 2); ASSERT_EQ(pg_chunk_collection->m_pg_chunks[1]->m_state, ChunkState::INUSE); } } diff --git a/src/lib/homestore_backend/tests/test_homestore_backend_dynamic.cpp b/src/lib/homestore_backend/tests/test_homestore_backend_dynamic.cpp index 04aaf2ea..4938f904 100644 --- a/src/lib/homestore_backend/tests/test_homestore_backend_dynamic.cpp +++ b/src/lib/homestore_backend/tests/test_homestore_backend_dynamic.cpp @@ -464,11 +464,10 @@ void HomeObjectFixture::ReplaceMember(bool withGC) { set_basic_flip("pg_blob_iterator_generate_shard_blob_list_error", 1); // simulate generate shard blob list error set_basic_flip("pg_blob_iterator_load_blob_data_error", 1, 10); // simulate load blob data error - set_basic_flip("state_machine_write_corrupted_data", 3, 25); // simulate random data corruption - set_basic_flip("snapshot_receiver_pg_error", 1); // simulate pg creation error - set_basic_flip("snapshot_receiver_shard_write_data_error", 2, 33); // simulate shard write data error - set_basic_flip("snapshot_receiver_blob_write_data_error", 4, 15); // simulate blob write data error - set_basic_flip("snapshot_receiver_blk_allocation_error", 4, 15); // simulate blob allocation error + set_basic_flip("state_machine_write_corrupted_data", 3, 25); // simulate random data corruption + set_basic_flip("snapshot_receiver_pg_error", 1); // simulate pg creation error + set_basic_flip("snapshot_receiver_blob_write_data_error", 4, 15); // simulate blob write data error + set_basic_flip("snapshot_receiver_blk_allocation_error", 4, 15); // simulate blob allocation error #endif std::string task_id = "task_id"; diff --git a/src/lib/memory_backend/mem_shard_manager.cpp b/src/lib/memory_backend/mem_shard_manager.cpp index 8566408b..3eb66e84 100644 --- a/src/lib/memory_backend/mem_shard_manager.cpp +++ b/src/lib/memory_backend/mem_shard_manager.cpp @@ -6,8 +6,8 @@ namespace homeobject { uint64_t ShardManager::max_shard_size() { return Gi; } -ShardManager::AsyncResult< ShardInfo > MemoryHomeObject::_create_shard(pg_id_t pg_owner, uint64_t size_bytes, std::string meta, - trace_id_t tid) { +ShardManager::AsyncResult< ShardInfo > MemoryHomeObject::_create_shard(pg_id_t pg_owner, uint64_t size_bytes, + std::string meta, trace_id_t tid) { (void)tid; auto const now = get_current_timestamp(); auto info = ShardInfo(0ull, pg_owner, ShardInfo::State::OPEN, 0, now, now, size_bytes, size_bytes);