From 951d61d590979d7ac017489e95f8eec5632dafa2 Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Fri, 15 Sep 2023 19:59:53 -0600 Subject: [PATCH] Movable io_blob_safe does not require wrapping. (#59) --- conanfile.py | 2 +- src/include/homeobject/blob_manager.hpp | 7 +++-- src/lib/blob_manager.cpp | 6 ++++ src/lib/homestore/tests/test_home_object.cpp | 8 ++---- src/lib/memory/blob_manager.cpp | 11 +------- src/lib/memory/tests/BlobManagerTest.cpp | 29 ++++++++++---------- 6 files changed, 30 insertions(+), 33 deletions(-) diff --git a/conanfile.py b/conanfile.py index 1cf54553..b6e820c0 100644 --- 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 index 8e5f5505..d4e3b58b 100644 --- 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 index 25067f2c..cbdbcb59 100644 --- 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 index ed606a29..964ffd85 100644 --- 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 index 2758e3c4..9812d7bf 100644 --- 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 index 0ce8aad0..bf4816ea 100644 --- 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[]) {