From 8b434ad1c985123895a178e3cfee6afc38d7fe1c Mon Sep 17 00:00:00 2001 From: Brian Szmyd Date: Thu, 28 Sep 2023 19:04:52 -0700 Subject: [PATCH] Need to call discover for SM tests to work. --- .clang-format | 1 - src/lib/homeobject_impl.hpp | 7 +++++++ src/lib/homestore_backend/heap_chunk_selector.cpp | 12 ++++++------ src/lib/homestore_backend/hs_homeobject.cpp | 8 ++++---- src/lib/homestore_backend/hs_pg_manager.cpp | 4 ++-- src/lib/homestore_backend/hs_shard_manager.cpp | 10 +++++----- .../homestore_backend/replication_state_machine.cpp | 6 +++--- .../homestore_backend/tests/test_home_object.cpp | 2 +- .../homestore_backend/tests/test_shard_manager.cpp | 2 +- src/lib/memory_backend/mem_blob_manager.cpp | 9 ++++----- src/lib/memory_backend/mem_homeobject.cpp | 13 +++---------- src/lib/memory_backend/mem_homeobject.hpp | 2 +- src/lib/pg_manager.cpp | 8 ++++---- src/lib/shard_manager.cpp | 2 +- 14 files changed, 42 insertions(+), 44 deletions(-) diff --git a/.clang-format b/.clang-format index 2f771200..b7bf369b 100644 --- a/.clang-format +++ b/.clang-format @@ -16,7 +16,6 @@ AlignConsecutiveDeclarations: false AlignEscapedNewlines: Right AlignOperands: false AlignTrailingComments: true -AllowShortBlocksOnASingleLine: true AllowShortIfStatementsOnASingleLine: true AllowShortBlocksOnASingleLine: true AllowShortCaseLabelsOnASingleLine: false diff --git a/src/lib/homeobject_impl.hpp b/src/lib/homeobject_impl.hpp index 1a37b8b7..9b0be613 100644 --- a/src/lib/homeobject_impl.hpp +++ b/src/lib/homeobject_impl.hpp @@ -7,6 +7,13 @@ #include +#define LOGT(...) LOGTRACEMOD(homeobject, ##__VA_ARGS__) +#define LOGD(...) LOGDEBUGMOD(homeobject, ##__VA_ARGS__) +#define LOGI(...) LOGINFOMOD(homeobject, ##__VA_ARGS__) +#define LOGW(...) LOGWARNMOD(homeobject, ##__VA_ARGS__) +#define LOGE(...) LOGERRORMOD(homeobject, ##__VA_ARGS__) +#define LOGC(...) LOGCRITICALMOD(homeobject, ##__VA_ARGS__) + namespace homestore { class ReplicationService; } diff --git a/src/lib/homestore_backend/heap_chunk_selector.cpp b/src/lib/homestore_backend/heap_chunk_selector.cpp index 2d3cd6ca..b3f17fca 100644 --- a/src/lib/homestore_backend/heap_chunk_selector.cpp +++ b/src/lib/homestore_backend/heap_chunk_selector.cpp @@ -22,7 +22,7 @@ void HeapChunkSelector::add_chunk(csharedChunk& chunk) { m_chunks.emplace(VChunk void HeapChunkSelector::add_chunk_internal(const chunk_num_t chunkID) { if (m_chunks.find(chunkID) == m_chunks.end()) { // sanity check - LOGWARN("No chunk found for ChunkID {}", chunkID); + LOGWARNMOD(homeobject, "No chunk found for ChunkID {}", chunkID); return; } const auto& chunk = m_chunks[chunkID]; @@ -44,7 +44,7 @@ void HeapChunkSelector::add_chunk_internal(const chunk_num_t chunkID) { csharedChunk HeapChunkSelector::select_chunk(homestore::blk_count_t count, const homestore::blk_alloc_hints& hint) { auto& chunkIdHint = hint.chunk_id_hint; if (chunkIdHint.has_value()) { - LOGWARN("should not allocated a chunk with exiting chunk_id {} in hint!", chunkIdHint.value()); + LOGWARNMOD(homeobject, "should not allocated a chunk with exiting chunk_id {} in hint!", chunkIdHint.value()); return nullptr; } @@ -62,7 +62,7 @@ csharedChunk HeapChunkSelector::select_chunk(homestore::blk_count_t count, const return lhs.second->available_blk_count.load() < rhs.second->available_blk_count.load(); }); if (it == m_per_dev_heap.end()) { - LOGWARN("No pdev found for new pg"); + LOGWARNMOD(homeobject, "No pdev found for new pg"); return nullptr; } pdevID = it->first; @@ -72,7 +72,7 @@ csharedChunk HeapChunkSelector::select_chunk(homestore::blk_count_t count, const auto it = m_per_dev_heap.find(pdevID); if (it == m_per_dev_heap.end()) { - LOGWARN("No pdev found for pdev {}", pdevID); + LOGWARNMOD(homeobject, "No pdev found for pdev {}", pdevID); return nullptr; } @@ -90,7 +90,7 @@ csharedChunk HeapChunkSelector::select_chunk(homestore::blk_count_t count, const auto& avalableBlkCounter = it->second->available_blk_count; avalableBlkCounter.fetch_sub(vchunk.available_blks()); } else { - LOGWARN("No pdev found for pdev {}", pdevID); + LOGWARNMOD(homeobject, "No pdev found for pdev {}", pdevID); } return vchunk.get_internal_chunk(); @@ -106,7 +106,7 @@ void HeapChunkSelector::release_chunk(const chunk_num_t chunkID) { const auto& it = m_chunks.find(chunkID); if (it == m_chunks.end()) { // sanity check - LOGWARN("No chunk found for ChunkID {}", chunkID); + LOGWARNMOD(homeobject, "No chunk found for ChunkID {}", chunkID); } else { add_chunk_internal(chunkID); } diff --git a/src/lib/homestore_backend/hs_homeobject.cpp b/src/lib/homestore_backend/hs_homeobject.cpp index 92e6e047..7376d4e1 100644 --- a/src/lib/homestore_backend/hs_homeobject.cpp +++ b/src/lib/homestore_backend/hs_homeobject.cpp @@ -13,7 +13,7 @@ namespace homeobject { const std::string HSHomeObject::s_shard_info_sub_type = "shard_info"; extern std::shared_ptr< HomeObject > init_homeobject(std::weak_ptr< HomeObjectApplication >&& application) { - LOGINFOMOD(homeobject, "Initializing HomeObject"); + LOGI("Initializing HomeObject"); auto instance = std::make_shared< HSHomeObject >(std::move(application)); instance->init_homestore(); return instance; @@ -28,12 +28,12 @@ void HSHomeObject::init_homestore() { auto app = _application.lock(); RELEASE_ASSERT(app, "HomeObjectApplication lifetime unexpected!"); - LOGINFO("Starting iomgr with {} threads, spdk: {}", app->threads(), false); + LOGI("Starting iomgr with {} threads, spdk: {}", app->threads(), false); ioenvironment.with_iomgr(iomgr::iomgr_params{.num_threads = app->threads(), .is_spdk = app->spdk_mode()}); /// TODO Where should this come from? const uint64_t app_mem_size = 2 * Gi; - LOGINFO("Initialize and start HomeStore with app_mem_size = {}", homestore::in_bytes(app_mem_size)); + LOGI("Initialize and start HomeStore with app_mem_size = {}", homestore::in_bytes(app_mem_size)); std::vector< homestore::dev_info > device_info; for (auto const& path : app->devices()) { @@ -47,7 +47,7 @@ void HSHomeObject::init_homestore() { .start(hs_input_params{.devices = device_info, .app_mem_size = app_mem_size}, [this]() { register_homestore_metablk_callback(); }); if (need_format) { - LOGWARN("Seems like we are booting/starting first time, Formatting!!"); + LOGW("Seems like we are booting/starting first time, Formatting!!"); HomeStore::instance()->format_and_start({ {HS_SERVICE::META, hs_format_params{.size_pct = 5.0}}, {HS_SERVICE::LOG_REPLICATED, hs_format_params{.size_pct = 10.0}}, diff --git a/src/lib/homestore_backend/hs_pg_manager.cpp b/src/lib/homestore_backend/hs_pg_manager.cpp index 63a85a6b..2dfe776a 100644 --- a/src/lib/homestore_backend/hs_pg_manager.cpp +++ b/src/lib/homestore_backend/hs_pg_manager.cpp @@ -117,7 +117,7 @@ void HSHomeObject::on_pg_meta_blk_found(sisl::byte_view const& buf, void* meta_c .thenValue([this, pg_sb = std::move(pg_sb)](auto&& v) { if (v.hasError()) { // TODO: We need to raise an alert here, since without pg repl_dev all operations on that pg will fail - LOGERROR("open_repl_dev for group_id={} has failed", boost::uuids::to_string(pg_sb->replica_set_uuid)); + LOGE("open_repl_dev for group_id={} has failed", boost::uuids::to_string(pg_sb->replica_set_uuid)); return; } add_pg_to_map(std::make_shared< HS_PG >(pg_sb, std::move(v.value()))); @@ -154,4 +154,4 @@ HSHomeObject::HS_PG::HS_PG(homestore::superblk< HSHomeObject::pg_info_superblk > shared< homestore::ReplDev > rdev) : PG{pg_info_from_sb(sb)}, pg_sb_{sb}, repl_dev_{std::move(rdev)} {} -} // namespace homeobject \ No newline at end of file +} // namespace homeobject diff --git a/src/lib/homestore_backend/hs_shard_manager.cpp b/src/lib/homestore_backend/hs_shard_manager.cpp index ba3620b9..0c49480a 100644 --- a/src/lib/homestore_backend/hs_shard_manager.cpp +++ b/src/lib/homestore_backend/hs_shard_manager.cpp @@ -63,13 +63,13 @@ ShardManager::Result< ShardInfo > HSHomeObject::_create_shard(pg_id_t pg_owner, std::shared_lock lock_guard(_pg_lock); auto iter = _pg_map.find(pg_owner); if (iter == _pg_map.end()) { - LOGWARN("failed to create shard with non-exist pg [{}]", pg_owner); + LOGW("failed to create shard with non-exist pg [{}]", pg_owner); return folly::makeUnexpected(ShardError::UNKNOWN_PG); } repl_dev = std::static_pointer_cast< HS_PG >(iter->second)->repl_dev_; } if (!repl_dev) { - LOGWARN("failed to get repl dev instance for pg [{}]", pg_owner); + LOGW("failed to get repl dev instance for pg [{}]", pg_owner); return folly::makeUnexpected(ShardError::PG_NOT_READY); } @@ -109,7 +109,7 @@ bool HSHomeObject::precheck_and_decode_shard_msg(int64_t lsn, sisl::blob const& std::string* msg) { const ReplicationMessageHeader* msg_header = r_cast< const ReplicationMessageHeader* >(header.bytes); if (msg_header->header_crc != msg_header->calculate_crc()) { - LOGWARN("replication message header is corrupted with crc error, lsn:{}", lsn); + LOGW("replication message header is corrupted with crc error, lsn:{}", lsn); return false; } @@ -118,7 +118,7 @@ bool HSHomeObject::precheck_and_decode_shard_msg(int64_t lsn, sisl::blob const& auto crc = crc32_ieee(init_crc32, r_cast< const uint8_t* >(shard_msg.c_str()), shard_msg.size()); if (msg_header->payload_crc != crc) { - LOGWARN("replication message body is corrupted with crc error, lsn:{}", lsn); + LOGW("replication message body is corrupted with crc error, lsn:{}", lsn); return false; } *msg = std::move(shard_msg); @@ -187,7 +187,7 @@ void HSHomeObject::on_shard_message_commit(int64_t lsn, sisl::blob const& header std::scoped_lock lock_guard(_flying_shard_lock); auto iter = _flying_shards.find(lsn); if (iter == _flying_shards.end()) { - LOGWARN("can not find flying shards on lsn {}", lsn); + LOGW("can not find flying shards on lsn {}", lsn); if (ctx) { ctx->promise_.setValue(folly::makeUnexpected(ShardError::UNKNOWN)); } return; } diff --git a/src/lib/homestore_backend/replication_state_machine.cpp b/src/lib/homestore_backend/replication_state_machine.cpp index e9ebe6a8..63aca860 100644 --- a/src/lib/homestore_backend/replication_state_machine.cpp +++ b/src/lib/homestore_backend/replication_state_machine.cpp @@ -5,7 +5,7 @@ namespace homeobject { void ReplicationStateMachine::on_commit(int64_t lsn, const sisl::blob& header, const sisl::blob& key, const homestore::MultiBlkId& pbas, cintrusive< homestore::repl_req_ctx >& ctx) { - LOGINFO("applying raft log commit with lsn:{}", lsn); + LOGI("applying raft log commit with lsn:{}", lsn); const ReplicationMessageHeader* msg_header = r_cast< const ReplicationMessageHeader* >(header.bytes); switch (msg_header->message_type) { @@ -24,7 +24,7 @@ void ReplicationStateMachine::on_commit(int64_t lsn, const sisl::blob& header, c bool ReplicationStateMachine::on_pre_commit(int64_t lsn, sisl::blob const& header, sisl::blob const& key, cintrusive< homestore::repl_req_ctx >& ctx) { bool ret{false}; - LOGINFO("on_pre_commit with lsn:{}", lsn); + LOGI("on_pre_commit with lsn:{}", lsn); const ReplicationMessageHeader* msg_header = r_cast< const ReplicationMessageHeader* >(header.bytes); switch (msg_header->message_type) { @@ -43,7 +43,7 @@ bool ReplicationStateMachine::on_pre_commit(int64_t lsn, sisl::blob const& heade void ReplicationStateMachine::on_rollback(int64_t lsn, sisl::blob const& header, sisl::blob const& key, cintrusive< homestore::repl_req_ctx >& ctx) { - LOGINFO("rollback with lsn:{}", lsn); + LOGI("rollback with lsn:{}", lsn); const ReplicationMessageHeader* msg_header = r_cast< const ReplicationMessageHeader* >(header.bytes); switch (msg_header->message_type) { diff --git a/src/lib/homestore_backend/tests/test_home_object.cpp b/src/lib/homestore_backend/tests/test_home_object.cpp index 6620b3fd..ca82c72c 100644 --- a/src/lib/homestore_backend/tests/test_home_object.cpp +++ b/src/lib/homestore_backend/tests/test_home_object.cpp @@ -31,7 +31,7 @@ class FixtureApp : public homeobject::HomeObjectApplication { bool spdk_mode() const override { return false; } uint32_t threads() const override { return 2; } std::list< std::filesystem::path > devices() const override { - LOGINFO("creating {} device file with size={}", fpath_, homestore::in_bytes(2 * Gi)); + 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); diff --git a/src/lib/homestore_backend/tests/test_shard_manager.cpp b/src/lib/homestore_backend/tests/test_shard_manager.cpp index c53e138c..928b30d9 100644 --- a/src/lib/homestore_backend/tests/test_shard_manager.cpp +++ b/src/lib/homestore_backend/tests/test_shard_manager.cpp @@ -30,7 +30,7 @@ class FixtureApp : public homeobject::HomeObjectApplication { bool spdk_mode() const override { return false; } uint32_t threads() const override { return 2; } std::list< std::filesystem::path > devices() const override { - LOGINFO("creating {} device file with size={}", fpath_, homestore::in_bytes(2 * Gi)); + 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); diff --git a/src/lib/memory_backend/mem_blob_manager.cpp b/src/lib/memory_backend/mem_blob_manager.cpp index 03abcb30..c88e7500 100644 --- a/src/lib/memory_backend/mem_blob_manager.cpp +++ b/src/lib/memory_backend/mem_blob_manager.cpp @@ -9,11 +9,11 @@ namespace homeobject { #define WITH_ROUTE(blob) \ auto const route = BlobRoute{_shard.id, (blob)}; \ - LOGTRACEMOD(homeobject, "[route={}]", route); + LOGT("[route={}]", route); #define IF_BLOB_ALIVE \ if (auto blob_it = shard.btree_.find(route); shard.btree_.end() == blob_it || !blob_it->second) { \ - LOGWARNMOD(homeobject, "[route={}] missing", route); \ + LOGW("[route={}] missing", route); \ return folly::makeUnexpected(BlobError::UNKNOWN_BLOB); \ } else @@ -40,9 +40,8 @@ BlobManager::NullResult MemoryHomeObject::_del_blob(ShardInfo const& _shard, blo WITH_SHARD WITH_ROUTE(_blob) IF_BLOB_ALIVE { - auto del_blob = BlobExt(); - del_blob.blob_ = blob_it->second.blob_; - shard.btree_.assign_if_equal(route, blob_it->second, std::move(del_blob)); + shard.btree_.assign_if_equal(route, blob_it->second, + BlobExt{.state_ = BlobState::DELETED, .blob_ = blob_it->second.blob_}); return folly::Unit(); } } diff --git a/src/lib/memory_backend/mem_homeobject.cpp b/src/lib/memory_backend/mem_homeobject.cpp index ce72b51d..d21a891d 100644 --- a/src/lib/memory_backend/mem_homeobject.cpp +++ b/src/lib/memory_backend/mem_homeobject.cpp @@ -9,17 +9,10 @@ extern std::shared_ptr< HomeObject > init_homeobject(std::weak_ptr< HomeObjectAp return std::make_shared< MemoryHomeObject >(std::move(application)); } -#if 0 -void HomeObjectImpl::init_repl_svc() { - auto lg = std::scoped_lock(_repl_lock); - if (!_repl_svc) { - _our_id = boost::uuids::random_generator()(); - LOGINFOMOD(homeobject, "SvcId faked: {}", to_string(_our_id)); - _our_id = _application.lock()->discover_svcid(_our_id); - _repl_svc = home_replication::create_repl_service([](auto) { return nullptr; }); - } +MemoryHomeObject::MemoryHomeObject(std::weak_ptr< HomeObjectApplication >&& application) : + HomeObjectImpl::HomeObjectImpl(std::move(application)) { + _our_id = _application.lock()->discover_svcid(_our_id); } -#endif ShardIndex::~ShardIndex() { for (auto it = btree_.begin(); it != btree_.end(); ++it) { diff --git a/src/lib/memory_backend/mem_homeobject.hpp b/src/lib/memory_backend/mem_homeobject.hpp index 364be6ed..c5f1cf65 100644 --- a/src/lib/memory_backend/mem_homeobject.hpp +++ b/src/lib/memory_backend/mem_homeobject.hpp @@ -52,7 +52,7 @@ class MemoryHomeObject : public HomeObjectImpl { ShardIndex& _find_index(shard_id_t) const; public: - using HomeObjectImpl::HomeObjectImpl; + MemoryHomeObject(std::weak_ptr< HomeObjectApplication >&& application); ~MemoryHomeObject() override = default; }; diff --git a/src/lib/pg_manager.cpp b/src/lib/pg_manager.cpp index fc4c3839..6ff00905 100644 --- a/src/lib/pg_manager.cpp +++ b/src/lib/pg_manager.cpp @@ -7,7 +7,7 @@ namespace homeobject { std::shared_ptr< PGManager > HomeObjectImpl::pg_manager() { return shared_from_this(); } PGManager::NullAsyncResult HomeObjectImpl::create_pg(PGInfo&& pg_info) { - LOGINFO("Creating PG: [{}] of [{}] members", pg_info.id, pg_info.members.size()); + LOGI("[pg={}] has [{}] members", pg_info.id, pg_info.members.size()); auto saw_ourself = false; auto saw_leader = false; auto peers = std::set< std::string, std::less<> >(); @@ -23,14 +23,14 @@ PGManager::NullAsyncResult HomeObjectImpl::create_pg(PGInfo&& pg_info) { PGManager::NullAsyncResult HomeObjectImpl::replace_member(pg_id_t id, peer_id_t const& old_member, PGMember const& new_member) { - LOGINFO("Replacing PG: [{}] member [{}] with [{}]", id, to_string(old_member), to_string(new_member.id)); + LOGI("[pg={}] replace member [{}] with [{}]", id, to_string(old_member), to_string(new_member.id)); if (old_member == new_member.id) { - LOGWARN("Rejecting replace_member with identical replacement SvcId [{}]!", to_string(old_member)); + LOGW("rejecting identical replacement SvcId [{}]!", to_string(old_member)); return folly::makeUnexpected(PGError::INVALID_ARG); } if (old_member == our_uuid()) { - LOGWARN("Rejecting replace_member removing ourself {}!", to_string(old_member)); + LOGW("refusing to remove ourself {}!", to_string(old_member)); return folly::makeUnexpected(PGError::INVALID_ARG); } diff --git a/src/lib/shard_manager.cpp b/src/lib/shard_manager.cpp index 2878457f..aacde6ed 100644 --- a/src/lib/shard_manager.cpp +++ b/src/lib/shard_manager.cpp @@ -21,7 +21,7 @@ ShardManager::AsyncResult< InfoList > HomeObjectImpl::list_shards(pg_id_t pgid) auto info_l = std::list< ShardInfo >(); for (auto const& shard : pg->shards_) { - LOGDEBUG("Listing Shard {}", shard.info.id); + LOGD("found [shard={}]", shard.info.id); info_l.push_back(shard.info); } return info_l;