From 848dd25d613aff2c6e3a6ba4fc22fbedea0e2d37 Mon Sep 17 00:00:00 2001 From: wanghai01 Date: Thu, 10 Mar 2022 20:21:16 +0800 Subject: [PATCH] curvefs: batch get inode attr and xattr fater find in cache --- curvefs/src/client/fuse_client.cpp | 6 +-- curvefs/src/client/fuse_s3_client.cpp | 6 ++- curvefs/src/client/inode_cache_manager.cpp | 36 ++++++++++++++++-- curvefs/src/client/inode_cache_manager.h | 8 ++-- curvefs/src/client/inode_wrapper.h | 37 +++++++++++++++++++ .../client/rpcclient/metaserver_client.cpp | 8 ++-- .../src/client/rpcclient/metaserver_client.h | 8 ++-- .../test/client/mock_inode_cache_manager.h | 4 +- curvefs/test/client/mock_metaserver_client.h | 4 +- .../rpcclient/metaserver_client_test.cpp | 16 ++++---- .../test/client/test_inode_cache_manager.cpp | 12 +++--- 11 files changed, 107 insertions(+), 38 deletions(-) diff --git a/curvefs/src/client/fuse_client.cpp b/curvefs/src/client/fuse_client.cpp index 339cd64b0c..b72e876491 100644 --- a/curvefs/src/client/fuse_client.cpp +++ b/curvefs/src/client/fuse_client.cpp @@ -855,7 +855,7 @@ CURVEFS_ERROR FuseClient::CalOneLayerSumInfo(Inode *inode) { for (const auto &it : dentryList) { inodeIds.emplace(it.inodeid()); } - ret = inodeManager_->BatchGetInodeAttr(inodeIds, &attrs); + ret = inodeManager_->BatchGetInodeAttr(&inodeIds, &attrs); if (ret == CURVEFS_ERROR::OK) { uint64_t files = 0; uint64_t subdirs = 0; @@ -923,7 +923,7 @@ CURVEFS_ERROR FuseClient::CalAllLayerSumInfo(Inode *inode) { } // check size if (inodeIds.size() >= attrsLimit || iStack.empty()) { - ret = inodeManager_->BatchGetInodeAttr(inodeIds, &attrs); + ret = inodeManager_->BatchGetInodeAttr(&inodeIds, &attrs); if (ret == CURVEFS_ERROR::OK) { for (const auto &it : attrs) { if (it.type() == FsFileType::TYPE_DIRECTORY) { @@ -1008,7 +1008,7 @@ CURVEFS_ERROR FuseClient::FastCalAllLayerSumInfo(Inode *inode) { } // check size if (inodeIds.size() >= xattrsLimit || iStack.empty()) { - ret = inodeManager_->BatchGetXAttr(inodeIds, &xattrs); + ret = inodeManager_->BatchGetXAttr(&inodeIds, &xattrs); if (ret == CURVEFS_ERROR::OK) { for (const auto &it : xattrs) { if (it.xattrinfos().count(XATTRFILES)) { diff --git a/curvefs/src/client/fuse_s3_client.cpp b/curvefs/src/client/fuse_s3_client.cpp index 68a39ee21d..5355d0d101 100644 --- a/curvefs/src/client/fuse_s3_client.cpp +++ b/curvefs/src/client/fuse_s3_client.cpp @@ -108,8 +108,10 @@ CURVEFS_ERROR FuseS3Client::FuseOpWrite(fuse_req_t req, fuse_ino_t ino, Inode *inode = inodeWrapper->GetMutableInodeUnlocked(); *wSize = wRet; + size_t changeSize = 0; // update file len if (inode->length() < off + *wSize) { + changeSize = off + *wSize - inode->length(); inode->set_length(off + *wSize); } struct timespec now; @@ -125,10 +127,10 @@ CURVEFS_ERROR FuseS3Client::FuseOpWrite(fuse_req_t req, fuse_ino_t ino, // Todo: do some cache flush later } - if (enableSumInDir_) { + if (enableSumInDir_ && changeSize != 0) { XAttr xattr; xattr.mutable_xattrinfos()->insert({XATTRFBYTES, - std::to_string(*wSize)}); + std::to_string(changeSize)}); uint64_t parentId; if (inodeManager_->GetParent(ino, &parentId)) { ret = UpdateParentInodeXattr(parentId, xattr, true); diff --git a/curvefs/src/client/inode_cache_manager.cpp b/curvefs/src/client/inode_cache_manager.cpp index b1a4f5ba5a..efe9dacd9f 100644 --- a/curvefs/src/client/inode_cache_manager.cpp +++ b/curvefs/src/client/inode_cache_manager.cpp @@ -82,8 +82,23 @@ CURVEFS_ERROR InodeCacheManagerImpl::GetInode(uint64_t inodeid, } CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttr( - const std::set &inodeIds, + std::set *inodeIds, std::list *attr) { + // get some inode in icache + for (auto iter = inodeIds->begin(); iter != inodeIds->end();) { + std::shared_ptr inodeWrapper; + NameLockGuard lock(nameLock_, std::to_string(*iter)); + bool ok = iCache_->Get(*iter, &inodeWrapper); + if (ok) { + InodeAttr tmpAttr; + inodeWrapper->GetInodeAttrLocked(&tmpAttr); + attr->emplace_back(tmpAttr); + iter = inodeIds->erase(iter); + } else { + ++iter; + } + } + MetaStatusCode ret = metaClient_->BatchGetInodeAttr(fsId_, inodeIds, attr); if (MetaStatusCode::OK != ret) { LOG(ERROR) << "metaClient BatchGetInodeAttr failed, MetaStatusCode = " @@ -94,8 +109,23 @@ CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttr( } CURVEFS_ERROR InodeCacheManagerImpl::BatchGetXAttr( - const std::set &inodeIds, + std::set *inodeIds, std::list *xattr) { + // get some inode in icache + for (auto iter = inodeIds->begin(); iter != inodeIds->end();) { + std::shared_ptr inodeWrapper; + NameLockGuard lock(nameLock_, std::to_string(*iter)); + bool ok = iCache_->Get(*iter, &inodeWrapper); + if (ok) { + XAttr tmpXattr; + inodeWrapper->GetXattrLocked(&tmpXattr); + xattr->emplace_back(tmpXattr); + iter = inodeIds->erase(iter); + } else { + ++iter; + } + } + MetaStatusCode ret = metaClient_->BatchGetXAttr(fsId_, inodeIds, xattr); if (MetaStatusCode::OK != ret) { LOG(ERROR) << "metaClient BatchGetXAttr failed, MetaStatusCode = " @@ -182,7 +212,7 @@ void InodeCacheManagerImpl::FlushInodeOnce() { void InodeCacheManagerImpl::AddParent(uint64_t inodeId, uint64_t parentId) { curve::common::LockGuard lg2(parentIdMapMutex_); - if (parentIdMap_.count(inodeId)) { + if (parentIdMap_.count(inodeId) && parentIdMap_[inodeId] == parentId) { LOG(WARNING) << "AddParent but exist, inodeId = " << inodeId << ", parentId = " << parentIdMap_[inodeId] << ", need2AddParentId = " << parentId; diff --git a/curvefs/src/client/inode_cache_manager.h b/curvefs/src/client/inode_cache_manager.h index 814cfc97b1..75e0a78a00 100644 --- a/curvefs/src/client/inode_cache_manager.h +++ b/curvefs/src/client/inode_cache_manager.h @@ -67,10 +67,10 @@ class InodeCacheManager { std::shared_ptr &out) = 0; // NOLINT virtual CURVEFS_ERROR BatchGetInodeAttr( - const std::set &inodeIds, + std::set *inodeIds, std::list *attrs) = 0; - virtual CURVEFS_ERROR BatchGetXAttr(const std::set &inodeIds, + virtual CURVEFS_ERROR BatchGetXAttr(std::set *inodeIds, std::list *xattrs) = 0; virtual CURVEFS_ERROR CreateInode(const InodeParam ¶m, @@ -125,10 +125,10 @@ class InodeCacheManagerImpl : public InodeCacheManager { CURVEFS_ERROR GetInode(uint64_t inodeid, std::shared_ptr &out) override; // NOLINT - CURVEFS_ERROR BatchGetInodeAttr(const std::set &inodeIds, + CURVEFS_ERROR BatchGetInodeAttr(std::set *inodeIds, std::list *attrs) override; - CURVEFS_ERROR BatchGetXAttr(const std::set &inodeIds, + CURVEFS_ERROR BatchGetXAttr(std::set *inodeIds, std::list *xattrs) override; CURVEFS_ERROR CreateInode(const InodeParam ¶m, diff --git a/curvefs/src/client/inode_wrapper.h b/curvefs/src/client/inode_wrapper.h index af2c58d80d..fe21b541c6 100644 --- a/curvefs/src/client/inode_wrapper.h +++ b/curvefs/src/client/inode_wrapper.h @@ -176,6 +176,43 @@ class InodeWrapper : public std::enable_shared_from_this { return; } + void GetInodeAttrLocked(InodeAttr *attr) { + curve::common::UniqueLock lg(mtx_); + attr->set_inodeid(inode_.inodeid()); + attr->set_fsid(inode_.fsid()); + attr->set_length(inode_.length()); + attr->set_ctime(inode_.ctime()); + attr->set_ctime_ns(inode_.ctime_ns()); + attr->set_mtime(inode_.mtime()); + attr->set_mtime_ns(inode_.mtime_ns()); + attr->set_atime(inode_.atime()); + attr->set_atime_ns(inode_.atime_ns()); + attr->set_uid(inode_.uid()); + attr->set_gid(inode_.gid()); + attr->set_mode(inode_.mode()); + attr->set_nlink(inode_.nlink()); + attr->set_type(inode_.type()); + if (inode_.has_symlink()) { + attr->set_symlink(inode_.symlink()); + } + if (inode_.has_rdev()) { + attr->set_rdev(inode_.rdev()); + } + if (inode_.has_dtime()) { + attr->set_dtime(inode_.dtime()); + } + if (inode_.has_openmpcount()) { + attr->set_openmpcount(inode_.openmpcount()); + } + } + + void GetXattrLocked(XAttr *xattr) { + curve::common::UniqueLock lg(mtx_); + xattr->set_fsid(inode_.fsid()); + xattr->set_inodeid(inode_.inodeid()); + *(xattr->mutable_xattrinfos()) = inode_.xattr(); + } + bool GetXattrUnLocked(const char *name, std::string *value) { auto it = inode_.xattr().find(name); if (it != inode_.xattr().end()) { diff --git a/curvefs/src/client/rpcclient/metaserver_client.cpp b/curvefs/src/client/rpcclient/metaserver_client.cpp index ea3bfec284..c66ea81a5e 100644 --- a/curvefs/src/client/rpcclient/metaserver_client.cpp +++ b/curvefs/src/client/rpcclient/metaserver_client.cpp @@ -448,9 +448,9 @@ MetaStatusCode MetaServerClientImpl::GetInode(uint32_t fsId, uint64_t inodeid, bool GroupInodeIdByPartition( uint32_t fsId, std::shared_ptr metaCache, - const std::set &inodeIds, + std::set *inodeIds, std::unordered_map> *inodeGroups) { - for (const auto &it : inodeIds) { + for (const auto &it : *inodeIds) { uint32_t pId = 0; if (metaCache->GetPartitionIdByInodeId(fsId, it, &pId)) { auto iter = inodeGroups->find(pId); @@ -469,7 +469,7 @@ bool GroupInodeIdByPartition( } MetaStatusCode MetaServerClientImpl::BatchGetInodeAttr(uint32_t fsId, - const std::set &inodeIds, + std::set *inodeIds, std::list *attr) { uint32_t limit = opt_.batchLimit; // group inodeid by partition @@ -548,7 +548,7 @@ MetaStatusCode MetaServerClientImpl::BatchGetInodeAttr(uint32_t fsId, } MetaStatusCode MetaServerClientImpl::BatchGetXAttr(uint32_t fsId, - const std::set &inodeIds, + std::set *inodeIds, std::list *xattr) { uint32_t limit = opt_.batchLimit; // group inodeid by partition diff --git a/curvefs/src/client/rpcclient/metaserver_client.h b/curvefs/src/client/rpcclient/metaserver_client.h index f532f68154..8c550caf54 100644 --- a/curvefs/src/client/rpcclient/metaserver_client.h +++ b/curvefs/src/client/rpcclient/metaserver_client.h @@ -87,11 +87,11 @@ class MetaServerClient { Inode *out) = 0; virtual MetaStatusCode BatchGetInodeAttr(uint32_t fsId, - const std::set &inodeIds, + std::set *inodeIds, std::list *attr) = 0; virtual MetaStatusCode BatchGetXAttr(uint32_t fsId, - const std::set &inodeIds, + std::set *inodeIds, std::list *xattr) = 0; virtual MetaStatusCode UpdateInode(const Inode &inode, @@ -159,11 +159,11 @@ class MetaServerClientImpl : public MetaServerClient { Inode *out) override; MetaStatusCode BatchGetInodeAttr(uint32_t fsId, - const std::set &inodeIds, + std::set *inodeIds, std::list *attr) override; MetaStatusCode BatchGetXAttr(uint32_t fsId, - const std::set &inodeIds, + std::set *inodeIds, std::list *xattr) override; MetaStatusCode UpdateInode(const Inode &inode, diff --git a/curvefs/test/client/mock_inode_cache_manager.h b/curvefs/test/client/mock_inode_cache_manager.h index 00981cfcea..97f4560b33 100644 --- a/curvefs/test/client/mock_inode_cache_manager.h +++ b/curvefs/test/client/mock_inode_cache_manager.h @@ -45,10 +45,10 @@ class MockInodeCacheManager : public InodeCacheManager { uint64_t inodeid, std::shared_ptr &out)); // NOLINT MOCK_METHOD2(BatchGetInodeAttr, CURVEFS_ERROR( - const std::set &inodeIds, std::list *attrs)); + std::set *inodeIds, std::list *attrs)); MOCK_METHOD2(BatchGetXAttr, CURVEFS_ERROR( - const std::set &inodeIds, std::list *xattrs)); + std::set *inodeIds, std::list *xattrs)); MOCK_METHOD2(CreateInode, CURVEFS_ERROR(const InodeParam ¶m, std::shared_ptr &out)); // NOLINT diff --git a/curvefs/test/client/mock_metaserver_client.h b/curvefs/test/client/mock_metaserver_client.h index 2e7a008748..1cb7c48b51 100644 --- a/curvefs/test/client/mock_metaserver_client.h +++ b/curvefs/test/client/mock_metaserver_client.h @@ -76,11 +76,11 @@ class MockMetaServerClient : public MetaServerClient { uint32_t fsId, uint64_t inodeid, Inode *out)); MOCK_METHOD3(BatchGetInodeAttr, MetaStatusCode( - uint32_t fsId, const std::set &inodeIds, + uint32_t fsId, std::set *inodeIds, std::list *attr)); MOCK_METHOD3(BatchGetXAttr, MetaStatusCode( - uint32_t fsId, const std::set &inodeIds, + uint32_t fsId, std::set *inodeIds, std::list *xattr)); MOCK_METHOD2(UpdateInode, diff --git a/curvefs/test/client/rpcclient/metaserver_client_test.cpp b/curvefs/test/client/rpcclient/metaserver_client_test.cpp index 73a1af45b3..706093e6fa 100644 --- a/curvefs/test/client/rpcclient/metaserver_client_test.cpp +++ b/curvefs/test/client/rpcclient/metaserver_client_test.cpp @@ -1059,7 +1059,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetInodeAttr) { SetArgPointee<3>(applyIndex), Return(true))); MetaStatusCode status = metaserverCli_.BatchGetInodeAttr( - fsid, inodeIds, &attr); + fsid, &inodeIds, &attr); ASSERT_EQ(MetaStatusCode::RPC_ERROR, status); // test1: batchGetInodeAttr ok @@ -1081,7 +1081,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetInodeAttr) { BatchGetInodeAttrResponse>))); EXPECT_CALL(*mockMetacache_.get(), UpdateApplyIndex(_, _)); - status = metaserverCli_.BatchGetInodeAttr(fsid, inodeIds, &attr); + status = metaserverCli_.BatchGetInodeAttr(fsid, &inodeIds, &attr); ASSERT_EQ(MetaStatusCode::OK, status); ASSERT_EQ(attr.size(), 2); ASSERT_THAT(attr.begin()->inodeid(), AnyOf(inodeId1, inodeId2)); @@ -1097,7 +1097,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetInodeAttr) { DoAll(SetArgPointee<2>(response), Invoke(SetRpcService))); - status = metaserverCli_.BatchGetInodeAttr(fsid, inodeIds, &attr); + status = metaserverCli_.BatchGetInodeAttr(fsid, &inodeIds, &attr); ASSERT_EQ(MetaStatusCode::NOT_FOUND, status); // test3: test response do not have applyindex @@ -1113,7 +1113,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetInodeAttr) { Invoke(SetRpcService))); - status = metaserverCli_.BatchGetInodeAttr(fsid, inodeIds, &attr); + status = metaserverCli_.BatchGetInodeAttr(fsid, &inodeIds, &attr); ASSERT_EQ(MetaStatusCode::RPC_ERROR, status); } @@ -1161,7 +1161,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetXAttr) { SetArgPointee<3>(applyIndex), Return(true))); MetaStatusCode status = metaserverCli_.BatchGetXAttr( - fsid, inodeIds, &xattr); + fsid, &inodeIds, &xattr); ASSERT_EQ(MetaStatusCode::RPC_ERROR, status); // test1: batchGetXAttr ok @@ -1183,7 +1183,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetXAttr) { BatchGetXAttrResponse>))); EXPECT_CALL(*mockMetacache_.get(), UpdateApplyIndex(_, _)); - status = metaserverCli_.BatchGetXAttr(fsid, inodeIds, &xattr); + status = metaserverCli_.BatchGetXAttr(fsid, &inodeIds, &xattr); ASSERT_EQ(MetaStatusCode::OK, status); ASSERT_EQ(xattr.size(), 2); ASSERT_THAT(xattr.begin()->inodeid(), AnyOf(inodeId1, inodeId2)); @@ -1199,7 +1199,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetXAttr) { DoAll(SetArgPointee<2>(response), Invoke(SetRpcService))); - status = metaserverCli_.BatchGetXAttr(fsid, inodeIds, &xattr); + status = metaserverCli_.BatchGetXAttr(fsid, &inodeIds, &xattr); ASSERT_EQ(MetaStatusCode::NOT_FOUND, status); // test3: test response do not have applyindex @@ -1215,7 +1215,7 @@ TEST_F(MetaServerClientImplTest, test_BatchGetXAttr) { Invoke(SetRpcService))); - status = metaserverCli_.BatchGetXAttr(fsid, inodeIds, &xattr); + status = metaserverCli_.BatchGetXAttr(fsid, &inodeIds, &xattr); ASSERT_EQ(MetaStatusCode::RPC_ERROR, status); } diff --git a/curvefs/test/client/test_inode_cache_manager.cpp b/curvefs/test/client/test_inode_cache_manager.cpp index cd4b335e61..777431151a 100644 --- a/curvefs/test/client/test_inode_cache_manager.cpp +++ b/curvefs/test/client/test_inode_cache_manager.cpp @@ -234,16 +234,16 @@ TEST_F(TestInodeCacheManager, BatchGetInodeAttr) { attr.set_inodeid(inodeId2); attrs.emplace_back(attr); - EXPECT_CALL(*metaClient_, BatchGetInodeAttr(fsId_, inodeIds, _)) + EXPECT_CALL(*metaClient_, BatchGetInodeAttr(fsId_, &inodeIds, _)) .WillOnce(Return(MetaStatusCode::NOT_FOUND)) .WillOnce(DoAll(SetArgPointee<2>(attrs), Return(MetaStatusCode::OK))); std::list getAttrs; - CURVEFS_ERROR ret = iCacheManager_->BatchGetInodeAttr(inodeIds, &getAttrs); + CURVEFS_ERROR ret = iCacheManager_->BatchGetInodeAttr(&inodeIds, &getAttrs); ASSERT_EQ(CURVEFS_ERROR::NOTEXIST, ret); - ret = iCacheManager_->BatchGetInodeAttr(inodeIds, &getAttrs); + ret = iCacheManager_->BatchGetInodeAttr(&inodeIds, &getAttrs); ASSERT_EQ(CURVEFS_ERROR::OK, ret); ASSERT_EQ(getAttrs.size(), 2); ASSERT_THAT(getAttrs.begin()->inodeid(), AnyOf(inodeId1, inodeId2)); @@ -274,16 +274,16 @@ TEST_F(TestInodeCacheManager, BatchGetXAttr) { xattr.mutable_xattrinfos()->find(XATTRFBYTES)->second = "200"; xattrs.emplace_back(xattr); - EXPECT_CALL(*metaClient_, BatchGetXAttr(fsId_, inodeIds, _)) + EXPECT_CALL(*metaClient_, BatchGetXAttr(fsId_, &inodeIds, _)) .WillOnce(Return(MetaStatusCode::NOT_FOUND)) .WillOnce(DoAll(SetArgPointee<2>(xattrs), Return(MetaStatusCode::OK))); std::list getXAttrs; - CURVEFS_ERROR ret = iCacheManager_->BatchGetXAttr(inodeIds, &getXAttrs); + CURVEFS_ERROR ret = iCacheManager_->BatchGetXAttr(&inodeIds, &getXAttrs); ASSERT_EQ(CURVEFS_ERROR::NOTEXIST, ret); - ret = iCacheManager_->BatchGetXAttr(inodeIds, &getXAttrs); + ret = iCacheManager_->BatchGetXAttr(&inodeIds, &getXAttrs); ASSERT_EQ(CURVEFS_ERROR::OK, ret); ASSERT_EQ(getXAttrs.size(), 2); ASSERT_THAT(getXAttrs.begin()->inodeid(), AnyOf(inodeId1, inodeId2));