fix review comments and add uts for multi-shards on same pg

src/lib/homestore_backend/hs_homeobject.cpp

@@ -52,7 +52,7 @@ void HSHomeObject::init_homestore() {
             {HS_SERVICE::LOG_REPLICATED, hs_format_params{.size_pct = 10.0}},
             {HS_SERVICE::LOG_LOCAL, hs_format_params{.size_pct = 0.1}}, // TODO: Remove this after HS disables LOG_LOCAL
             {HS_SERVICE::REPLICATION,
-             hs_format_params{.size_pct = 80.0,
+             hs_format_params{.size_pct = 79.0,
                               .num_chunks = 65000,
                               .block_size = 1024,
                               .alloc_type = blk_allocator_type_t::append,

src/lib/homestore_backend/hs_homeobject.hpp

@@ -114,7 +114,7 @@ class HSHomeObject : public HomeObjectImpl {
     void on_shard_message_commit(int64_t lsn, sisl::blob const& header, homestore::MultiBlkId const& blkids,
                                  homestore::ReplDev* repl_dev, cintrusive< homestore::repl_req_ctx >& hs_ctx);
 
-    ShardManager::Result< homestore::chunk_num_t > get_shard_chunk(shard_id_t id) const;
+    std::optional< homestore::chunk_num_t > get_shard_chunk(shard_id_t id) const;
 
     std::optional< homestore::chunk_num_t > get_any_chunk_id(pg_id_t const pg);
 

src/lib/homestore_backend/hs_shard_manager.cpp

@@ -188,12 +188,12 @@ void HSHomeObject::update_shard_in_map(const ShardInfo& shard_info) {
     hs_shard->update_info(shard_info);
 }
 
-ShardManager::Result< homestore::chunk_num_t > HSHomeObject::get_shard_chunk(shard_id_t id) const {
+std::optional< homestore::chunk_num_t > HSHomeObject::get_shard_chunk(shard_id_t id) const {
     std::scoped_lock lock_guard(_shard_lock);
     auto shard_iter = _shard_map.find(id);
-    if (shard_iter == _shard_map.end()) { return folly::makeUnexpected(ShardError::UNKNOWN_SHARD); }
+    if (shard_iter == _shard_map.end()) { return std::nullopt; }
     auto hs_shard = d_cast< HS_Shard* >((*shard_iter->second).get());
-    return hs_shard->sb_->chunk_id;
+    return std::make_optional< homestore::chunk_num_t >(hs_shard->sb_->chunk_id);
 }
 
 std::optional< homestore::chunk_num_t > HSHomeObject::get_any_chunk_id(pg_id_t const pg_id) {
@@ -207,7 +207,7 @@ std::optional< homestore::chunk_num_t > HSHomeObject::get_any_chunk_id(pg_id_t c
     }
 
     auto& shards = pg->shards_;
-    if (shards.empty()) { return std::optional< homestore::chunk_num_t >(); }
+    if (shards.empty()) { return std::nullopt; }
     auto hs_shard = d_cast< HS_Shard* >(shards.front().get());
     // cache it;
     pg->any_allocated_chunk_id_ = hs_shard->sb_->chunk_id;

src/lib/homestore_backend/replication_state_machine.cpp

@@ -51,7 +51,7 @@ homestore::blk_alloc_hints ReplicationStateMachine::get_blk_alloc_hints(sisl::bl
 
     case ReplicationMessageType::SEAL_SHARD_MSG: {
         auto chunk_id = home_object_->get_shard_chunk(msg_header->shard_id);
-        RELEASE_ASSERT(!!chunk_id, "unknown shard id to get binded chunk");
+        RELEASE_ASSERT(chunk_id.has_value(), "unknown shard id to get binded chunk");
         homestore::blk_alloc_hints hints;
         hints.chunk_id_hint = chunk_id.value();
         return hints;

src/lib/homestore_backend/tests/test_shard_manager.cpp

@@ -120,6 +120,62 @@ TEST_F(ShardManagerTesting, CreateShardSuccess) {
     EXPECT_EQ(_pg_id, shard_info.placement_group);
 }
 
+TEST_F(ShardManagerTesting, CreateMultiShards) {
+    auto e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get();
+    ASSERT_TRUE(!!e);
+    homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(_home_object.get());
+    auto chunk_num_1 = ho->get_shard_chunk(e.value().id);
+    ASSERT_TRUE(chunk_num_1.has_value());
+
+    // create another shard again.
+    e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get();
+    ASSERT_TRUE(!!e);
+    auto chunk_num_2 = ho->get_shard_chunk(e.value().id);
+    ASSERT_TRUE(chunk_num_2.has_value());
+
+    // check if both chunk is on the same pdev;
+    auto alloc_hint1 = ho->chunk_selector()->chunk_to_hints(chunk_num_1.value());
+    auto alloc_hint2 = ho->chunk_selector()->chunk_to_hints(chunk_num_2.value());
+    ASSERT_TRUE(alloc_hint1.pdev_id_hint.has_value());
+    ASSERT_TRUE(alloc_hint2.pdev_id_hint.has_value());
+    ASSERT_TRUE(alloc_hint1.pdev_id_hint.value() == alloc_hint2.pdev_id_hint.value());
+}
+
+TEST_F(ShardManagerTesting, CreateMultiShardsOnMultiPG) {
+    // create another PG;
+    auto peer1 = _home_object->our_uuid();
+    auto peer2 = boost::uuids::random_generator()();
+
+    auto new_pg_id = static_cast< homeobject::pg_id_t >(_pg_id + 1);
+    auto info = homeobject::PGInfo(_pg_id + 1);
+    info.members.insert(homeobject::PGMember{peer1, "peer1", 1});
+    info.members.insert(homeobject::PGMember{peer2, "peer2", 0});
+    EXPECT_TRUE(_home_object->pg_manager()->create_pg(std::move(info)).get());
+
+    std::vector< homeobject::pg_id_t > pgs{_pg_id, new_pg_id};
+
+    for (const auto pg : pgs) {
+        auto e = _home_object->shard_manager()->create_shard(pg, Mi).get();
+        ASSERT_TRUE(!!e);
+        homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(_home_object.get());
+        auto chunk_num_1 = ho->get_shard_chunk(e.value().id);
+        ASSERT_TRUE(chunk_num_1.has_value());
+
+        // create another shard again.
+        e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get();
+        ASSERT_TRUE(!!e);
+        auto chunk_num_2 = ho->get_shard_chunk(e.value().id);
+        ASSERT_TRUE(chunk_num_2.has_value());
+
+        // check if both chunk is on the same pdev;
+        auto alloc_hint1 = ho->chunk_selector()->chunk_to_hints(chunk_num_1.value());
+        auto alloc_hint2 = ho->chunk_selector()->chunk_to_hints(chunk_num_2.value());
+        ASSERT_TRUE(alloc_hint1.pdev_id_hint.has_value());
+        ASSERT_TRUE(alloc_hint2.pdev_id_hint.has_value());
+        ASSERT_TRUE(alloc_hint1.pdev_id_hint.value() == alloc_hint2.pdev_id_hint.value());
+    }
+}
+
 TEST_F(ShardManagerTesting, CreateShardAndValidateMembers) {
     auto e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get();
     ASSERT_TRUE(!!e);