From b23c3e3484649a382e71a0760bf74e3b309aeeaa Mon Sep 17 00:00:00 2001 From: hzwuhongsong Date: Thu, 29 Jun 2023 10:21:01 +0800 Subject: [PATCH] curvefs/client: fix s3 object will not be removed --- curvefs/src/metaserver/trash.cpp | 16 ++++++- curvefs/test/metaserver/BUILD | 2 +- curvefs/test/metaserver/trash_test.cpp | 65 +++++++++++++++++++++++--- 3 files changed, 74 insertions(+), 9 deletions(-) diff --git a/curvefs/src/metaserver/trash.cpp b/curvefs/src/metaserver/trash.cpp index c75a15e5b1..b6d99d52d4 100644 --- a/curvefs/src/metaserver/trash.cpp +++ b/curvefs/src/metaserver/trash.cpp @@ -168,6 +168,21 @@ MetaStatusCode TrashImpl::DeleteInodeAndData(const TrashItem &item) { clientAdaptorOption.chunkSize = s3Info.chunksize(); s3Adaptor_->Reinit(clientAdaptorOption, s3Info.ak(), s3Info.sk(), s3Info.endpoint(), s3Info.bucketname()); + ret = inodeStorage_->PaddingInodeS3ChunkInfo(item.fsId, + item.inodeId, inode.mutable_s3chunkinfomap()); + if (ret != MetaStatusCode::OK) { + LOG(ERROR) << "GetInode chunklist fail, fsId = " << item.fsId + << ", inodeId = " << item.inodeId + << ", retCode = " << MetaStatusCode_Name(ret); + return ret; + } + if (inode.s3chunkinfomap().empty()) { + LOG(WARNING) << "GetInode chunklist empty, fsId = " << item.fsId + << ", inodeId = " << item.inodeId; + return MetaStatusCode::NOT_FOUND; + } + VLOG(9) << "DeleteInodeAndData, inode: " + << inode.ShortDebugString(); int retVal = s3Adaptor_->Delete(inode); if (retVal != 0) { LOG(ERROR) << "S3ClientAdaptor delete s3 data failed" @@ -176,7 +191,6 @@ MetaStatusCode TrashImpl::DeleteInodeAndData(const TrashItem &item) { return MetaStatusCode::S3_DELETE_ERR; } } - ret = inodeStorage_->Delete(Key4Inode(item.fsId, item.inodeId)); if (ret != MetaStatusCode::OK && ret != MetaStatusCode::NOT_FOUND) { LOG(ERROR) << "Delete Inode fail, fsId = " << item.fsId diff --git a/curvefs/test/metaserver/BUILD b/curvefs/test/metaserver/BUILD index 9548d12721..12ef0512b7 100644 --- a/curvefs/test/metaserver/BUILD +++ b/curvefs/test/metaserver/BUILD @@ -26,7 +26,6 @@ cc_test( "mock_metaserver_s3.h", "metaserver_s3_adaptor_test.h", "metaserver_s3_adaptor_test.cpp", - "mock_metaserver_s3_adaptor.h", "metaserver_s3_test.cpp", "mock_s3compact_inode.h", "s3compact_test.cpp", @@ -46,6 +45,7 @@ cc_test( "//curvefs/test/metaserver/storage:metaserver_storage_test_utils", "//curvefs/test/metaserver/mock:metaserver_test_mock", "@com_google_absl//absl/types:optional", + "//curvefs/test/client/rpcclient:rpcclient_test_mock", ], ) diff --git a/curvefs/test/metaserver/trash_test.cpp b/curvefs/test/metaserver/trash_test.cpp index f37630af36..d6a9642b48 100644 --- a/curvefs/test/metaserver/trash_test.cpp +++ b/curvefs/test/metaserver/trash_test.cpp @@ -28,6 +28,8 @@ #include "curvefs/src/metaserver/storage/rocksdb_storage.h" #include "curvefs/test/metaserver/storage/utils.h" #include "src/fs/ext4_filesystem_impl.h" +#include "curvefs/test/client/rpcclient/mock_mds_client.h" +#include "curvefs/test/metaserver/mock_metaserver_s3_adaptor.h" using ::testing::AtLeast; using ::testing::StrEq; @@ -45,6 +47,7 @@ namespace { auto localfs = curve::fs::Ext4FileSystemImpl::getInstance(); } +using ::curvefs::client::rpcclient::MockMdsClient; using ::curvefs::metaserver::storage::KVStorage; using ::curvefs::metaserver::storage::StorageOptions; using ::curvefs::metaserver::storage::RocksDBStorage; @@ -103,7 +106,29 @@ class TestTrash : public ::testing::Test { inode.set_gid(0); inode.set_mode(0); inode.set_nlink(0); - inode.set_type(FsFileType::TYPE_FILE); + inode.set_type(FsFileType::TYPE_S3); + return inode; + } + + Inode GenInodeHasChunks(uint32_t fsId, uint64_t inodeId) { + Inode inode; + inode.set_fsid(fsId); + inode.set_inodeid(inodeId); + inode.set_length(4096); + inode.set_ctime(0); + inode.set_ctime_ns(0); + inode.set_mtime(0); + inode.set_mtime_ns(0); + inode.set_atime(0); + inode.set_atime_ns(0); + inode.set_uid(0); + inode.set_gid(0); + inode.set_mode(0); + inode.set_nlink(0); + inode.set_type(FsFileType::TYPE_S3); + + S3ChunkInfoList s3ChunkInfoList; + inode.mutable_s3chunkinfomap()->insert({0, s3ChunkInfoList}); return inode; } @@ -119,17 +144,18 @@ TEST_F(TestTrash, testAdd3ItemAndDelete) { option.scanPeriodSec = 1; option.expiredAfterSec = 1; + option.mdsClient = std::make_shared(); + option.s3Adaptor = std::make_shared(); trashManager_->Init(option); trashManager_->Run(); - auto trash1 = std::make_shared(inodeStorage_); auto trash2 = std::make_shared(inodeStorage_); trashManager_->Add(1, trash1); trashManager_->Add(2, trash2); - inodeStorage_->Insert(GenInode(1, 1)); - inodeStorage_->Insert(GenInode(1, 2)); - inodeStorage_->Insert(GenInode(2, 1)); + inodeStorage_->Insert(GenInodeHasChunks(1, 1)); + inodeStorage_->Insert(GenInodeHasChunks(1, 2)); + inodeStorage_->Insert(GenInodeHasChunks(2, 1)); ASSERT_EQ(inodeStorage_->Size(), 3); @@ -138,17 +164,42 @@ TEST_F(TestTrash, testAdd3ItemAndDelete) { trash2->Add(2, 1, 0); std::this_thread::sleep_for(std::chrono::seconds(5)); - std::list list; trashManager_->ListItems(&list); ASSERT_EQ(0, list.size()); - ASSERT_EQ(inodeStorage_->Size(), 0); trashManager_->Fini(); } +TEST_F(TestTrash, testAdd3ItemAndNoDelete) { + TrashOption option; + option.scanPeriodSec = 1; + option.expiredAfterSec = 1; + option.mdsClient = std::make_shared(); + option.s3Adaptor = std::make_shared(); + trashManager_->Init(option); + trashManager_->Run(); + + auto trash1 = std::make_shared(inodeStorage_); + trashManager_->Add(1, trash1); + + inodeStorage_->Insert(GenInode(1, 1)); + inodeStorage_->Insert(GenInode(1, 2)); + inodeStorage_->Insert(GenInode(2, 1)); + ASSERT_EQ(inodeStorage_->Size(), 3); + trash1->Add(1, 1, 0); + trash1->Add(1, 2, 0); + std::this_thread::sleep_for(std::chrono::seconds(5)); + std::list list; + + trashManager_->ListItems(&list); + ASSERT_EQ(0, list.size()); + ASSERT_EQ(inodeStorage_->Size(), 3); + trashManager_->Fini(); +} + } // namespace metaserver } // namespace curvefs