From e610df05ca995b1d1c2e9bca216417b4ebaaf636 Mon Sep 17 00:00:00 2001 From: Sanal P Date: Fri, 31 Oct 2025 11:48:17 -0700 Subject: [PATCH] Check is_blk_alloced and use free_blk_now during log replay. For same lba if there are multiple writes, we free the old blkid's. If those free blkid's are reused and freed again there will be multiple log entries. So during log replay check if they are alloced and if alloced directly free the blk from blk allocator bitmap cache. --- conanfile.py | 4 ++-- src/lib/listener.hpp | 3 +++ src/lib/volume/tests/CMakeLists.txt | 2 +- src/lib/volume/volume.cpp | 1 + src/lib/volume_mgr.cpp | 15 ++++++++++++--- 5 files changed, 19 insertions(+), 6 deletions(-) diff --git a/conanfile.py b/conanfile.py index d37515a..1eb8b61 100644 --- a/conanfile.py +++ b/conanfile.py @@ -9,7 +9,7 @@ class HomeBlocksConan(ConanFile): name = "homeblocks" - version = "5.0.3" + version = "5.0.4" homepage = "https://github.com/eBay/HomeBlocks" description = "Block Store built on HomeStore" @@ -46,7 +46,7 @@ def build_requirements(self): self.test_requires("gtest/1.17.0") def requirements(self): - self.requires("homestore/[^7.0]@oss/master", transitive_headers=True) + self.requires("homestore/[^7.1]@oss/master", transitive_headers=True) self.requires("iomgr/[^12.0]@oss/master", transitive_headers=True) self.requires("sisl/[^13.0]@oss/master", transitive_headers=True) diff --git a/src/lib/listener.hpp b/src/lib/listener.hpp index 9791737..e035749 100644 --- a/src/lib/listener.hpp +++ b/src/lib/listener.hpp @@ -56,6 +56,9 @@ class HBListener : public homestore::ReplDevListener { void on_complete_replace_member(const std::string& task_id, const homestore::replica_member_info& member_out, const homestore::replica_member_info& member_in, homestore::trace_id_t tid) override {} + void on_clean_replace_member_task(const std::string& task_id, const homestore::replica_member_info& member_out, + const homestore::replica_member_info& member_in, + homestore::trace_id_t tid) override {} void on_remove_member(const homestore::replica_id_t&, homestore::trace_id_t) override {} void on_rollback(int64_t lsn, const sisl::blob& header, const sisl::blob& key, cintrusive< homestore::repl_req_ctx >& ctx) override {} diff --git a/src/lib/volume/tests/CMakeLists.txt b/src/lib/volume/tests/CMakeLists.txt index 7b567da..5719747 100644 --- a/src/lib/volume/tests/CMakeLists.txt +++ b/src/lib/volume/tests/CMakeLists.txt @@ -33,5 +33,5 @@ target_link_libraries(test_volume_chunk_selector ) add_test(NAME VolumeTest COMMAND test_volume --gc_timer_nsecs=3 --index_chunk_size_mb=128 --data_chunk_size_mb=128) -add_test(NAME VolumeIOTest COMMAND test_volume_io --index_chunk_size_mb=128 --data_chunk_size_mb=128 --gtest_filter=-VolumeIOTest.LongRunningRandomIO:VolumeIOTest.WriteCrash:VolumeIOTest.IndexPutFailure) # FIXME: turn on after io issue is fixed; +add_test(NAME VolumeIOTest COMMAND test_volume_io --index_chunk_size_mb=128 --data_chunk_size_mb=128) add_test(NAME VolumeChunkSelectorTest COMMAND test_volume_chunk_selector) diff --git a/src/lib/volume/volume.cpp b/src/lib/volume/volume.cpp index 94bdaa7..0ece11c 100644 --- a/src/lib/volume/volume.cpp +++ b/src/lib/volume/volume.cpp @@ -239,6 +239,7 @@ VolumeManager::NullAsyncResult Volume::write(const vol_interface_req_ptr& vol_re lba_t start_lba = vol_req->lba; for (auto& blkid : new_blkids) { DEBUG_ASSERT_EQ(blkid.num_pieces(), 1, "Multiple blkid pieces"); + LOGT("volume write blkid={} lba={}", blkid.to_integer(), start_lba); // Split the large blkid to individual blkid having only one block because each LBA points // to a blkid containing single blk which is stored in index value. Calculate the checksum for each diff --git a/src/lib/volume_mgr.cpp b/src/lib/volume_mgr.cpp index 03ce435..b33d79d 100644 --- a/src/lib/volume_mgr.cpp +++ b/src/lib/volume_mgr.cpp @@ -363,11 +363,20 @@ void HomeBlocksImpl::on_write(int64_t lsn, const sisl::blob& header, const sisl: } // Free all the old blkids. This happens for both normal writes - // and crash recovery. + // and crash recovery. During recovery we also if it has been alloced, + // because we could have stale log entries which have old blkid's + // which may be already freed. for (uint32_t i = 0; i < journal_entry->num_old_blks; i++) { BlkId old_blkid = *r_cast< const BlkId* >(key_buffer); - LOGT("on_write free blk {}", old_blkid); - vol_ptr->rd()->async_free_blks(lsn, old_blkid); + if (repl_ctx == nullptr) { + if (homestore::hs()->data_service().is_blk_alloced(old_blkid)) { + LOGT("on_write free blk {} start_lba {}", old_blkid, journal_entry->start_lba); + homestore::hs()->data_service().free_blk_now(old_blkid); + } + } else { + LOGT("on_write free blk {} start_lba {}", old_blkid, journal_entry->start_lba); + homestore::hs()->data_service().free_blk_now(old_blkid); + } key_buffer += sizeof(BlkId); }