Fixed review comments

conanfile.py

@@ -5,7 +5,7 @@
 
 class HomestoreConan(ConanFile):
     name = "homestore"
-    version = "4.5.10"
+    version = "4.5.11"
 
     homepage = "https://github.com/eBay/Homestore"
     description = "HomeStore Storage Engine"

src/lib/blkalloc/bitmap_blk_allocator.cpp

@@ -31,13 +31,10 @@ BitmapBlkAllocator::BitmapBlkAllocator(BlkAllocConfig const& cfg, bool is_fresh,
             nullptr);
     }
 
-    if (is_fresh || !is_persistent()) {
-        m_disk_bm = std::make_unique< sisl::Bitset >(m_num_blks, m_chunk_id, m_align_size);
-        do_init();
+    if (is_fresh) {
+        if (is_persistent()) { m_disk_bm = std::make_unique< sisl::Bitset >(m_num_blks, m_chunk_id, m_align_size); }
     }
-}
 
-void BitmapBlkAllocator::do_init() {
     // NOTE:  Blocks per portion must be modulo word size so locks do not fall on same word
     m_blks_per_portion = sisl::round_up(m_blks_per_portion, m_disk_bm ? m_disk_bm->word_size() : 64u);
 
@@ -54,12 +51,12 @@ void BitmapBlkAllocator::on_meta_blk_found(void* mblk_cookie, sisl::byte_view co
         hs_utils::extract_byte_array(buf, meta_service().is_aligned_buf_needed(size), meta_service().align_size())}};
 
     m_alloced_blk_count.store(m_disk_bm->get_set_count(), std::memory_order_relaxed);
-    do_init();
-
     load();
 }
 
 void BitmapBlkAllocator::cp_flush(CP*) {
+    if (!is_persistent()) { return; }
+
     if (m_is_disk_bm_dirty.load()) {
         sisl::byte_array bitmap_buf = acquire_underlying_buffer();
         if (m_meta_blk_cookie) {
@@ -99,6 +96,8 @@ BlkAllocStatus BitmapBlkAllocator::alloc_on_disk(BlkId const& bid) {
             {
                 auto lock{portion.portion_auto_lock()};
                 if (!hs()->is_initializing()) {
+                    // During recovery we might try to free the entry which is already freed while replaying the
+                    // journal, This assert is valid only post recovery.
                     BLKALLOC_REL_ASSERT(m_disk_bm->is_bits_reset(b.blk_num(), b.blk_count()),
                                         "Expected disk blks to reset");
                 }
@@ -133,9 +132,8 @@ void BitmapBlkAllocator::free_on_disk(BlkId const& bid) {
         {
             auto lock{portion.portion_auto_lock()};
             if (!hs()->is_initializing()) {
-                /* During recovery we might try to free the entry which is already freed while replaying the journal,
-                 * This assert is valid only post recovery.
-                 */
+                // During recovery we might try to free the entry which is already freed while replaying the journal,
+                // This assert is valid only post recovery.
                 if (!m_disk_bm->is_bits_set(b.blk_num(), b.blk_count())) {
                     BLKALLOC_LOG(ERROR, "bit not set {} nblks {} chunk number {}", b.blk_num(), b.blk_count(),
                                  m_chunk_id);

src/lib/blkalloc/bitmap_blk_allocator.h

@@ -96,6 +96,10 @@ class BitmapBlkAllocator : public BlkAllocator {
     /* Get status */
     nlohmann::json get_status(int log_level) const override;
 
+    void incr_alloced_blk_count(blk_count_t nblks) { m_alloced_blk_count.fetch_add(nblks, std::memory_order_relaxed); }
+    void decr_alloced_blk_count(blk_count_t nblks) { m_alloced_blk_count.fetch_sub(nblks, std::memory_order_relaxed); }
+    int64_t get_alloced_blk_count() const { return m_alloced_blk_count.load(std::memory_order_acquire); }
+
 private:
     void do_init();
     sisl::ThreadVector< BlkId >* get_alloc_blk_list();
@@ -117,5 +121,6 @@ class BitmapBlkAllocator : public BlkAllocator {
     std::unique_ptr< sisl::Bitset > m_disk_bm{nullptr};
     std::atomic< bool > m_is_disk_bm_dirty{true}; // initially disk_bm treated as dirty
     void* m_meta_blk_cookie{nullptr};
+    std::atomic< int64_t > m_alloced_blk_count{0};
 };
 } // namespace homestore

src/lib/blkalloc/blk_allocator.h

@@ -166,10 +166,6 @@ class BlkAllocator {
     virtual std::string to_string() const = 0;
     virtual void cp_flush(CP* cp) = 0;
 
-    void incr_alloced_blk_count(blk_count_t nblks) { m_alloced_blk_count.fetch_add(nblks, std::memory_order_relaxed); }
-    void decr_alloced_blk_count(blk_count_t nblks) { m_alloced_blk_count.fetch_sub(nblks, std::memory_order_relaxed); }
-    int64_t get_alloced_blk_count() const { return m_alloced_blk_count.load(std::memory_order_acquire); }
-
     uint32_t get_align_size() const { return m_align_size; }
     blk_num_t get_total_blks() const { return m_num_blks; }
     const std::string& get_name() const { return m_name; }
@@ -186,7 +182,6 @@ class BlkAllocator {
     const blk_num_t m_num_blks;
     const chunk_num_t m_chunk_id;
     const bool m_is_persistent;
-    std::atomic< int64_t > m_alloced_blk_count{0};
 };
 
 } // namespace homestore

src/lib/blkalloc/varsize_blk_allocator.cpp

@@ -232,6 +232,7 @@ bool VarsizeBlkAllocator::allocator_state_machine() {
 }
 
 void VarsizeBlkAllocator::load() {
+    BLKALLOC_DBG_ASSERT_CMP(is_persistent(), ==, true, "Load called on non-persistent blk allocator");
     m_cache_bm->copy(*get_disk_bitmap());
 
     BLKALLOC_LOG(INFO, "VarSizeBlkAllocator initialized loading bitmap of size={} used blks={} from persistent storage",

src/lib/device/virtual_dev.cpp

@@ -254,7 +254,8 @@ BlkAllocStatus VirtualDev::alloc_blks_from_chunk(blk_count_t nblks, blk_alloc_hi
 
 void VirtualDev::free_blk(BlkId const& bid, VDevCPContext* vctx) {
     auto do_free_action = [this](auto const& b, VDevCPContext* vctx) {
-        if (vctx) {
+        if (vctx && (m_allocator_type != blk_allocator_type_t::append)) {
+            // We don't want to accumulate here for append blk allocator.
             vctx->m_free_blkid_list.push_back(b);
         } else {
             BlkAllocator* allocator = m_dmgr.get_chunk_mutable(b.chunk_num())->blk_allocator_mutable();