From c9d65005351671258fc7d3595f1802b1ec211051 Mon Sep 17 00:00:00 2001 From: Hanqing Wu Date: Wed, 29 Jun 2022 19:35:04 +0800 Subject: [PATCH] curve-fuse: fix SEGV when creating partition concurrently Signed-off-by: Hanqing Wu --- curvefs/src/client/rpcclient/mds_client.cpp | 7 +++-- curvefs/src/client/rpcclient/metacache.cpp | 28 +++++++++++-------- curvefs/src/client/rpcclient/metacache.h | 17 ++++++----- .../test/client/rpcclient/metacache_test.cpp | 4 +-- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/curvefs/src/client/rpcclient/mds_client.cpp b/curvefs/src/client/rpcclient/mds_client.cpp index 9688ce2830..6451c77765 100644 --- 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 index 04d68b5374..0802edd035 100644 --- 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> copysetMap; if (!DoListOrCreatePartitions(true, &partitionInfos, ©setMap)) { return false; @@ -266,13 +266,17 @@ bool MetaCache::ListPartitions(uint32_t fsID) { } bool MetaCache::CreatePartitions(int currentNum, - PatitionInfoList *newPartitions) { + PartitionInfoList *newPartitions) { std::lock_guard 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> *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> 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 index 8447483d3e..fecec3440e 100644 --- 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, std::shared_ptr 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; - using FS2PatitionInfoMap = std::unordered_map; + using PartitionInfoList = std::vector; using CopysetInfoMap = std::unordered_map>; @@ -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> *copysetMap); void DoAddOrResetPartitionAndCopyset( - PatitionInfoList partitionInfos, + PartitionInfoList partitionInfos, std::map> copysetMap, bool reset); @@ -206,7 +205,7 @@ class MetaCache { std::unordered_map 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 index 2dadce23fe..66d89de3ac 100644 --- 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_; curve::client::CopysetInfo metaServerList_; - MetaCache::PatitionInfoList pInfoList_; - MetaCache::PatitionInfoList pInfoList2_; + MetaCache::PartitionInfoList pInfoList_; + MetaCache::PartitionInfoList pInfoList2_; std::map copysetMap_; CopysetTarget expect;