From bce854c15852577428e8d3bc762182ce30f7636c Mon Sep 17 00:00:00 2001 From: wanghai01 Date: Tue, 27 Sep 2022 19:57:28 +0800 Subject: [PATCH] curvefs: fix batch get inodeattr when readdir Signed-off-by: wanghai01 --- curvefs/src/client/fuse_client.cpp | 2 +- curvefs/src/client/inode_cache_manager.cpp | 44 +++++++++++---- curvefs/src/client/inode_cache_manager.h | 14 +++-- curvefs/src/client/inode_wrapper.h | 4 ++ .../test/client/mock_inode_cache_manager.h | 2 +- .../test/client/test_inode_cache_manager.cpp | 54 +++++++++++++++++++ 6 files changed, 104 insertions(+), 16 deletions(-) diff --git a/curvefs/src/client/fuse_client.cpp b/curvefs/src/client/fuse_client.cpp index b166ae8993..c427d57cdb 100644 --- a/curvefs/src/client/fuse_client.cpp +++ b/curvefs/src/client/fuse_client.cpp @@ -881,7 +881,7 @@ CURVEFS_ERROR FuseClient::FuseOpReadDirPlus(fuse_req_t req, fuse_ino_t ino, inodeIds.emplace(dentry.inodeid()); } VLOG(3) << "batch get inode size = " << inodeIds.size(); - ret = inodeManager_->BatchGetInodeAttrAsync(ino, inodeIds, + ret = inodeManager_->BatchGetInodeAttrAsync(ino, &inodeIds, &inodeAttrMap); if (ret != CURVEFS_ERROR::OK) { LOG(ERROR) << "BatchGetInodeAttr failed when FuseOpReadDir" diff --git a/curvefs/src/client/inode_cache_manager.cpp b/curvefs/src/client/inode_cache_manager.cpp index c92cfd3a54..65f191c9a1 100644 --- a/curvefs/src/client/inode_cache_manager.cpp +++ b/curvefs/src/client/inode_cache_manager.cpp @@ -206,8 +206,8 @@ CURVEFS_ERROR InodeCacheManagerImpl::GetInodeAttr(uint64_t inodeId, CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttr( std::set *inodeIds, - std::list *attr) { - // get some inode in icache + std::list *attrs) { + // get some inode attr in icache for (auto iter = inodeIds->begin(); iter != inodeIds->end();) { std::shared_ptr inodeWrapper; NameLockGuard lock(nameLock_, std::to_string(*iter)); @@ -215,7 +215,7 @@ CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttr( if (ok) { InodeAttr tmpAttr; inodeWrapper->GetInodeAttr(&tmpAttr); - attr->emplace_back(tmpAttr); + attrs->emplace_back(tmpAttr); iter = inodeIds->erase(iter); } else { ++iter; @@ -226,7 +226,8 @@ CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttr( return CURVEFS_ERROR::OK; } - MetaStatusCode ret = metaClient_->BatchGetInodeAttr(fsId_, *inodeIds, attr); + MetaStatusCode ret = metaClient_->BatchGetInodeAttr(fsId_, *inodeIds, + attrs); if (MetaStatusCode::OK != ret) { LOG(ERROR) << "metaClient BatchGetInodeAttr failed, MetaStatusCode = " << ret << ", MetaStatusCode_Name = " @@ -237,21 +238,37 @@ CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttr( CURVEFS_ERROR InodeCacheManagerImpl::BatchGetInodeAttrAsync( uint64_t parentId, - const std::set &inodeIds, + std::set *inodeIds, std::map *attrs) { - if (inodeIds.empty()) { - return CURVEFS_ERROR::OK; + NameLockGuard lg(asyncNameLock_, std::to_string(parentId)); + std::map cachedAttr; + bool ok = iAttrCache_->Get(parentId, &cachedAttr); + + // get some inode attr 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->GetInodeAttr(&tmpAttr); + attrs->emplace(*iter, tmpAttr); + iter = inodeIds->erase(iter); + } else if (ok && cachedAttr.find(*iter) != cachedAttr.end()) { + attrs->emplace(*iter, cachedAttr[*iter]); + iter = inodeIds->erase(iter); + } else { + ++iter; + } } - NameLockGuard lg(asyncNameLock_, std::to_string(parentId)); - bool ok = iAttrCache_->Get(parentId, attrs); - if (ok) { + if (inodeIds->empty()) { return CURVEFS_ERROR::OK; } // split inodeIds by partitionId and batch limit std::vector> inodeGroups; - if (!metaClient_->SplitRequestInodes(fsId_, inodeIds, &inodeGroups)) { + if (!metaClient_->SplitRequestInodes(fsId_, *inodeIds, &inodeGroups)) { return CURVEFS_ERROR::NOTEXIST; } @@ -461,6 +478,11 @@ void InodeCacheManagerImpl::TrimIcache(uint64_t trimSize) { iCache_->Remove(inodeId); } trimSize--; + // remove the attr of the inode in iattrcache + auto parents = inodeWrapper->GetParentLocked(); + for (uint64_t parent : parents) { + iAttrCache_->Remove(parent, inodeId); + } } else { LOG(ERROR) << "icache size " << iCache_->Size(); assert(0); diff --git a/curvefs/src/client/inode_cache_manager.h b/curvefs/src/client/inode_cache_manager.h index 03e4eda0b1..bd74086e62 100644 --- a/curvefs/src/client/inode_cache_manager.h +++ b/curvefs/src/client/inode_cache_manager.h @@ -74,7 +74,7 @@ class InodeAttrCache { curve::common::LockGuard lg(iAttrCacheMutex_); auto iter = iAttrCache_.find(parentId); if (iter != iAttrCache_.end()) { - *imap = iter->second; + imap->insert(iter->second.begin(), iter->second.end()); return true; } return false; @@ -102,6 +102,14 @@ class InodeAttrCache { << size << ", after = " << iAttrCache_.size(); } + void Remove(uint64_t parentId, uint64_t inodeId) { + curve::common::LockGuard lg(iAttrCacheMutex_); + auto iter = iAttrCache_.find(parentId); + if (iter != iAttrCache_.end()) { + iter->second.erase(inodeId); + } + } + private: // inodeAttr cache; > std::map> iAttrCache_; @@ -140,7 +148,7 @@ class InodeCacheManager { virtual CURVEFS_ERROR BatchGetInodeAttrAsync( uint64_t parentId, - const std::set &inodeIds, + std::set *inodeIds, std::map *attrs) = 0; virtual CURVEFS_ERROR BatchGetXAttr(std::set *inodeIds, @@ -233,7 +241,7 @@ class InodeCacheManagerImpl : public InodeCacheManager, std::list *attrs) override; CURVEFS_ERROR BatchGetInodeAttrAsync(uint64_t parentId, - const std::set &inodeIds, + std::set *inodeIds, std::map *attrs = nullptr) override; CURVEFS_ERROR BatchGetXAttr(std::set *inodeIds, diff --git a/curvefs/src/client/inode_wrapper.h b/curvefs/src/client/inode_wrapper.h index e832bdc101..b5120401e6 100644 --- a/curvefs/src/client/inode_wrapper.h +++ b/curvefs/src/client/inode_wrapper.h @@ -203,6 +203,10 @@ class InodeWrapper : public std::enable_shared_from_this { return curve::common::UniqueLock(mtx_); } + const google::protobuf::RepeatedField& GetParentLocked() { + return inode_.parent(); + } + CURVEFS_ERROR UpdateParent(uint64_t oldParent, uint64_t newParent); // dir will not update parent diff --git a/curvefs/test/client/mock_inode_cache_manager.h b/curvefs/test/client/mock_inode_cache_manager.h index 141d30f06a..3b48280974 100644 --- a/curvefs/test/client/mock_inode_cache_manager.h +++ b/curvefs/test/client/mock_inode_cache_manager.h @@ -62,7 +62,7 @@ class MockInodeCacheManager : public InodeCacheManager { std::set *inodeIds, std::list *attrs)); MOCK_METHOD3(BatchGetInodeAttrAsync, - CURVEFS_ERROR(uint64_t parentId, const std::set &inodeIds, + CURVEFS_ERROR(uint64_t parentId, std::set *inodeIds, std::map *attrs)); MOCK_METHOD2(BatchGetXAttr, CURVEFS_ERROR( diff --git a/curvefs/test/client/test_inode_cache_manager.cpp b/curvefs/test/client/test_inode_cache_manager.cpp index 2f74ee4b83..4e4ac4fcbf 100644 --- a/curvefs/test/client/test_inode_cache_manager.cpp +++ b/curvefs/test/client/test_inode_cache_manager.cpp @@ -20,12 +20,14 @@ * Author: xuchaojie */ +#include #include #include #include #include #include +#include "curvefs/src/client/inode_wrapper.h" #include "curvefs/src/client/rpcclient/metaserver_client.h" #include "curvefs/test/client/mock_metaserver_client.h" #include "curvefs/src/client/inode_cache_manager.h" @@ -402,6 +404,58 @@ TEST_F(TestInodeCacheManager, BatchGetInodeAttr) { ASSERT_EQ(getAttrs.begin()->length(), fileLength); } +TEST_F(TestInodeCacheManager, BatchGetInodeAttrAsync) { + uint64_t parentId = 1; + uint64_t inodeId1 = 100; + uint64_t inodeId2 = 200; + + // in + std::set inodeIds; + inodeIds.emplace(inodeId1); + inodeIds.emplace(inodeId2); + + // out + Inode inode; + inode.set_inodeid(inodeId1); + + std::vector> inodeGroups; + inodeGroups.emplace_back(std::vector{inodeId2}); + + std::map attrs; + + RepeatedPtrField inodeAttrs; + inodeAttrs.Add()->set_inodeid(inodeId2); + + // fill icache + EXPECT_CALL(*metaClient_, GetInode(_, inodeId1, _, _)) + .WillOnce(DoAll(SetArgPointee<2>(inode), + Return(MetaStatusCode::OK))); + std::shared_ptr wrapper; + iCacheManager_->GetInode(inodeId1, wrapper); + + EXPECT_CALL(*metaClient_, SplitRequestInodes(_, _, _)) + .WillOnce(DoAll(SetArgPointee<2>(inodeGroups), + Return(true))); + EXPECT_CALL(*metaClient_, BatchGetInodeAttrAsync(_, _, _)) + .WillOnce(Invoke([inodeAttrs](uint32_t fsId, + const std::vector &inodeIds, + MetaServerClientDone *done) { + done->SetMetaStatusCode(MetaStatusCode::OK); + static_cast(done) + ->SetInodeAttrs(inodeAttrs); + done->Run(); + return MetaStatusCode::OK; + })); + + CURVEFS_ERROR ret = iCacheManager_->BatchGetInodeAttrAsync(parentId, + &inodeIds, &attrs); + + ASSERT_EQ(CURVEFS_ERROR::OK, ret); + ASSERT_EQ(attrs.size(), 2); + ASSERT_TRUE(attrs.find(inodeId1) != attrs.end()); + ASSERT_TRUE(attrs.find(inodeId2) != attrs.end()); +} + TEST_F(TestInodeCacheManager, BatchGetXAttr) { uint64_t inodeId1 = 100; uint64_t inodeId2 = 200;