fix comments

src/lib/homestore/heap_chunk_selector.cpp

@@ -15,7 +15,17 @@ namespace homeobject {
 // it means after the single thread initialization,
 // 1 the key collection of m_per_dev_heap will never change.
 // 2 the key collection of m_chunks will never change
-void HeapChunkSelector::add_chunk(csharedChunk& chunk) {
+
+// this should only be called when initializing HeapChunkSelector in Homestore
+void HeapChunkSelector::add_chunk(csharedChunk& chunk) { m_chunks.emplace(VChunk(chunk).get_chunk_id(), chunk); }
+
+void HeapChunkSelector::add_chunk_internal(const chunk_num_t chunkID) {
+    if (m_chunks.find(chunkID) == m_chunks.end()) {
+        // sanity check
+        LOGWARN("No chunk found for ChunkID {}", chunkID);
+        return;
+    }
+    const auto& chunk = m_chunks[chunkID];
     VChunk vchunk(chunk);
     auto pdevID = vchunk.get_pdev_id();
     // add this find here, since we don`t want to call make_shared in try_emplace every time.
@@ -24,8 +34,6 @@ void HeapChunkSelector::add_chunk(csharedChunk& chunk) {
     auto& avalableBlkCounter = it->second->available_blk_count;
     avalableBlkCounter.fetch_add(vchunk.available_blks());
 
-    m_chunks.emplace(vchunk.get_chunk_id(), chunk);
-
     auto& heapLock = it->second->mtx;
     auto& heap = it->second->m_heap;
     std::lock_guard< std::mutex > l(heapLock);
@@ -94,38 +102,19 @@ void HeapChunkSelector::foreach_chunks(std::function< void(csharedChunk&) >&& cb
                   [cb = std::move(cb)](auto& p) { cb(p.second); });
 }
 
-void HeapChunkSelector::release_chunk(const homestore::chunk_num_t chunkID) {
+void HeapChunkSelector::release_chunk(const chunk_num_t chunkID) {
     const auto& it = m_chunks.find(chunkID);
     if (it == m_chunks.end()) {
+        // sanity check
         LOGWARN("No chunk found for ChunkID {}", chunkID);
     } else {
-        add_chunk(it->second);
+        add_chunk_internal(chunkID);
     }
 }
 
-void HeapChunkSelector::mark_chunk_selected(const homestore::chunk_num_t chunkID) {
-    const auto& it = m_chunks.find(chunkID);
-    if (it == m_chunks.end()) {
-        LOGWARN("No chunk found for ChunkID {}", chunkID);
-        return;
-    }
-    uint32_t pdevID = VChunk(m_chunks[chunkID]).get_pdev_id();
-    auto& heapLock = m_per_dev_heap[pdevID]->mtx;
-    auto& heap = m_per_dev_heap[pdevID]->m_heap;
-    std::vector< VChunk > temp;
-    std::lock_guard< std::mutex > l(heapLock);
-    for (; !heap.empty();) {
-        VChunk vchunk = heap.top();
-        heap.pop();
-        if (vchunk.get_chunk_id() == chunkID) {
-            auto pdevID = vchunk.get_pdev_id();
-            m_per_dev_heap[pdevID]->available_blk_count.fetch_sub(vchunk.available_blks());
-            break;
-        }
-        temp.emplace_back(vchunk);
-    }
-    for (auto& vchunk : temp) {
-        heap.emplace(vchunk);
-    }
+void HeapChunkSelector::build_per_dev_chunk_heap(const std::unordered_set< chunk_num_t >& excludingChunks) {
+    for (const auto& p : m_chunks) {
+        if (excludingChunks.find(p.first) == excludingChunks.end()) { add_chunk_internal(p.first); }
+    };
 }
 } // namespace homeobject

src/lib/homestore/heap_chunk_selector.h

@@ -27,6 +27,7 @@ class HeapChunkSelector : public homestore::ChunkSelector {
     };
 
     using VChunkHeap = std::priority_queue< VChunk, std::vector< VChunk >, VChunkComparator >;
+    using chunk_num_t = homestore::chunk_num_t;
 
     struct PerDevHeap {
         std::mutex mtx;
@@ -40,16 +41,17 @@ class HeapChunkSelector : public homestore::ChunkSelector {
 
     // this function is used to return a chunk back to ChunkSelector when sealing a shard, and will only be used by
     // Homeobject.
-    void release_chunk(const homestore::chunk_num_t);
+    void release_chunk(const chunk_num_t);
 
-    // homestore will initialize HeapChunkSelector by adding all the chunks. but some of them are already
-    // selected by open shards. so after homeobject restarts, we need to mark all these chunks as selected
-    void mark_chunk_selected(const homestore::chunk_num_t);
+    // this should be called after ShardManager is initialized and get all the open shards
+    void build_per_dev_chunk_heap(const std::unordered_set< chunk_num_t >& excludingChunks);
 
 private:
     std::unordered_map< uint32_t, std::shared_ptr< PerDevHeap > > m_per_dev_heap;
 
     // hold all the chunks , selected or not
-    std::unordered_map< homestore::chunk_num_t, csharedChunk > m_chunks;
+    std::unordered_map< chunk_num_t, csharedChunk > m_chunks;
+
+    void add_chunk_internal(const chunk_num_t);
 };
 } // namespace homeobject

src/lib/homestore/tests/test_heap_chunk_selector.cpp

@@ -12,6 +12,7 @@ SISL_LOGGING_INIT(logging, HOMEOBJECT_LOG_MODS)
 SISL_OPTIONS_ENABLE(logging)
 
 namespace homestore {
+
 class Chunk : public std::enable_shared_from_this< Chunk > {
 public:
     uint32_t available_blks() const { return m_available_blks; }
@@ -58,6 +59,7 @@ cshared< Chunk > VChunk::get_internal_chunk() const { return m_internal_chunk->g
 using homeobject::csharedChunk;
 using homeobject::HeapChunkSelector;
 using homestore::Chunk;
+using homestore::chunk_num_t;
 
 class HeapChunkSelectorTest : public ::testing::Test {
 protected:
@@ -71,6 +73,8 @@ class HeapChunkSelectorTest : public ::testing::Test {
         HCS.add_chunk(std::make_shared< Chunk >(3, 7, 1));
         HCS.add_chunk(std::make_shared< Chunk >(3, 8, 2));
         HCS.add_chunk(std::make_shared< Chunk >(3, 9, 3));
+        std::unordered_set< chunk_num_t > excludingChunks;
+        HCS.build_per_dev_chunk_heap(excludingChunks);
     };
 
 public:
@@ -120,22 +124,6 @@ TEST_F(HeapChunkSelectorTest, test_release_chunk) {
     ASSERT_EQ(chunk2->available_blks(), 2);
 }
 
-TEST_F(HeapChunkSelectorTest, test_mark_chunk_selected) {
-    HCS.mark_chunk_selected(2);
-    homestore::blk_count_t count = 1;
-    homestore::blk_alloc_hints hints;
-    hints.pdev_id_hint = 1;
-    // we mark chunk id 2 is select ,so we will get 3 and 1
-    // from pdev 1;
-    auto chunk = HCS.select_chunk(count, hints);
-    ASSERT_EQ(chunk->get_pdev_id(), 1);
-    ASSERT_EQ(chunk->available_blks(), 3);
-
-    chunk = HCS.select_chunk(count, hints);
-    ASSERT_EQ(chunk->get_pdev_id(), 1);
-    ASSERT_EQ(chunk->available_blks(), 1);
-}
-
 int main(int argc, char* argv[]) {
     int parsed_argc = argc;
     ::testing::InitGoogleTest(&parsed_argc, argv);