Movable io_blob_safe does not require wrapping. (#59)

eBay · Sep 16, 2023 · 951d61d · 951d61d
1 parent 57fe3e6
commit 951d61d
Show file tree

Hide file tree

Showing 6 changed files with 30 additions and 33 deletions.
diff --git a/conanfile.py b/conanfile.py
@@ -9,7 +9,7 @@
 
 class HomeObjectConan(ConanFile):
     name = "homeobject"
-    version = "0.9.3"
+    version = "0.10.1"
     homepage = "https://github.com/eBay/HomeObject"
     description = "Blob Store built on HomeReplication"
     topics = ("ebay")

diff --git a/src/include/homeobject/blob_manager.hpp b/src/include/homeobject/blob_manager.hpp
@@ -12,9 +12,12 @@ namespace homeobject {
 ENUM(BlobError, uint16_t, UNKNOWN = 1, TIMEOUT, INVALID_ARG, NOT_LEADER, UNKNOWN_SHARD, UNKNOWN_BLOB,
      CHECKSUM_MISMATCH);
 
-using unique_buffer = std::unique_ptr< sisl::byte_array_impl >;
 struct Blob {
-    unique_buffer body;
+    Blob(sisl::io_blob_safe b, std::string const& u, uint64_t o) : body(std::move(b)), user_key(u), object_off(o) {}
+
+    Blob clone() const;
+
+    sisl::io_blob_safe body;
     std::string user_key;
     uint64_t object_off;
     std::optional< peer_id > current_leader{std::nullopt};

diff --git a/src/lib/blob_manager.cpp b/src/lib/blob_manager.cpp
@@ -31,4 +31,10 @@ BlobManager::NullAsyncResult HomeObjectImpl::del(shard_id shard, blob_id const&
     });
 }
 
+Blob Blob::clone() const {
+    auto new_body = sisl::io_blob_safe(body.size);
+    std::memcpy(new_body.bytes, body.bytes, body.size);
+    return Blob(std::move(new_body), user_key, object_off);
+}
+
 } // namespace homeobject
diff --git a/src/lib/homestore/tests/test_home_object.cpp b/src/lib/homestore/tests/test_home_object.cpp
@@ -91,11 +91,9 @@ TEST_F(HomeObjectFixture, TestValidations) {
 }
 
 TEST_F(HomeObjectFixture, PutBlobMissingShard) {
-    EXPECT_EQ(BlobError::UNKNOWN_SHARD,
-              _obj_inst->blob_manager()
-                  ->put(1, homeobject::Blob{std::make_unique< sisl::byte_array_impl >(4096), "user_key", 0ul})
-                  .get()
-                  .error());
+    EXPECT_EQ(
+        BlobError::UNKNOWN_SHARD,
+        _obj_inst->blob_manager()->put(1, homeobject::Blob{sisl::io_blob_safe(4096), "user_key", 0ul}).get().error());
 }
 
 TEST_F(HomeObjectFixture, GetBlobMissingShard) {

diff --git a/src/lib/memory/blob_manager.cpp b/src/lib/memory/blob_manager.cpp
@@ -14,7 +14,6 @@ BlobManager::Result< blob_id > MemoryHomeObject::_put_blob(ShardInfo const& shar
 
     // Generate BlobID (with RAFT this will happen implicitly) and Route
     auto const route = BlobRoute{shard.id, our_shard._shard_seq_num++};
-    if ((UINT64_MAX >> 16) < route.blob) { return folly::makeUnexpected(BlobError::UNKNOWN); }
     LOGTRACEMOD(homeobject, "Writing Blob {}", route);
 
     // Write (move) Blob to Heap
@@ -48,15 +47,7 @@ BlobManager::Result< Blob > MemoryHomeObject::_get_blob(ShardInfo const& shard,
     // This is only *safe* because we defer GC.
     auto& ext_blob = blob_it->second;
     RELEASE_ASSERT(ext_blob._blob != nullptr, "Blob Deleted!");
-    RELEASE_ASSERT(!!ext_blob._blob->body, "BlobBody Deleted!");
-    auto unsafe_ptr = ext_blob._blob->body.get();
-
-    Blob user_blob;
-    user_blob.object_off = ext_blob._blob->object_off;
-    user_blob.user_key = ext_blob._blob->user_key;
-    user_blob.body = std::make_unique< sisl::byte_array_impl >(unsafe_ptr->size);
-    std::memcpy(user_blob.body->bytes, unsafe_ptr->bytes, user_blob.body->size);
-    return user_blob;
+    return ext_blob._blob->clone();
 }
 
 BlobManager::NullResult MemoryHomeObject::_del_blob(ShardInfo const& shard, blob_id id) {

diff --git a/src/lib/memory/tests/BlobManagerTest.cpp b/src/lib/memory/tests/BlobManagerTest.cpp
@@ -70,10 +70,9 @@ class BlobManagerFixture : public ::testing::Test {
         EXPECT_EQ(BlobError::UNKNOWN_BLOB, g_e.error());
 
         LOGDEBUG("Insert Blob to: {}", _shard_1.id);
-        auto o_e =
-            m_memory_homeobj->blob_manager()
-                ->put(_shard_1.id, Blob{std::make_unique< sisl::byte_array_impl >(4 * Ki, 512u), "test_blob", 4 * Mi})
-                .get();
+        auto o_e = m_memory_homeobj->blob_manager()
+                       ->put(_shard_1.id, Blob{sisl::io_blob_safe(4 * Ki, 512u), "test_blob", 4 * Mi})
+                       .get();
         EXPECT_TRUE(!!o_e);
         o_e.then([this](auto&& b) mutable { _blob_id = std::move(b); });
     }
@@ -108,17 +107,16 @@ TEST_F(BlobManagerFixture, BasicTests) {
                 our_calls.push_back(
                     m_memory_homeobj->blob_manager()->get(_shard_2.id, (i - _shard_2.id)).deferValue([](auto const&) {
                     }));
-                our_calls.push_back(m_memory_homeobj->blob_manager()->put(i, Blob()).deferValue(
-                    [](auto const& e) { EXPECT_EQ(BlobError::UNKNOWN_SHARD, e.error()); }));
                 our_calls.push_back(
                     m_memory_homeobj->blob_manager()
-                        ->put(_shard_1.id,
-                              Blob{std::make_unique< sisl::byte_array_impl >(4 * Ki, 512u), "test_blob", 4 * Mi})
-                        .deferValue([](auto const& e) { EXPECT_TRUE(!!e); }));
+                        ->put(i, Blob{sisl::io_blob_safe(512u, 512u), "test_blob", 0ul})
+                        .deferValue([](auto const& e) { EXPECT_EQ(BlobError::UNKNOWN_SHARD, e.error()); }));
+                our_calls.push_back(m_memory_homeobj->blob_manager()
+                                        ->put(_shard_1.id, Blob{sisl::io_blob_safe(4 * Ki, 512u), "test_blob", 4 * Mi})
+                                        .deferValue([](auto const& e) { EXPECT_TRUE(!!e); }));
                 our_calls.push_back(
                     m_memory_homeobj->blob_manager()
-                        ->put(_shard_2.id,
-                              Blob{std::make_unique< sisl::byte_array_impl >(8 * Ki, 512u), "test_blob_2", 4 * Mi})
+                        ->put(_shard_2.id, Blob{sisl::io_blob_safe(8 * Ki, 512u), "test_blob_2", 4 * Mi})
                         .deferValue([](auto const& e) { EXPECT_TRUE(!!e); }));
                 our_calls.push_back(
                     m_memory_homeobj->blob_manager()->del(i, _blob_id).deferValue([](auto const& e) {}));
@@ -136,14 +134,15 @@ TEST_F(BlobManagerFixture, BasicTests) {
         t.join();
     folly::collectAll(calls).via(folly::getGlobalCPUExecutor()).get();
     EXPECT_TRUE(m_memory_homeobj->shard_manager()->seal_shard(_shard_1.id).get());
-    auto p_e =
-        m_memory_homeobj->blob_manager()
-            ->put(_shard_1.id, Blob{std::make_unique< sisl::byte_array_impl >(4 * Ki, 512u), "test_blob", 4 * Mi})
-            .get();
+    auto p_e = m_memory_homeobj->blob_manager()
+                   ->put(_shard_1.id, Blob{sisl::io_blob_safe(4 * Ki, 512u), "test_blob", 4 * Mi})
+                   .get();
     ASSERT_TRUE(!p_e);
     EXPECT_EQ(BlobError::INVALID_ARG, p_e.error());
 
     EXPECT_TRUE(m_memory_homeobj->blob_manager()->del(_shard_1.id, _blob_id).get());
+    EXPECT_EQ(BlobError::UNKNOWN_BLOB, m_memory_homeobj->blob_manager()->get(_shard_1.id, _blob_id).get().error());
+    EXPECT_EQ(BlobError::UNKNOWN_BLOB, m_memory_homeobj->blob_manager()->del(_shard_1.id, _blob_id).get().error());
 }
 
 int main(int argc, char* argv[]) {