From 85c893b783612e371f1b5902d2a22ede2ccdd039 Mon Sep 17 00:00:00 2001
From: chuandew <chuandewang@outlook.com>
Date: Mon, 13 Jan 2025 18:11:10 +0800
Subject: [PATCH] [refactor][metaserver] Refact metastore rw lock scope and
 some log

---
 src/metaserver/copyset/copyset_node.cpp |  9 ++++---
 src/metaserver/metastore.cpp            | 33 ++++++++++++-------------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/metaserver/copyset/copyset_node.cpp b/src/metaserver/copyset/copyset_node.cpp
index 34327c9d..90cff659 100644
--- a/src/metaserver/copyset/copyset_node.cpp
+++ b/src/metaserver/copyset/copyset_node.cpp
@@ -577,7 +577,7 @@ void CopysetNode::ListPeers(std::vector<Peer>* peers) const {
 // partition info list success.
 bool CopysetNode::GetPartitionInfoList(
     std::list<pb::common::PartitionInfo>* partitionInfoList) {
-  uint32_t retryCount = 0;
+  uint32_t retry_count = 0;
   while (true) {
     if (IsLoading()) {
       LOG(INFO) << "Copyset is loading, return empty partition list";
@@ -587,8 +587,11 @@ bool CopysetNode::GetPartitionInfoList(
     if (ret) {
       return true;
     }
-    LOG(WARNING) << "Copyset is not loading, but GetPartitionInfoList fail,"
-                 << " retryCount = " << retryCount++;
+    retry_count++;
+    if (retry_count % 100 == 0) {
+      LOG(WARNING) << "Copyset is not loading, but GetPartitionInfoList fail,"
+                   << " retryCount = " << retry_count;
+    }
   }
 }
 
diff --git a/src/metaserver/metastore.cpp b/src/metaserver/metastore.cpp
index d2bdac6e..1f8a27f7 100644
--- a/src/metaserver/metastore.cpp
+++ b/src/metaserver/metastore.cpp
@@ -30,8 +30,8 @@
 #include <vector>
 
 #include "absl/types/optional.h"
-#include "dingofs/metaserver.pb.h"
 #include "common/define.h"
+#include "dingofs/metaserver.pb.h"
 #include "metaserver/copyset/copyset_node.h"
 #include "metaserver/partition_clean_manager.h"
 #include "metaserver/recycle_cleaner.h"
@@ -219,25 +219,27 @@ void MetaStoreImpl::SaveBackground(const std::string& path,
 
 bool MetaStoreImpl::Save(const std::string& dir,
                          OnSnapshotSaveDoneClosure* done) {
-  brpc::ClosureGuard doneGuard(done);
-  WriteLockGuard writeLockGuard(rwLock_);
+  brpc::ClosureGuard done_guard(done);
+  {
+    WriteLockGuard write_lock_guard(rwLock_);
 
-  MetaStoreFStream fstream(&partitionMap_, kvStorage_,
-                           copysetNode_->GetPoolId(),
-                           copysetNode_->GetCopysetId());
+    MetaStoreFStream fstream(&partitionMap_, kvStorage_,
+                             copysetNode_->GetPoolId(),
+                             copysetNode_->GetCopysetId());
 
-  const std::string metadata = dir + "/" + kMetaDataFilename;
-  bool succ = fstream.Save(metadata);
-  if (!succ) {
-    done->SetError(MetaStatusCode::SAVE_META_FAIL);
-    return false;
+    const std::string metadata = dir + "/" + kMetaDataFilename;
+    bool succ = fstream.Save(metadata);
+    if (!succ) {
+      done->SetError(MetaStatusCode::SAVE_META_FAIL);
+      return false;
+    }
   }
 
   // checkpoint storage
   butil::Timer timer;
   timer.start();
   std::vector<std::string> files;
-  succ = kvStorage_->Checkpoint(dir, &files);
+  bool succ = kvStorage_->Checkpoint(dir, &files);
   if (!succ) {
     done->SetError(MetaStatusCode::SAVE_META_FAIL);
     return false;
@@ -443,15 +445,12 @@ bool MetaStoreImpl::GetPartitionInfoList(
   int ret = rwLock_.TryRDLock();
   if (ret == 0) {
     for (const auto& it : partitionMap_) {
-      PartitionInfo partitionInfo = it.second->GetPartitionInfo();
-      partitionInfoList->push_back(std::move(partitionInfo));
+      PartitionInfo partition_info = it.second->GetPartitionInfo();
+      partitionInfoList->push_back(std::move(partition_info));
     }
     rwLock_.Unlock();
     return true;
   } else {
-    LOG(WARNING) << "metastore GetPartitionInfoList fail, it fail to get"
-                    " the rwLock_, ret:"
-                 << ret;
     return false;
   }
 }