the chunk should be selected for open shard when recovery

conanfile.py

@@ -10,7 +10,7 @@
 
 class HomeObjectConan(ConanFile):
     name = "homeobject"
-    version = "2.7.7"
+    version = "2.7.8"
 
     homepage = "https://github.com/eBay/HomeObject"
     description = "Blob Store built on HomeReplication"

src/lib/homestore_backend/gc_manager.cpp

@@ -6,8 +6,8 @@ namespace homeobject {
 
 /* GCManager */
 
-GCManager::GCManager(std::shared_ptr< HeapChunkSelector > chunk_selector, HSHomeObject* homeobject) :
-        m_chunk_selector{chunk_selector}, m_hs_home_object{homeobject} {
+GCManager::GCManager(HSHomeObject* homeobject) :
+        m_chunk_selector{homeobject->chunk_selector()}, m_hs_home_object{homeobject} {
     homestore::HomeStore::instance()->meta_service().register_handler(
         GCManager::_gc_actor_meta_name,
         [this](homestore::meta_blk* mblk, sisl::byte_view buf, size_t size) {

src/lib/homestore_backend/gc_manager.hpp

@@ -29,7 +29,7 @@ using GCBlobIndexTable = homestore::IndexTable< BlobRouteByChunkKey, BlobRouteVa
 
 class GCManager {
 public:
-    GCManager(std::shared_ptr< HeapChunkSelector > chunk_selector, HSHomeObject* homeobject);
+    GCManager(HSHomeObject* homeobject);
     ~GCManager();
 
     // Disallow copy and move

src/lib/homestore_backend/heap_chunk_selector.cpp

@@ -390,7 +390,6 @@ void HeapChunkSelector::switch_chunks_for_pg(const pg_id_t pg_id, const chunk_nu
                     "gc task_id={}, the pchunk_id for vchunk={} in chunkselector for pg_id={} is already {},  skip "
                     "switching chunks!",
                     task_id, v_chunk_id, pg_id, new_chunk_id);
-
         return;
     } else {
         RELEASE_ASSERT(
@@ -405,9 +404,6 @@ void HeapChunkSelector::switch_chunks_for_pg(const pg_id_t pg_id, const chunk_nu
                     "pchunk_id={}",
                     task_id, v_chunk_id, pg_id, old_chunk_id, new_chunk_id);
     }
-
-    // LOGDEBUGMOD(homeobject, "gc: after switch chunks for pg_id={}, pg_chunks={}", pg_chunks);
-
     pg_chunk_collection->available_blk_count += new_available_blks - old_available_blks;
 }
 

src/lib/homestore_backend/hs_homeobject.cpp

@@ -221,9 +221,6 @@ void HSHomeObject::init_homestore() {
 
         // We dont have any repl dev now, explicitly call init_completed_cb() where we register PG/Shard meta types.
         repl_app->on_repl_devs_init_completed();
-
-        // we need to regitster the meta blk handlers after metaservice is ready.
-        gc_mgr_ = std::make_shared< GCManager >(chunk_selector_, this);
         // if this is the first time we are starting, we need to create gc metablk for each pdev， which record the
         // reserved chunks and indextable.
         auto pdev_chunks = chunk_selector_->get_pdev_chunks();
@@ -321,6 +318,9 @@ void HSHomeObject::on_replica_restart() {
             },
             [this](bool success) { on_snp_rcvr_shard_list_meta_blk_recover_completed(success); }, true);
         HomeStore::instance()->meta_service().read_sub_sb(_snp_rcvr_shard_list_meta_name);
+
+        // gc_manager will be created only once here.
+        gc_mgr_ = std::make_shared< GCManager >(this);
     });
 }
 
@@ -352,7 +352,6 @@ void HSHomeObject::init_cp() {
 
 void HSHomeObject::init_gc() {
     using namespace homestore;
-    if (!gc_mgr_) gc_mgr_ = std::make_shared< GCManager >(chunk_selector_, this);
     // when initializing, there is not gc task. we need to recover reserved chunks here, so that the reserved chunks
     // will not be put into pdev heap when built
     HomeStore::instance()->meta_service().read_sub_sb(GCManager::_gc_actor_meta_name);

src/lib/homestore_backend/hs_homeobject.hpp

@@ -681,7 +681,6 @@ class HSHomeObject : public HomeObjectImpl {
     static std::string serialize_pg_info(const PGInfo& info);
     static PGInfo deserialize_pg_info(const unsigned char* pg_info_str, size_t size);
     void add_pg_to_map(unique< HS_PG > hs_pg);
-    const HS_PG* _get_hs_pg_unlocked(pg_id_t pg_id) const;
 
     // create shard related
     shard_id_t generate_new_shard_id(pg_id_t pg);
@@ -923,7 +922,6 @@ class HSHomeObject : public HomeObjectImpl {
 
     void update_pg_meta_after_gc(const pg_id_t pg_id, const homestore::chunk_num_t move_from_chunk,
                                  const homestore::chunk_num_t move_to_chunk, const uint64_t task_id);
-
     uint32_t get_pg_tombstone_blob_count(pg_id_t pg_id) const;
 
     // Snapshot persistence related
@@ -933,6 +931,7 @@ class HSHomeObject : public HomeObjectImpl {
     const Shard* _get_hs_shard(const shard_id_t shard_id) const;
     std::shared_ptr< GCBlobIndexTable > get_gc_index_table(std::string uuid) const;
     void trigger_immediate_gc();
+    const HS_PG* _get_hs_pg_unlocked(pg_id_t pg_id) const;
 
 private:
     std::shared_ptr< BlobIndexTable > create_pg_index_table();

src/lib/homestore_backend/hs_shard_manager.cpp

@@ -489,14 +489,14 @@ void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& h, hom
     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)); }
+        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)); }
+        if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(ShardError(ShardErrorCode::CRC_MISMATCH))); }
         return;
     }
 #endif

src/lib/homestore_backend/replication_state_machine.cpp

@@ -607,11 +607,8 @@ folly::Future< std::error_code > ReplicationStateMachine::on_fetch_data(const in
                                                                         const homestore::MultiBlkId& local_blk_id,
                                                                         sisl::sg_list& sgs) {
     if (0 == header.size()) {
-        LOGD("Header is empty, which means this req has been committed at leader, so ignore this request. req "
-             "lsn={}",
-             lsn);
-        RELEASE_ASSERT(lsn != -1, "the lsn of a committed req should not be -1");
-        return folly::makeFuture< std::error_code >(std::error_code{});
+        LOGW("Header is empty in on_fetch_data for lsn {}", lsn);
+        return folly::makeFuture< std::error_code >(std::make_error_code(std::errc::invalid_argument));
     }
 
     // the lsn here will mostly be -1 ,since this lsn has not been appeneded and thus get no lsn
@@ -1006,4 +1003,60 @@ void ReplicationStateMachine::handle_no_space_left(homestore::repl_lsn_t lsn, ho
     }
 }
 
+void ReplicationStateMachine::on_log_replay_done(const homestore::group_id_t& group_id) {
+    // when we reaching here, it means all the logs of this group has been replayed, but we don`t join the raft group
+    // ATM and thus no new request can be handled. Now, we can safely mark all the chunks with open shard in this group
+    // to inuse state, so that gc can not gc this chunk.
+
+    // we must do this job here, because:
+
+    // 1 we should change the chunk state after all the logs are replayed. since if we mark the chunk state to inuse
+    // before all the logs are replayed, there is a case that the chunk will be released by the last seal_shard, which
+    // is prior to the lastest open_shard of this chunk. for example: lsn 10 is create_shard_1 ,lsn 20 is seal_shard_1
+    // and lsn 30 is create_shard_2. shard1 and shard 2 are in the same chunk. now if we mark the chunk to inuse before
+    // replay lsn 20, then when replaying lsn 20, the chunk will be released by lsn 20(commit_seal_shard). and when we
+    // completed replay, the final state of this chunk is available. but what we want is inuse, since shard_2 is opened
+    // in lsn 30 and not sealed.
+
+    // 2 if we mark the chunk state to in use after the repl_dev join the raft group, then it can accept new request or
+    // can be gc before we mark it to inuse.
+
+    // so this is the only timepoint we should make all the chunks, which contains open shards, to inuse state.
+
+    auto pg_id_opt = home_object_->get_pg_id_with_group_id(group_id);
+    if (!pg_id_opt.has_value()) {
+        // there are two cases we might reach here.
+
+        //  1 when baseline resync, crash happens after pg is destroyed but before new pg is created in follower. when
+        //  recovery, we will find we have a group , but the corresponding pg does not exist.
+
+        //  2 when replacememeber, we will destroy the repl_dev after destroying the pg. if crash happens after pg is
+        //  destroyed but before the repl_dev superblk is destroyed, then when recovery, we will find we have a
+        //  group(repl_dev), but have not corresponding pg.
+
+        //  3 when fail to create repl_dev, there might be some stale repl_dev left on a node. for example, success to
+        //  add the first memeber but fail to add the second memeber, the there will be a stale repl_dev on leader and
+        //  the first member. see https://github.com/eBay/HomeObject/pull/136#discussion_r1470504271
+        LOGW("can not find any pg for group={}!", group_id);
+        return;
+    }
+
+    const auto pg_id = pg_id_opt.value();
+    RELEASE_ASSERT(home_object_->pg_exists(pg_id), "pg={} should exist, but not! fatal error!", pg_id);
+
+    const auto& shards_in_pg = (const_cast< HSHomeObject::HS_PG* >(home_object_->_get_hs_pg_unlocked(pg_id)))->shards_;
+    auto chunk_selector = home_object_->chunk_selector();
+
+    for (const auto& shard_iter : shards_in_pg) {
+        const auto& shard_sb = ((d_cast< HSHomeObject::HS_Shard* >(shard_iter.get()))->sb_).get();
+        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);
+            LOGD("vchunk={} is selected for shard={} in pg={} when recovery", vchunk_id, shard_sb->info.id, pg_id);
+        }
+    }
+}
+
 } // namespace homeobject

src/lib/homestore_backend/replication_state_machine.hpp

@@ -224,6 +224,11 @@ class ReplicationStateMachine : public homestore::ReplDevListener {
     ///
     void notify_committed_lsn(int64_t lsn) override;
 
+    /// @brief this is called after all the logs are replayed but before joining raft group.
+    /// @param group_id - the group , where all the logs are replayed but not join raft group
+    ///
+    void on_log_replay_done(const homestore::group_id_t& group_id) override;
+
 private:
     HSHomeObject* home_object_{nullptr};