Short circuit sealing a sealed shard. Test as well. (#60)

eBay · Sep 19, 2023 · 291b070 · 291b070
1 parent 951d61d
commit 291b070
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 10 deletions.
diff --git a/src/lib/memory/shard_manager.cpp b/src/lib/memory/shard_manager.cpp
@@ -38,12 +38,13 @@ ShardManager::Result< ShardInfo > MemoryHomeObject::_create_shard(pg_id pg_owner
 ShardManager::Result< ShardInfo > MemoryHomeObject::_seal_shard(shard_id id) {
     auto lg = std::scoped_lock(_pg_lock, _shard_lock);
     auto shard_it = _shard_map.find(id);
-    if (_shard_map.end() == shard_it) return folly::makeUnexpected(ShardError::UNKNOWN_SHARD);
+    RELEASE_ASSERT(_shard_map.end() != shard_it, "Missing ShardIterator!");
 
     auto const_it = shard_it->second;
     auto pg_it = _pg_map.find(const_it->placement_group);
     RELEASE_ASSERT(_pg_map.end() != pg_it, "Missing ShardInfo!");
 
+    // Copy the Info and replace it
     auto new_info = *const_it;
     new_info.state = ShardInfo::State::SEALED;
     pg_it->second.second.erase(const_it);

diff --git a/src/lib/memory/tests/ShardManagerTest.cpp b/src/lib/memory/tests/ShardManagerTest.cpp
@@ -117,13 +117,15 @@ TEST_F(ShardManagerFixtureWShard, SealShardNoShard) {
 }
 
 TEST_F(ShardManagerFixtureWShard, SealShard) {
-    auto e = m_memory_homeobj->shard_manager()->seal_shard(_shard.id).get();
-    ASSERT_TRUE(!!e);
-    e.then([this](auto const& info) {
-        EXPECT_TRUE(info.id == _shard.id);
-        EXPECT_TRUE(info.placement_group == _shard.placement_group);
-        EXPECT_EQ(info.state, ShardInfo::State::SEALED);
-    });
+    for (auto i = 0; 2 > i; ++i) {
+        auto e = m_memory_homeobj->shard_manager()->seal_shard(_shard.id).get();
+        ASSERT_TRUE(!!e);
+        e.then([this](auto const& info) {
+            EXPECT_TRUE(info.id == _shard.id);
+            EXPECT_TRUE(info.placement_group == _shard.placement_group);
+            EXPECT_EQ(info.state, ShardInfo::State::SEALED);
+        });
+    }
 }
 
 int main(int argc, char* argv[]) {

diff --git a/src/lib/shard_manager.cpp b/src/lib/shard_manager.cpp
@@ -31,8 +31,11 @@ ShardManager::AsyncResult< InfoList > HomeObjectImpl::list_shards(pg_id pg) cons
 }
 
 ShardManager::AsyncResult< ShardInfo > HomeObjectImpl::seal_shard(shard_id id) {
-    return _defer().thenValue(
-        [this, id](auto) mutable -> ShardManager::Result< ShardInfo > { return _seal_shard(id); });
+    return _get_shard(id).thenValue([this](auto const e) mutable -> ShardManager::Result< ShardInfo > {
+        if (!e) return folly::makeUnexpected(ShardError::UNKNOWN_SHARD);
+        if (ShardInfo::State::SEALED == e.value().state) return e.value();
+        return _seal_shard(e.value().id);
+    });
 }
 
 ShardManager::AsyncResult< ShardInfo > HomeObjectImpl::get_shard(shard_id id) const { return _get_shard(id); }