Make logging module consistent and remove atomic.

.clang-format

@@ -16,7 +16,6 @@ AlignConsecutiveDeclarations: false
 AlignEscapedNewlines: Right
 AlignOperands:   false
 AlignTrailingComments: true
-AllowShortBlocksOnASingleLine: true
 AllowShortIfStatementsOnASingleLine: true
 AllowShortBlocksOnASingleLine: true
 AllowShortCaseLabelsOnASingleLine: false

src/lib/homeobject_impl.hpp

@@ -7,6 +7,13 @@
 
 #include <sisl/logging/logging.h>
 
+#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;
 }
@@ -91,7 +98,7 @@ class HomeObjectImpl : public HomeObject,
 
     ///
     mutable std::shared_mutex _pg_lock;
-    std::map< pg_id_t, shared< PG > > _pg_map;
+    std::map< pg_id_t, unique< PG > > _pg_map;
 
     mutable std::shared_mutex _shard_lock;
     std::map< shard_id_t, ShardIterator > _shard_map;

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);
     }

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}},

src/lib/homestore_backend/hs_homeobject.hpp

@@ -71,7 +71,7 @@ class HSHomeObject : public HomeObjectImpl {
     void register_homestore_metablk_callback();
     void* get_shard_metablk(shard_id_t id);
     void on_pg_meta_blk_found(sisl::byte_view const& buf, void* meta_cookie);
-    void add_pg_to_map(shared< HS_PG > hs_pg);
+    void add_pg_to_map(unique< HS_PG > hs_pg);
 
 public:
     using HomeObjectImpl::HomeObjectImpl;

src/lib/homestore_backend/hs_pg_manager.cpp

@@ -52,7 +52,7 @@ PGManager::NullAsyncResult HSHomeObject::_create_pg(PGInfo&& pg_info, std::set<
         .create_repl_dev(pg_info.replica_set_uuid, std::move(peers), std::make_unique< ReplicationStateMachine >(this))
         .thenValue([this, pg_info = std::move(pg_info)](auto&& v) -> PGManager::NullResult {
             if (v.hasError()) { return folly::makeUnexpected(toPgError(v.error())); }
-            add_pg_to_map(std::make_shared< HS_PG >(std::move(pg_info), std::move(v.value())));
+            add_pg_to_map(std::make_unique< HS_PG >(std::move(pg_info), std::move(v.value())));
             return folly::Unit();
         });
 }
@@ -62,7 +62,7 @@ PGManager::NullAsyncResult HSHomeObject::_replace_member(pg_id_t id, peer_id_t c
     return folly::makeSemiFuture< PGManager::NullResult >(folly::makeUnexpected(PGError::UNSUPPORTED_OP));
 }
 
-void HSHomeObject::add_pg_to_map(shared< HS_PG > hs_pg) {
+void HSHomeObject::add_pg_to_map(unique< HS_PG > hs_pg) {
     RELEASE_ASSERT(hs_pg->pg_info_.replica_set_uuid == hs_pg->repl_dev_->group_id(),
                    "PGInfo replica set uuid mismatch with ReplDev instance for {}",
                    boost::uuids::to_string(hs_pg->pg_info_.replica_set_uuid));
@@ -117,10 +117,10 @@ 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())));
+            add_pg_to_map(std::make_unique< 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
+} // namespace homeobject

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_;
+        repl_dev = static_cast< HS_PG* >(iter->second.get())->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;
         }

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) {

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);

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);

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();
     }
 }

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) {

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;
 };
 

src/lib/memory_backend/mem_pg_manager.cpp

@@ -3,7 +3,7 @@
 namespace homeobject {
 PGManager::NullAsyncResult MemoryHomeObject::_create_pg(PGInfo&& pg_info, std::set< std::string, std::less<> >) {
     auto lg = std::scoped_lock(_pg_lock);
-    auto [it1, _] = _pg_map.try_emplace(pg_info.id, std::make_shared< PG >(pg_info));
+    auto [it1, _] = _pg_map.try_emplace(pg_info.id, std::make_unique< PG >(pg_info));
     RELEASE_ASSERT(_pg_map.end() != it1, "Unknown map insert error!");
     return folly::makeSemiFuture< PGManager::NullResult >(folly::Unit());
 }
@@ -12,4 +12,4 @@ PGManager::NullAsyncResult MemoryHomeObject::_replace_member(pg_id_t id, peer_id
                                                              PGMember const& new_member) {
     return folly::makeSemiFuture< PGManager::NullResult >(folly::makeUnexpected(PGError::UNSUPPORTED_OP));
 }
-} // namespace homeobject
+} // namespace homeobject

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);
     }
 

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;