From ab4406576652cb8a77a72194b045c3f71ba654b6 Mon Sep 17 00:00:00 2001 From: yawzhang Date: Tue, 11 Feb 2025 19:58:37 +0800 Subject: [PATCH] Remove optimization on blk free operation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The optimization in blk free may cause the following issue, just remove it and wait for GC handling: - T1: blob1 is written with LSN 1 — [blkid=10, chunk=1, cnt=5]. - T2: blob1 is deleted with LSN 10, causing the last_append_offset to revert to 10. - T3: blob2 is written with LSN 11 — [blkid=10, chunk=1, cnt=5]. - T4: The SM is terminated and restarted. - T5: LSN 1 is replayed, committing block [blkid=10, chunk=1, cnt=5]. - T6: LSN 11 is replayed, committing block [blkid=10, chunk=1, cnt=5]. - T7: LSN 10 is committed, freeing block [blkid=10, chunk=1, cnt=5]. - T8: LSN 11 is committed again, but since the blocks have already been freed, they are not available for LSN 11. --- conanfile.py | 2 +- src/lib/blkalloc/append_blk_allocator.cpp | 28 ++--------------------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/conanfile.py b/conanfile.py index a37363bac..b0c3d3640 100644 --- a/conanfile.py +++ b/conanfile.py @@ -9,7 +9,7 @@ class HomestoreConan(ConanFile): name = "homestore" - version = "6.6.19" + version = "6.6.20" homepage = "https://github.com/eBay/Homestore" description = "HomeStore Storage Engine" diff --git a/src/lib/blkalloc/append_blk_allocator.cpp b/src/lib/blkalloc/append_blk_allocator.cpp index 1380a5ff6..eca445381 100644 --- a/src/lib/blkalloc/append_blk_allocator.cpp +++ b/src/lib/blkalloc/append_blk_allocator.cpp @@ -127,33 +127,9 @@ void AppendBlkAllocator::cp_flush(CP* cp) { } } -// -// free operation does: -// 1. book keeping "total freeable" space -// 2. if the blk being freed happens to be last block, move last_append_offset backwards accordingly; -// +// free operation books keeping "total freeable" space void AppendBlkAllocator::free(const BlkId& bid) { - // If we are freeing the last block, just move the offset back - blk_num_t cur_last_offset = m_last_append_offset.load(); - auto const input_last_offset = bid.blk_num() + bid.blk_count(); - blk_num_t new_last_offset; - bool freeing_in_middle{false}; - do { - if (input_last_offset == cur_last_offset) { - new_last_offset = bid.blk_num(); - freeing_in_middle = false; - } else { - new_last_offset = cur_last_offset; - freeing_in_middle = true; - } - } while (!m_last_append_offset.compare_exchange_weak(cur_last_offset, new_last_offset)); - - if (freeing_in_middle) { - // Freeing something in the middle, increment the count - m_freeable_nblks.fetch_add(bid.blk_count()); - } else { - m_commit_offset.store(m_last_append_offset.load()); - } + m_freeable_nblks.fetch_add(bid.blk_count()); m_is_dirty.store(true); }