curve-fuse: fix SEGV when creating partition concurrently

Signed-off-by: Hanqing Wu <[email protected]>
uvletter · Jun 30, 2022 · c9d6500 · c9d6500
1 parent 1d164c2
commit c9d6500
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 25 deletions.
diff --git a/curvefs/src/client/rpcclient/mds_client.cpp b/curvefs/src/client/rpcclient/mds_client.cpp
@@ -304,10 +304,11 @@ bool MdsClientImpl::CreatePartition(
             return TopoStatusCode::TOPO_CREATE_PARTITION_FAIL;
         }
 
+        partitionInfos->reserve(count);
         partitionInfos->clear();
-        for (int i = 0; i < partitionNum; i++) {
-            partitionInfos->push_back(response.partitioninfolist(i));
-        }
+        std::move(response.mutable_partitioninfolist()->begin(),
+                  response.mutable_partitioninfolist()->end(),
+                  std::back_inserter(*partitionInfos));
 
         return TopoStatusCode::TOPO_OK;
     };

diff --git a/curvefs/src/client/rpcclient/metacache.cpp b/curvefs/src/client/rpcclient/metacache.cpp
@@ -129,7 +129,7 @@ bool MetaCache::SelectTarget(uint32_t fsID, CopysetTarget *target,
 
         if (!SelectPartition(target)) {
             LOG(ERROR) << "select target for {fsid:" << fsID
-                       << "} fail,  select paritiotn fail";
+                       << "} fail,  select partition fail";
             return false;
         }
     }
@@ -254,7 +254,7 @@ bool MetaCache::ListPartitions(uint32_t fsID) {
     WriteLockGuard wl4CopysetMap(rwlock4copysetInfoMap_);
 
     fsID_ = fsID;
-    PatitionInfoList partitionInfos;
+    PartitionInfoList partitionInfos;
     std::map<PoolIDCopysetID, CopysetInfo<MetaserverID>> copysetMap;
     if (!DoListOrCreatePartitions(true, &partitionInfos, &copysetMap)) {
         return false;
@@ -266,13 +266,17 @@ bool MetaCache::ListPartitions(uint32_t fsID) {
 }
 
 bool MetaCache::CreatePartitions(int currentNum,
-                                 PatitionInfoList *newPartitions) {
+                                 PartitionInfoList *newPartitions) {
     std::lock_guard<Mutex> lg(createMutex_);
 
     // already create
     {
         ReadLockGuard rl(rwlock4Partitions_);
         if (partitionInfos_.size() > currentNum) {
+            newPartitions->reserve(partitionInfos_.size() - currentNum);
+            newPartitions->insert(newPartitions->end(),
+                                  partitionInfos_.begin() + currentNum,
+                                  partitionInfos_.end());
             return true;
         }
     }
@@ -293,7 +297,7 @@ bool MetaCache::CreatePartitions(int currentNum,
 }
 
 bool MetaCache::DoListOrCreatePartitions(
-    bool list, PatitionInfoList *partitionInfos,
+    bool list, PartitionInfoList *partitionInfos,
     std::map<PoolIDCopysetID, CopysetInfo<MetaserverID>> *copysetMap) {
     // TODO(@lixiaocui): list or get partition need too many rpc,
     // it's better to return all infos once.
@@ -374,7 +378,7 @@ bool MetaCache::DoListOrCreatePartitions(
 }
 
 void MetaCache::DoAddOrResetPartitionAndCopyset(
-    PatitionInfoList partitionInfos,
+    PartitionInfoList partitionInfos,
     std::map<PoolIDCopysetID, CopysetInfo<MetaserverID>> copysetMap,
     bool reset) {
     if (reset) {
@@ -498,18 +502,20 @@ bool MetaCache::SelectPartition(CopysetTarget *target) {
     }
 
     if (candidate.empty()) {
-        // create parition for fs
+        // create partition for fs
         LOG(INFO) << "no partition can be select for fsid:" << fsID_
                   << ", need create new partitions";
-        PatitionInfoList newPartitions;
+        PartitionInfoList newPartitions;
         if (!CreatePartitions(currentNum, &newPartitions)) {
             LOG(ERROR) << "create partition for fsid:" << fsID_ << " fail";
             return false;
         }
-        target->groupID = CopysetGroupID(newPartitions[0].poolid(),
-                                         newPartitions[0].copysetid());
-        target->partitionID = newPartitions[0].partitionid();
-        target->txId = newPartitions[0].txid();
+        CHECK(!newPartitions.empty());
+        const auto index = butil::fast_rand() % newPartitions.size();
+        auto iter = newPartitions.begin() + index;
+        target->groupID = CopysetGroupID(iter->poolid(), iter->copysetid());
+        target->partitionID = iter->partitionid();
+        target->txId = iter->txid();
     } else {
         // random select a partition
         const auto index = butil::fast_rand() % candidate.size();

diff --git a/curvefs/src/client/rpcclient/metacache.h b/curvefs/src/client/rpcclient/metacache.h
@@ -111,15 +111,14 @@ class MetaCache {
  public:
     void Init(MetaCacheOpt opt, std::shared_ptr<Cli2Client> cli2Client,
               std::shared_ptr<MdsClient> mdsClient) {
-        metacacheopt_ = opt;
-        cli2Client_ = cli2Client;
-        mdsClient_ = mdsClient;
+        metacacheopt_ = std::move(opt);
+        cli2Client_ = std::move(cli2Client);
+        mdsClient_ = std::move(mdsClient);
         init_ = false;
     }
 
     using PoolIDCopysetID = uint64_t;
-    using PatitionInfoList = std::vector<PartitionInfo>;
-    using FS2PatitionInfoMap = std::unordered_map<uint32_t, PatitionInfoList>;
+    using PartitionInfoList = std::vector<PartitionInfo>;
     using CopysetInfoMap =
         std::unordered_map<PoolIDCopysetID, CopysetInfo<MetaserverID>>;
 
@@ -162,12 +161,12 @@ class MetaCache {
 
  private:
     void GetTxId(uint32_t partitionId, uint64_t *txId);
-    bool CreatePartitions(int currentNum, PatitionInfoList *newPartitions);
+    bool CreatePartitions(int currentNum, PartitionInfoList *newPartitions);
     bool DoListOrCreatePartitions(
-        bool list, PatitionInfoList *partitionInfos,
+        bool list, PartitionInfoList *partitionInfos,
         std::map<PoolIDCopysetID, CopysetInfo<MetaserverID>> *copysetMap);
     void DoAddOrResetPartitionAndCopyset(
-        PatitionInfoList partitionInfos,
+        PartitionInfoList partitionInfos,
         std::map<PoolIDCopysetID, CopysetInfo<MetaserverID>> copysetMap,
         bool reset);
 
@@ -206,7 +205,7 @@ class MetaCache {
     std::unordered_map<uint32_t, uint64_t> partitionTxId_;
 
     RWLock rwlock4Partitions_;
-    PatitionInfoList partitionInfos_;
+    PartitionInfoList partitionInfos_;
     RWLock rwlock4copysetInfoMap_;
     CopysetInfoMap copysetInfoMap_;
 

diff --git a/curvefs/test/client/rpcclient/metacache_test.cpp b/curvefs/test/client/rpcclient/metacache_test.cpp
@@ -133,8 +133,8 @@ class MetaCacheTest : public testing::Test {
     std::shared_ptr<MockCli2Client> mockCli2Client_;
 
     curve::client::CopysetInfo<MetaserverID> metaServerList_;
-    MetaCache::PatitionInfoList pInfoList_;
-    MetaCache::PatitionInfoList pInfoList2_;
+    MetaCache::PartitionInfoList pInfoList_;
+    MetaCache::PartitionInfoList pInfoList2_;
     std::map<PartitionID, Copyset> copysetMap_;
 
     CopysetTarget expect;