From 53ff3852971d8fd282433fc5c3df63acd5809a28 Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Wed, 18 Oct 2023 09:58:42 -0700 Subject: [PATCH 1/6] Enable Basic shard tests for HomeStore. --- src/lib/homestore_backend/CMakeLists.txt | 2 +- .../homestore_backend/tests/CMakeLists.txt | 19 ++++++++++--------- ...t_shard_manager.cpp => hs_shard_tests.cpp} | 0 ...home_object.cpp => test_hs_homeobject.cpp} | 0 4 files changed, 11 insertions(+), 10 deletions(-) rename src/lib/homestore_backend/tests/{test_shard_manager.cpp => hs_shard_tests.cpp} (100%) rename src/lib/homestore_backend/tests/{test_home_object.cpp => test_hs_homeobject.cpp} (100%) diff --git a/src/lib/homestore_backend/CMakeLists.txt b/src/lib/homestore_backend/CMakeLists.txt index 85f3ea18..842f45f1 100644 --- a/src/lib/homestore_backend/CMakeLists.txt +++ b/src/lib/homestore_backend/CMakeLists.txt @@ -32,7 +32,7 @@ target_link_libraries(shard_homestore_test homeobject_homestore ${COMMON_TEST_DEPS} ) -#add_test(NAME ShardHomestoreTest COMMAND shard_homestore_test -csv error) +add_test(NAME ShardHomestoreTest COMMAND shard_homestore_test -csv error) add_executable (blob_homestore_test) target_sources(blob_homestore_test PRIVATE $) diff --git a/src/lib/homestore_backend/tests/CMakeLists.txt b/src/lib/homestore_backend/tests/CMakeLists.txt index edda8ba0..f550d0b0 100644 --- a/src/lib/homestore_backend/tests/CMakeLists.txt +++ b/src/lib/homestore_backend/tests/CMakeLists.txt @@ -2,22 +2,23 @@ cmake_minimum_required(VERSION 3.13) include_directories (BEFORE .) -add_executable(test_home_object) -target_sources(test_home_object PRIVATE test_home_object.cpp) -target_link_libraries(test_home_object +add_executable(test_hs_homeobject) +target_sources(test_hs_homeobject PRIVATE test_hs_homeobject.cpp) +target_link_libraries(test_hs_homeobject homeobject_homestore ${COMMON_TEST_DEPS} ) -add_test(NAME HomeObject COMMAND ${CMAKE_BINARY_DIR}/bin/test_home_object) -set_property(TEST HomeObject PROPERTY RUN_SERIAL 1) +add_test(NAME HSHomeObject COMMAND ${CMAKE_BINARY_DIR}/bin/test_hs_homeobject) +set_property(TEST HSHomeObject PROPERTY RUN_SERIAL 1) -add_executable (test_shard_manager) -target_sources(test_shard_manager PRIVATE test_shard_manager.cpp) -target_link_libraries(test_shard_manager +add_executable (hs_shard_tests) +target_sources(hs_shard_tests PRIVATE hs_shard_tests.cpp) +target_link_libraries(hs_shard_tests homeobject_homestore ${COMMON_TEST_DEPS} ) -add_test(NAME ShardManagerTest COMMAND ${CMAKE_BINARY_DIR}/bin/test_shard_manager) +add_test(NAME HSShardTests COMMAND ${CMAKE_BINARY_DIR}/bin/hs_shard_tests) +set_property(TEST HSShardTests PROPERTY RUN_SERIAL 1) add_executable (test_heap_chunk_selector) target_sources(test_heap_chunk_selector PRIVATE test_heap_chunk_selector.cpp ../heap_chunk_selector.cpp) diff --git a/src/lib/homestore_backend/tests/test_shard_manager.cpp b/src/lib/homestore_backend/tests/hs_shard_tests.cpp similarity index 100% rename from src/lib/homestore_backend/tests/test_shard_manager.cpp rename to src/lib/homestore_backend/tests/hs_shard_tests.cpp diff --git a/src/lib/homestore_backend/tests/test_home_object.cpp b/src/lib/homestore_backend/tests/test_hs_homeobject.cpp similarity index 100% rename from src/lib/homestore_backend/tests/test_home_object.cpp rename to src/lib/homestore_backend/tests/test_hs_homeobject.cpp From a2fb770bfb822f4753c0871cc3b065fb6ee764c5 Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Wed, 18 Oct 2023 08:27:01 -0700 Subject: [PATCH 2/6] Cleanup CMake --- CMakeLists.txt | 59 ++++++++++++++++++--------------------- cmake/mem_sanitizer.cmake | 2 -- conanfile.py | 2 +- src/CMakeLists.txt | 1 - 4 files changed, 28 insertions(+), 36 deletions(-) delete mode 100644 cmake/mem_sanitizer.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 1e1aaa61..28f3b7da 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,6 @@ cmake_minimum_required (VERSION 3.11) -project (homeobject) +project (homeobject LANGUAGES CXX) -include (cmake/Flags.cmake) set(CMAKE_CXX_STANDARD 20) if(EXISTS ${CMAKE_BINARY_DIR}/conanbuildinfo.cmake) @@ -11,53 +10,49 @@ else () message("The file conanbuildinfo.cmake doesn't exist, some properties will be unavailable") endif () -if (NOT DEFINED BUILD_COVERAGE) - set(BUILD_COVERAGE OFF) -endif() if (NOT DEFINED CMAKE_BUILD_TYPE) set (CMAKE_BUILD_TYPE "Debug") endif() -if (DEFINED BUILD_COVERAGE) - if (${BUILD_COVERAGE}) - include (cmake/CodeCoverage.cmake) - APPEND_COVERAGE_COMPILER_FLAGS() - endif () -endif () -if (NOT DEFINED MEMORY_SANITIZER_ON) - set(MEMORY_SANITIZER_ON OFF) -endif() -if (${MEMORY_SANITIZER_ON}) - include (cmake/mem_sanitizer.cmake) - message(STATUS "********* WARNING: Running with Memory Sanitizer ON *********") +include (cmake/Flags.cmake) +if (BUILD_TESTING) + enable_testing() + if ((DEFINED CODE_COVERAGE) AND (${CODE_COVERAGE})) + include (cmake/CodeCoverage.cmake) + APPEND_COVERAGE_COMPILER_FLAGS() + elseif ((DEFINED MEMORY_SANITIZER_ON) AND (${MEMORY_SANITIZER_ON})) + message(WARNING "********* Running with Memory Sanitizer *********") + add_flags("-fsanitize=address \ + -fsanitize=undefined \ + -fsanitize-address-use-after-scope \ + -fno-sanitize=alignment \ + -DCDS_ADDRESS_SANITIZER_ENABLED \ + -fno-omit-frame-pointer \ + -fno-optimize-sibling-calls " + ) + set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address -fsanitize=undefined") + endif() + find_package(GTest QUIET REQUIRED) endif() -find_program(CCACHE_FOUND ccache) +find_program(CCACHE_FOUND ccache QUIET) if (CCACHE_FOUND) - set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ccache) - set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ccache) + set_property(GLOBAL PROPERTY RULE_LAUNCH_COMPILE ccache) + set_property(GLOBAL PROPERTY RULE_LAUNCH_LINK ccache) endif () -if (BUILD_TESTING) -enable_testing() -endif() - -# add conan information +# add component version information add_flags("-DPACKAGE_NAME=${PROJECT_NAME}") -if (DEFINED CONAN_PACKAGE_VERSION) - message("Package Version: [${CONAN_PACKAGE_VERSION}]") - add_flags("-DPACKAGE_VERSION=\\\"${CONAN_PACKAGE_VERSION}\\\"") -else () - message("Unknown Package Version") - add_flags("-DPACKAGE_VERSION=\\\"${CONAN_PACKAGE_VERSION}\\\"") +if (NOT DEFINED CONAN_PACKAGE_VERSION) + set(CONAN_PACKAGE_VERSION "0.0.0") endif () +add_flags("-DPACKAGE_VERSION=${CONAN_PACKAGE_VERSION}") add_subdirectory(src) # build info string(TOUPPER "${CMAKE_BUILD_TYPE}" UC_CMAKE_BUILD_TYPE) message(STATUS "Build configuration: ${CMAKE_BUILD_TYPE}") -message(STATUS "C compiler info: ${CMAKE_C_COMPILER_ID} ${CMAKE_C_COMPILER_VERSION} ${CMAKE_C_COMPILER_EXTERNAL_TOOLCHAIN}") message(STATUS "C++ compiler info: ${CMAKE_CXX_COMPILER_ID} ${CMAKE_CXX_COMPILER_VERSION} ${CMAKE_CXX_COMPILER_EXTERNAL_TOOLCHAIN}") message(STATUS "C flags: ${CMAKE_C_FLAGS} ${CMAKE_C_FLAGS_${UC_CMAKE_BUILD_TYPE}}") message(STATUS "C++ flags: ${CMAKE_CXX_FLAGS} ${CMAKE_CXX_FLAGS_${UC_CMAKE_BUILD_TYPE}}") diff --git a/cmake/mem_sanitizer.cmake b/cmake/mem_sanitizer.cmake deleted file mode 100644 index cfe1e889..00000000 --- a/cmake/mem_sanitizer.cmake +++ /dev/null @@ -1,2 +0,0 @@ -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fsanitize=address -fsanitize=undefined -fsanitize-address-use-after-scope -fno-sanitize=alignment -DCDS_ADDRESS_SANITIZER_ENABLED -fno-omit-frame-pointer -fno-optimize-sibling-calls") -set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} -fsanitize=address -fsanitize=undefined") diff --git a/conanfile.py b/conanfile.py index eef11900..6e4ad169 100644 --- a/conanfile.py +++ b/conanfile.py @@ -73,7 +73,7 @@ def build(self): if self.options.sanitize: definitions['MEMORY_SANITIZER_ON'] = 'ON' elif self.options.coverage: - definitions['BUILD_COVERAGE'] = 'ON' + definitions['CODE_COVERAGE'] = 'ON' cmake = CMake(self) cmake.configure(defs=definitions) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 02db6580..a5aa4fe7 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -3,7 +3,6 @@ cmake_minimum_required (VERSION 3.11) find_package(Threads QUIET REQUIRED) find_package(sisl QUIET REQUIRED) find_package(homestore QUIET REQUIRED) -find_package(GTest QUIET REQUIRED) link_directories(${spdk_LIB_DIRS} ${dpdk_LIB_DIRS}) From 8573c7b547432abfd4dfb59bb6736c665e650f46 Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Wed, 18 Oct 2023 12:41:17 -0700 Subject: [PATCH 3/6] Fixup logging. --- src/lib/blob_route.hpp | 4 +-- src/lib/homestore_backend/CMakeLists.txt | 2 +- src/lib/homestore_backend/hs_blob_manager.cpp | 30 +++++++++---------- src/lib/homestore_backend/hs_homeobject.hpp | 12 ++++---- src/lib/homestore_backend/hs_pg_manager.cpp | 2 +- src/lib/homestore_backend/index_kv.cpp | 6 ++-- src/lib/homestore_backend/index_kv.hpp | 23 ++++++++++---- .../replication_state_machine.cpp | 1 - src/lib/memory_backend/mem_homeobject.hpp | 2 +- src/lib/tests/BlobManagerTest.cpp | 5 ++-- src/lib/tests/fixture_app.hpp | 7 ++++- 11 files changed, 55 insertions(+), 39 deletions(-) diff --git a/src/lib/blob_route.hpp b/src/lib/blob_route.hpp index a87f5bbf..adfcde10 100644 --- a/src/lib/blob_route.hpp +++ b/src/lib/blob_route.hpp @@ -15,9 +15,7 @@ struct BlobRoute { shard_id_t shard; blob_id_t blob; auto operator<=>(BlobRoute const&) const = default; - sisl::blob to_blob() const { - return sisl::blob{uintptr_cast(const_cast< BlobRoute* >(this)), sizeof(*this)}; - } + sisl::blob to_blob() const { return sisl::blob{uintptr_cast(const_cast< BlobRoute* >(this)), sizeof(*this)}; } }; #pragma pack() diff --git a/src/lib/homestore_backend/CMakeLists.txt b/src/lib/homestore_backend/CMakeLists.txt index 842f45f1..f55ae4da 100644 --- a/src/lib/homestore_backend/CMakeLists.txt +++ b/src/lib/homestore_backend/CMakeLists.txt @@ -40,5 +40,5 @@ target_link_libraries(blob_homestore_test homeobject_homestore ${COMMON_TEST_DEPS} ) -#add_test(NAME BlobHomestoreTest COMMAND blob_homestore_test -csv error) +add_test(NAME BlobHomestoreTest COMMAND blob_homestore_test -csv error --num-iters 100) endif() diff --git a/src/lib/homestore_backend/hs_blob_manager.cpp b/src/lib/homestore_backend/hs_blob_manager.cpp index a076ebe9..3b9bbe11 100644 --- a/src/lib/homestore_backend/hs_blob_manager.cpp +++ b/src/lib/homestore_backend/hs_blob_manager.cpp @@ -2,6 +2,7 @@ #include "replication_message.hpp" #include "replication_state_machine.hpp" #include "lib/homeobject_impl.hpp" +#include "lib/blob_route.hpp" SISL_LOGGING_DECL(blobmgr) @@ -128,7 +129,7 @@ void HSHomeObject::on_blob_put_commit(int64_t lsn, sisl::blob const& header, sis auto msg_header = r_cast< ReplicationMessageHeader* >(header.bytes); if (msg_header->corrupted()) { - LOGERROR("replication message header is corrupted with crc error, lsn:{}", lsn); + LOGE("replication message header is corrupted with crc error, lsn:{}", lsn); if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(BlobError::CHECKSUM_MISMATCH)); } return; } @@ -150,7 +151,7 @@ void HSHomeObject::on_blob_put_commit(int64_t lsn, sisl::blob const& header, sis // Write to index table with key {shard id, blob id } and value {pba}. auto r = add_to_index_table(index_table, blob_info); if (r.hasError()) { - LOGERROR("Failed to insert into index table for blob {} err {}", lsn, r.error()); + LOGE("Failed to insert into index table for blob {} err {}", lsn, r.error()); ctx->promise_.setValue(folly::makeUnexpected(r.error())); return; } @@ -176,7 +177,7 @@ BlobManager::AsyncResult< Blob > HSHomeObject::_get_blob(ShardInfo const& shard, auto r = get_from_index_table(index_table, shard.id, blob_id); if (!r) { - LOGWARN("Blob not found in index id {} shard {}", blob_id, shard.id); + LOGW("Blob not found in index id {} shard {}", blob_id, shard.id); return folly::makeUnexpected(r.error()); } @@ -193,20 +194,19 @@ BlobManager::AsyncResult< Blob > HSHomeObject::_get_blob(ShardInfo const& shard, .thenValue([this, blob_id, req_len, req_offset, shard, multi_blkids, iov_base](auto&& result) mutable -> BlobManager::AsyncResult< Blob > { if (result) { - LOGERROR("Failed to read blob {} shard {} err {}", blob_id, shard.id, result.value()); + LOGE("Failed to read blob {} shard {} err {}", blob_id, shard.id, result.value()); return folly::makeUnexpected(BlobError::READ_FAILED); } + auto const b_route = BlobRoute{blob_id, shard.id}; BlobHeader* header = (BlobHeader*)iov_base.get(); if (!header->valid()) { - LOGERROR("Invalid header found for blob {} shard {}", blob_id, shard.id); - LOGERROR("Blob header {}", header->to_string()); + LOGE("Invalid header found for [route={}] [header={}]", b_route, header->to_string()); return folly::makeUnexpected(BlobError::READ_FAILED); } if (header->shard_id != shard.id) { - LOGERROR("Invalid shard id found in header for blob {} shard {}", blob_id, shard.id); - LOGERROR("Blob header {}", header->to_string()); + LOGE("Invalid shard id found in header for [route={}] [header={}]", b_route, header->to_string()); return folly::makeUnexpected(BlobError::READ_FAILED); } @@ -225,15 +225,15 @@ BlobManager::AsyncResult< Blob > HSHomeObject::_get_blob(ShardInfo const& shard, compute_blob_payload_hash(header->hash_algorithm, blob_bytes, blob_size, user_key_bytes, user_key_size, computed_hash, BlobHeader::blob_max_hash_len); if (std::memcmp(computed_hash, header->hash, BlobHeader::blob_max_hash_len) != 0) { - LOGERROR("Hash mismatch for blob {} shard {}", blob_id, shard.id); - LOGERROR("Computed: {} Header: {}", hex_bytes(computed_hash, BlobHeader::blob_max_hash_len), - hex_bytes(header->hash, BlobHeader::blob_max_hash_len)); + LOGE("Hash mismatch for [route={}] [header={}] [comp={} != read={}]", b_route, header->to_string(), + hex_bytes(computed_hash, BlobHeader::blob_max_hash_len), + hex_bytes(header->hash, BlobHeader::blob_max_hash_len)); return folly::makeUnexpected(BlobError::CHECKSUM_MISMATCH); } if (req_offset + req_len > blob_size) { - LOGERROR("Invalid offset length request in get blob {} offset {} len {} size {}", blob_id, req_offset, - req_len, blob_size); + LOGE("Invalid offset length request in get blob {} offset {} len {} size {}", blob_id, req_offset, + req_len, blob_size); return folly::makeUnexpected(BlobError::INVALID_ARG); } @@ -265,7 +265,7 @@ homestore::blk_alloc_hints HSHomeObject::blob_put_get_blk_alloc_hints(sisl::blob auto msg_header = r_cast< ReplicationMessageHeader* >(header.bytes); if (msg_header->corrupted()) { - LOGERROR("replication message header is corrupted with crc error shard:{}", msg_header->shard_id); + LOGE("replication message header is corrupted with crc error shard:{}", msg_header->shard_id); if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(BlobError::CHECKSUM_MISMATCH)); } return {}; } @@ -275,7 +275,7 @@ homestore::blk_alloc_hints HSHomeObject::blob_put_get_blk_alloc_hints(sisl::blob RELEASE_ASSERT(shard_iter != _shard_map.end(), "Couldnt find shard id"); auto hs_shard = d_cast< HS_Shard* >((*shard_iter->second).get()); auto chunk_id = hs_shard->sb_->chunk_id; - LOGINFO("Got shard id {} chunk id {}", msg_header->shard_id, chunk_id); + LOGI("Got shard id {} chunk id {}", msg_header->shard_id, chunk_id); homestore::blk_alloc_hints hints; hints.chunk_id_hint = chunk_id; return hints; diff --git a/src/lib/homestore_backend/hs_homeobject.hpp b/src/lib/homestore_backend/hs_homeobject.hpp index 4db16ae1..27642521 100644 --- a/src/lib/homestore_backend/hs_homeobject.hpp +++ b/src/lib/homestore_backend/hs_homeobject.hpp @@ -4,13 +4,13 @@ #include #include +#include #include #include #include "heap_chunk_selector.h" #include "lib/homeobject_impl.hpp" #include "replication_message.hpp" -#include "index_kv.hpp" namespace homestore { struct meta_blk; @@ -21,6 +21,8 @@ namespace homeobject { std::string hex_bytes(uint8_t* bytes, size_t len); +class BlobRouteKey; +class BlobRouteValue; using BlobIndexTable = homestore::IndexTable< BlobRouteKey, BlobRouteValue >; class HSHomeObject : public HomeObjectImpl { @@ -126,9 +128,9 @@ class HSHomeObject : public HomeObjectImpl { bool valid() const { return magic == blob_header_magic || version <= blob_header_version; } std::string to_string() { - return fmt::format("magic={:#x} version={} algo={} hash={} shard={} blob_size={} user_size={}", magic, version, - (uint8_t)hash_algorithm, hex_bytes(hash, blob_max_hash_len), shard_id, blob_size, - user_key_size); + return fmt::format("magic={:#x} version={} algo={} hash={} shard={} blob_size={} user_size={}", magic, + version, (uint8_t)hash_algorithm, hex_bytes(hash, blob_max_hash_len), shard_id, + blob_size, user_key_size); } }; #pragma pack() @@ -207,7 +209,7 @@ class BlobIndexServiceCallbacks : public homestore::IndexServiceCallbacks { BlobIndexServiceCallbacks(HSHomeObject* home_object) : home_object_(home_object) {} std::shared_ptr< homestore::IndexTableBase > on_index_table_found(const homestore::superblk< homestore::index_table_sb >& sb) override { - LOGINFO("Recovered index table to index service"); + LOGI("Recovered index table to index service"); return home_object_->recover_index_table(sb); } diff --git a/src/lib/homestore_backend/hs_pg_manager.cpp b/src/lib/homestore_backend/hs_pg_manager.cpp index 68428991..95c61b05 100644 --- a/src/lib/homestore_backend/hs_pg_manager.cpp +++ b/src/lib/homestore_backend/hs_pg_manager.cpp @@ -63,7 +63,7 @@ PGManager::NullAsyncResult HSHomeObject::_create_pg(PGInfo&& pg_info, std::set< RELEASE_ASSERT(index_table_pg_map_.count(uuid_str) == 0, "duplicate index table found"); index_table_pg_map_[uuid_str] = PgIndexTable{pg_id, index_table}; - LOGINFO("Index table created for pg {} uuid {}", pg_id, uuid_str); + LOGI("Index table created for pg {} uuid {}", pg_id, uuid_str); hs_pg->index_table_ = index_table; // Add to index service, so that it gets cleaned up when index service is shutdown. homestore::hs()->index_service().add_index_table(index_table); diff --git a/src/lib/homestore_backend/index_kv.cpp b/src/lib/homestore_backend/index_kv.cpp index 0a913cfc..4fbe7531 100644 --- a/src/lib/homestore_backend/index_kv.cpp +++ b/src/lib/homestore_backend/index_kv.cpp @@ -66,7 +66,7 @@ HSHomeObject::get_from_index_table(shared< BlobIndexTable > index_table, shard_i homestore::BtreeSingleGetRequest get_req{&index_key, &index_value}; auto status = index_table->get(get_req); if (status != homestore::btree_status_t::success) { - LOGERROR("Failed to get from index table {}", index_key.to_string()); + LOGERROR("Failed to get from index table [route={}]", index_key); return folly::makeUnexpected(BlobError::UNKNOWN_BLOB); } @@ -78,12 +78,12 @@ void HSHomeObject::print_btree_index(pg_id_t pg_id) { { std::shared_lock lock_guard(_pg_lock); auto iter = _pg_map.find(pg_id); - RELEASE_ASSERT (iter != _pg_map.end(), "Unknown PG"); + RELEASE_ASSERT(iter != _pg_map.end(), "Unknown PG"); index_table = static_cast< HS_PG* >(iter->second.get())->index_table_; RELEASE_ASSERT(index_table != nullptr, "Index table not intialized"); } - LOGINFO("Index UUID {}", boost::uuids::to_string(index_table->uuid())); + LOGI("Index UUID {}", boost::uuids::to_string(index_table->uuid())); index_table->print_tree(); } diff --git a/src/lib/homestore_backend/index_kv.hpp b/src/lib/homestore_backend/index_kv.hpp index 047f5c8a..7cdc9302 100644 --- a/src/lib/homestore_backend/index_kv.hpp +++ b/src/lib/homestore_backend/index_kv.hpp @@ -1,7 +1,6 @@ #pragma once #include -#include #include #include #include @@ -14,7 +13,6 @@ class BlobRouteKey : public homestore::BtreeKey { BlobRoute key_; public: - BlobRouteKey() = default; BlobRouteKey(const BlobRoute key) : key_(key) {} BlobRouteKey(const BlobRouteKey& other) : BlobRouteKey(other.serialize(), true) {} @@ -27,7 +25,7 @@ class BlobRouteKey : public homestore::BtreeKey { }; virtual void clone(const homestore::BtreeKey& other) override { key_ = ((BlobRouteKey&)other).key_; } - virtual ~BlobRouteKey() = default; + ~BlobRouteKey() override = default; int compare(const homestore::BtreeKey& o) const override { const BlobRouteKey& other = s_cast< const BlobRouteKey& >(o); @@ -76,9 +74,7 @@ class BlobRouteValue : public homestore::BtreeValue { } uint32_t serialized_size() const override { return pbas_.serialized_size(); } - static uint32_t get_fixed_size() { - return homestore::MultiBlkId::expected_serialized_size(1 /* num_pieces */); - } + static uint32_t get_fixed_size() { return homestore::MultiBlkId::expected_serialized_size(1 /* num_pieces */); } void deserialize(const sisl::blob& b, bool copy) override { pbas_.deserialize(b, copy); } std::string to_string() const override { return fmt::format("{}", pbas_.to_string()); } @@ -94,3 +90,18 @@ class BlobRouteValue : public homestore::BtreeValue { }; } // namespace homeobject + +namespace fmt { +template <> +struct formatter< homeobject::BlobRouteKey > { + template < typename ParseContext > + constexpr auto parse(ParseContext& ctx) { + return ctx.begin(); + } + + template < typename FormatContext > + auto format(homeobject::BlobRouteKey const& r, FormatContext& ctx) { + return format_to(ctx.out(), "{}", r.key()); + } +}; +} // namespace fmt diff --git a/src/lib/homestore_backend/replication_state_machine.cpp b/src/lib/homestore_backend/replication_state_machine.cpp index fcb202e7..28b4d9d1 100644 --- a/src/lib/homestore_backend/replication_state_machine.cpp +++ b/src/lib/homestore_backend/replication_state_machine.cpp @@ -25,7 +25,6 @@ void ReplicationStateMachine::on_commit(int64_t lsn, const sisl::blob& header, c } } - bool ReplicationStateMachine::on_pre_commit(int64_t lsn, sisl::blob const&, sisl::blob const&, cintrusive< homestore::repl_req_ctx >&) { LOGI("on_pre_commit with lsn:{}", lsn); diff --git a/src/lib/memory_backend/mem_homeobject.hpp b/src/lib/memory_backend/mem_homeobject.hpp index b661cdd2..d1520961 100644 --- a/src/lib/memory_backend/mem_homeobject.hpp +++ b/src/lib/memory_backend/mem_homeobject.hpp @@ -41,7 +41,7 @@ class MemoryHomeObject : public HomeObjectImpl { // BlobManager BlobManager::AsyncResult< blob_id_t > _put_blob(ShardInfo const&, Blob&&) override; BlobManager::AsyncResult< Blob > _get_blob(ShardInfo const&, blob_id_t, uint64_t off = 0, - uint64_t len = 0) const override; + uint64_t len = 0) const override; BlobManager::NullAsyncResult _del_blob(ShardInfo const&, blob_id_t) override; /// diff --git a/src/lib/tests/BlobManagerTest.cpp b/src/lib/tests/BlobManagerTest.cpp index f860ae98..66c75b22 100644 --- a/src/lib/tests/BlobManagerTest.cpp +++ b/src/lib/tests/BlobManagerTest.cpp @@ -15,7 +15,8 @@ TEST_F(TestFixture, BasicTests) { for (auto k = 0; batch_sz > k; ++k) { t_v.push_back(std::thread([this, &call_lock, &calls, batch_sz]() mutable { auto our_calls = std::list< folly::SemiFuture< folly::Unit > >(); - for (auto i = _blob_id + _shard_2.id + 1; (_blob_id + _shard_1.id + 1) + ((100 * Ki) / batch_sz) > i; ++i) { + for (auto i = _blob_id + _shard_2.id + 1; + (_blob_id + _shard_1.id + 1) + (SISL_OPTIONS["num_iters"].as< uint64_t >() / batch_sz) > i; ++i) { our_calls.push_back(homeobj_->blob_manager()->get(_shard_1.id, _blob_id).deferValue([](auto const& e) { EXPECT_TRUE(!!e); e.then([](auto const& blob) { @@ -67,7 +68,7 @@ TEST_F(TestFixture, BasicTests) { int main(int argc, char* argv[]) { int parsed_argc = argc; ::testing::InitGoogleTest(&parsed_argc, argv); - SISL_OPTIONS_LOAD(parsed_argc, argv, logging); + SISL_OPTIONS_LOAD(parsed_argc, argv, logging, blob_manager_test); sisl::logging::SetLogger(std::string(argv[0])); spdlog::set_pattern("[%D %T.%e] [%n] [%^%l%$] [%t] %v"); parsed_argc = 1; diff --git a/src/lib/tests/fixture_app.hpp b/src/lib/tests/fixture_app.hpp index 1ce6fe28..11c15582 100644 --- a/src/lib/tests/fixture_app.hpp +++ b/src/lib/tests/fixture_app.hpp @@ -16,7 +16,12 @@ #include SISL_LOGGING_INIT(logging, HOMEOBJECT_LOG_MODS) -SISL_OPTIONS_ENABLE(logging) + +SISL_OPTION_GROUP(blob_manager_test, + (num_iters, "", "num_iters", "number of iterations per loop", + ::cxxopts::value< uint64_t >()->default_value("100000"), "number")); + +SISL_OPTIONS_ENABLE(logging, blob_manager_test) using homeobject::Blob; using homeobject::blob_id_t; From 17b826a3e32377eb2ac6b1bcb054cfe086ffb02a Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Wed, 18 Oct 2023 12:46:20 -0700 Subject: [PATCH 4/6] Disable blob tests. --- src/lib/homestore_backend/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/homestore_backend/CMakeLists.txt b/src/lib/homestore_backend/CMakeLists.txt index f55ae4da..736b9dfb 100644 --- a/src/lib/homestore_backend/CMakeLists.txt +++ b/src/lib/homestore_backend/CMakeLists.txt @@ -40,5 +40,5 @@ target_link_libraries(blob_homestore_test homeobject_homestore ${COMMON_TEST_DEPS} ) -add_test(NAME BlobHomestoreTest COMMAND blob_homestore_test -csv error --num-iters 100) +#add_test(NAME BlobHomestoreTest COMMAND blob_homestore_test -csv error --num_iters 100) endif() From 77ad983aaf1e53af2965006947198836d79213c5 Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Wed, 18 Oct 2023 12:58:23 -0700 Subject: [PATCH 5/6] Improve bin-to-hex --- src/lib/homestore_backend/hs_blob_manager.cpp | 5 ++--- src/lib/homestore_backend/hs_homeobject.cpp | 14 +++++--------- 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/lib/homestore_backend/hs_blob_manager.cpp b/src/lib/homestore_backend/hs_blob_manager.cpp index 3b9bbe11..828d15df 100644 --- a/src/lib/homestore_backend/hs_blob_manager.cpp +++ b/src/lib/homestore_backend/hs_blob_manager.cpp @@ -225,9 +225,8 @@ BlobManager::AsyncResult< Blob > HSHomeObject::_get_blob(ShardInfo const& shard, compute_blob_payload_hash(header->hash_algorithm, blob_bytes, blob_size, user_key_bytes, user_key_size, computed_hash, BlobHeader::blob_max_hash_len); if (std::memcmp(computed_hash, header->hash, BlobHeader::blob_max_hash_len) != 0) { - LOGE("Hash mismatch for [route={}] [header={}] [comp={} != read={}]", b_route, header->to_string(), - hex_bytes(computed_hash, BlobHeader::blob_max_hash_len), - hex_bytes(header->hash, BlobHeader::blob_max_hash_len)); + LOGE("Hash mismatch for [route={}] [header={}] [computed={}]", b_route, header->to_string(), + hex_bytes(computed_hash, BlobHeader::blob_max_hash_len)); return folly::makeUnexpected(BlobError::CHECKSUM_MISMATCH); } diff --git a/src/lib/homestore_backend/hs_homeobject.cpp b/src/lib/homestore_backend/hs_homeobject.cpp index b5c96207..212d7b63 100644 --- a/src/lib/homestore_backend/hs_homeobject.cpp +++ b/src/lib/homestore_backend/hs_homeobject.cpp @@ -1,3 +1,7 @@ +#include "hs_homeobject.hpp" + +#include + #include #include #include @@ -5,7 +9,6 @@ #include #include -#include "hs_homeobject.hpp" #include "heap_chunk_selector.h" #include "index_kv.hpp" @@ -122,13 +125,6 @@ void HSHomeObject::on_shard_meta_blk_recover_completed(bool success) { chunk_selector_->build_per_dev_chunk_heap(excluding_chunks); } -std::string hex_bytes(uint8_t* bytes, size_t len) { - std::stringstream ss; - ss << std::hex; - for (size_t i = 0; i < len; i++) { - ss << std::setw(2) << std::setfill('0') << (int)bytes[i]; - } - return ss.str(); -} +std::string hex_bytes(uint8_t* bytes, size_t len) { return fmt::format("{}", spdlog::to_hex(bytes, bytes + len)); } } // namespace homeobject From 41fb51fe4c56dcee4eafcb29156f8525a2a0984d Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Wed, 18 Oct 2023 13:11:10 -0700 Subject: [PATCH 6/6] Remove redundant tests. --- src/lib/homestore_backend/CMakeLists.txt | 3 + .../tests/hs_shard_tests.cpp | 178 ++---------------- 2 files changed, 19 insertions(+), 162 deletions(-) diff --git a/src/lib/homestore_backend/CMakeLists.txt b/src/lib/homestore_backend/CMakeLists.txt index 736b9dfb..47dc29e3 100644 --- a/src/lib/homestore_backend/CMakeLists.txt +++ b/src/lib/homestore_backend/CMakeLists.txt @@ -25,6 +25,7 @@ target_link_libraries(pg_homestore_test ${COMMON_TEST_DEPS} ) add_test(NAME PGHomestoreTest COMMAND pg_homestore_test -csv error) +set_property(TEST PGHomestoreTest PROPERTY RUN_SERIAL 1) add_executable (shard_homestore_test) target_sources(shard_homestore_test PRIVATE $) @@ -33,6 +34,7 @@ target_link_libraries(shard_homestore_test ${COMMON_TEST_DEPS} ) add_test(NAME ShardHomestoreTest COMMAND shard_homestore_test -csv error) +set_property(TEST ShardHomestoreTest PROPERTY RUN_SERIAL 1) add_executable (blob_homestore_test) target_sources(blob_homestore_test PRIVATE $) @@ -41,4 +43,5 @@ target_link_libraries(blob_homestore_test ${COMMON_TEST_DEPS} ) #add_test(NAME BlobHomestoreTest COMMAND blob_homestore_test -csv error --num_iters 100) +#set_property(TEST BlobHomestoreTest PROPERTY RUN_SERIAL 1) endif() diff --git a/src/lib/homestore_backend/tests/hs_shard_tests.cpp b/src/lib/homestore_backend/tests/hs_shard_tests.cpp index 4ae77f7a..df7696b0 100644 --- a/src/lib/homestore_backend/tests/hs_shard_tests.cpp +++ b/src/lib/homestore_backend/tests/hs_shard_tests.cpp @@ -19,118 +19,19 @@ #include "lib/homestore_backend/replication_message.hpp" #include "lib/homestore_backend/replication_state_machine.hpp" +#include "lib/tests/fixture_app.hpp" + using homeobject::shard_id_t; using homeobject::ShardError; using homeobject::ShardInfo; using homeobject::ShardManager; -SISL_LOGGING_INIT(logging, HOMEOBJECT_LOG_MODS) -SISL_OPTIONS_ENABLE(logging) - -class FixtureApp : public homeobject::HomeObjectApplication { -private: - std::string fpath_{"/tmp/test_shard_manager.data.{}" + std::to_string(rand())}; - -public: - bool spdk_mode() const override { return false; } - uint32_t threads() const override { return 2; } - std::list< std::filesystem::path > devices() const override { - LOGI("creating {} device file with size={}", fpath_, homestore::in_bytes(2 * Gi)); - if (std::filesystem::exists(fpath_)) { std::filesystem::remove(fpath_); } - std::ofstream ofs{fpath_, std::ios::binary | std::ios::out | std::ios::trunc}; - std::filesystem::resize_file(fpath_, 2 * Gi); - - auto device_info = std::list< std::filesystem::path >(); - device_info.emplace_back(std::filesystem::canonical(fpath_)); - return device_info; - } - - ~FixtureApp() { - if (std::filesystem::exists(fpath_)) { std::filesystem::remove(fpath_); } - } - - homeobject::peer_id_t discover_svcid(std::optional< homeobject::peer_id_t > const&) const override { - return boost::uuids::random_generator()(); - } - std::string lookup_peer(homeobject::peer_id_t const&) const override { return "test_fixture.com"; } -}; - -class ShardManagerTesting : public ::testing::Test { -public: - homeobject::pg_id_t _pg_id{1u}; - homeobject::peer_id_t _peer1; - homeobject::peer_id_t _peer2; - homeobject::shard_id_t _shard_id{100u}; - - void SetUp() override { - app = std::make_shared< FixtureApp >(); - _home_object = homeobject::init_homeobject(std::weak_ptr< homeobject::HomeObjectApplication >(app)); - _peer1 = _home_object->our_uuid(); - _peer2 = boost::uuids::random_generator()(); - - auto info = homeobject::PGInfo(_pg_id); - 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()); - } - -protected: - std::shared_ptr< FixtureApp > app; - std::shared_ptr< homeobject::HomeObject > _home_object; -}; - -TEST_F(ShardManagerTesting, CreateShardTooBig) { - EXPECT_EQ(ShardError::INVALID_ARG, - _home_object->shard_manager() - ->create_shard(_pg_id, homeobject::ShardManager::max_shard_size() + 1) - .get() - .error()); -} - -TEST_F(ShardManagerTesting, CreateShardTooSmall) { - EXPECT_EQ(ShardError::INVALID_ARG, _home_object->shard_manager()->create_shard(_pg_id, 0ul).get().error()); -} - -TEST_F(ShardManagerTesting, CreateShardWithUnknownPG) { - EXPECT_EQ(ShardError::UNKNOWN_PG, _home_object->shard_manager()->create_shard(_pg_id + 1, Mi).get().error()); -} - -TEST_F(ShardManagerTesting, GetUnknownShard) { - EXPECT_EQ(ShardError::UNKNOWN_SHARD, _home_object->shard_manager()->get_shard(_shard_id).get().error()); -} - -TEST_F(ShardManagerTesting, ListShardsNoPg) { - EXPECT_EQ(ShardError::UNKNOWN_PG, _home_object->shard_manager()->list_shards(_pg_id + 1).get().error()); -} - -TEST_F(ShardManagerTesting, ListShardsOnEmptyPg) { - auto e = _home_object->shard_manager()->list_shards(_pg_id).get(); - ASSERT_TRUE(!!e); - e.then([this](auto const& info_list) { ASSERT_EQ(info_list.size(), 0); }); -} - -TEST_F(ShardManagerTesting, CreateShardSuccess) { - auto e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get(); - ASSERT_TRUE(!!e); - ShardInfo shard_info = e.value(); - EXPECT_EQ(ShardInfo::State::OPEN, shard_info.state); - EXPECT_EQ(Mi, shard_info.total_capacity_bytes); - EXPECT_EQ(Mi, shard_info.available_capacity_bytes); - EXPECT_EQ(0ul, shard_info.deleted_capacity_bytes); - 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); +TEST_F(TestFixture, CreateMultiShards) { + homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(homeobj_.get()); + auto chunk_num_1 = ho->get_shard_chunk(_shard_1.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); + auto chunk_num_2 = ho->get_shard_chunk(_shard_2.id); ASSERT_TRUE(chunk_num_2.has_value()); // check if both chunk is on the same pdev; @@ -141,28 +42,28 @@ TEST_F(ShardManagerTesting, CreateMultiShards) { ASSERT_TRUE(alloc_hint1.pdev_id_hint.value() == alloc_hint2.pdev_id_hint.value()); } -TEST_F(ShardManagerTesting, CreateMultiShardsOnMultiPG) { +TEST_F(TestFixture, CreateMultiShardsOnMultiPG) { // create another PG; - auto peer1 = _home_object->our_uuid(); + auto peer1 = homeobj_->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()); + EXPECT_TRUE(homeobj_->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(); + auto e = homeobj_->shard_manager()->create_shard(pg, Mi).get(); ASSERT_TRUE(!!e); - homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(_home_object.get()); + homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(homeobj_.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(); + e = homeobj_->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()); @@ -176,55 +77,8 @@ TEST_F(ShardManagerTesting, CreateMultiShardsOnMultiPG) { } } -TEST_F(ShardManagerTesting, CreateShardAndValidateMembers) { - auto e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get(); - ASSERT_TRUE(!!e); - ShardInfo shard_info = e.value(); - homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(_home_object.get()); - EXPECT_TRUE(ho != nullptr); - auto pg_iter = ho->_pg_map.find(_pg_id); - EXPECT_TRUE(pg_iter != ho->_pg_map.end()); - auto& pg = pg_iter->second; - EXPECT_TRUE(pg->shard_sequence_num_ == 1); - EXPECT_EQ(1, pg->shards_.size()); - auto& shard = *pg->shards_.begin(); - EXPECT_TRUE(shard->info == shard_info); -} - -TEST_F(ShardManagerTesting, GetKnownShard) { - auto e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get(); - ASSERT_TRUE(!!e); - ShardInfo shard_info = e.value(); - auto future = _home_object->shard_manager()->get_shard(shard_info.id).get(); - future.then([this, shard_info](auto const& info) { - EXPECT_TRUE(info.id == shard_info.id); - EXPECT_TRUE(info.placement_group == _pg_id); - EXPECT_EQ(info.state, ShardInfo::State::OPEN); - }); -} - -TEST_F(ShardManagerTesting, ListShards) { - auto e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get(); - ASSERT_TRUE(!!e); - ShardInfo shard_info = e.value(); - auto list_shard_result = _home_object->shard_manager()->list_shards(_pg_id).get(); - ASSERT_TRUE(!!list_shard_result); - list_shard_result.then([this, shard_info](auto const& info_list) { - ASSERT_EQ(info_list.size(), 1); - EXPECT_TRUE(info_list.begin()->id == shard_info.id); - EXPECT_TRUE(info_list.begin()->placement_group == _pg_id); - EXPECT_EQ(info_list.begin()->state, ShardInfo::State::OPEN); - }); -} - -TEST_F(ShardManagerTesting, SealUnknownShard) { - EXPECT_EQ(ShardError::UNKNOWN_SHARD, _home_object->shard_manager()->seal_shard(1000).get().error()); -} - -TEST_F(ShardManagerTesting, MockSealShard) { - auto e = _home_object->shard_manager()->create_shard(_pg_id, Mi).get(); - ASSERT_TRUE(!!e); - ShardInfo shard_info = e.value(); +TEST_F(TestFixture, MockSealShard) { + ShardInfo shard_info = _shard_1; shard_info.state = ShardInfo::State::SEALED; nlohmann::json j; @@ -238,7 +92,7 @@ TEST_F(ShardManagerTesting, MockSealShard) { j["shard_info"]["deleted_capacity"] = shard_info.deleted_capacity_bytes; auto seal_shard_msg = j.dump(); - homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(_home_object.get()); + homeobject::HSHomeObject* ho = dynamic_cast< homeobject::HSHomeObject* >(homeobj_.get()); auto* pg = s_cast< homeobject::HSHomeObject::HS_PG* >(ho->_pg_map[_pg_id].get()); auto repl_dev = pg->repl_dev_; const auto msg_size = sisl::round_up(seal_shard_msg.size(), repl_dev->get_blk_size()); @@ -269,7 +123,7 @@ TEST_F(ShardManagerTesting, MockSealShard) { auto pg_iter = ho->_pg_map.find(_pg_id); EXPECT_TRUE(pg_iter != ho->_pg_map.end()); auto& pg_result = pg_iter->second; - EXPECT_EQ(1, pg_result->shards_.size()); + EXPECT_EQ(2, pg_result->shards_.size()); auto& check_shard = pg_result->shards_.front(); EXPECT_EQ(ShardInfo::State::SEALED, check_shard->info.state); }