curvefs: fix trash will delete file data more than once

Signed-off-by: wanghai01 <seanhaizi@163.com>
xu-chaojie · Dec 5, 2023 · bfd5acb · bfd5acb
1 parent dd8ce61
commit bfd5acb
Show file tree

Hide file tree

Showing 11 changed files with 268 additions and 245 deletions.
diff --git a/curvefs/proto/metaserver.proto b/curvefs/proto/metaserver.proto
@@ -267,7 +267,7 @@ message Inode {
     optional uint64 rdev = 16;
     // field 17 is left for compatibility
     map<uint64, S3ChunkInfoList> s3ChunkInfoMap = 18; // TYPE_S3 only, first is chunk index
-    optional uint32 dtime = 19;
+    optional uint64 dtime = 19;
     optional uint32 openmpcount = 20; // openmpcount mount points had the file open
     map<string, bytes> xattr = 21;
     repeated uint64 parent = 22;

diff --git a/curvefs/src/metaserver/inode_manager.cpp b/curvefs/src/metaserver/inode_manager.cpp
@@ -283,26 +283,31 @@ MetaStatusCode InodeManager::DeleteInode(uint32_t fsId, uint64_t inodeId,
     VLOG(6) << "DeleteInode, fsId = " << fsId << ", inodeId = " << inodeId;
     NameLockGuard lg(inodeLock_, GetInodeLockName(fsId, inodeId));
     InodeAttr attr;
-    MetaStatusCode retGetAttr =
-        inodeStorage_->GetAttr(Key4Inode(fsId, inodeId), &attr);
-    if (retGetAttr != MetaStatusCode::OK) {
-        VLOG(9) << "GetInodeAttr fail, fsId = " << fsId
-                << ", inodeId = " << inodeId
-                << ", ret = " << MetaStatusCode_Name(retGetAttr);
+    auto ret = inodeStorage_->GetAttr(Key4Inode(fsId, inodeId), &attr);
+    if (ret == MetaStatusCode::NOT_FOUND) {
+        return MetaStatusCode::OK;
+    } else if (ret != MetaStatusCode::OK) {
+        LOG(ERROR) << "GetInodeAttr fail, fsId = " << fsId
+                   << ", inodeId = " << inodeId
+                   << ", ret = " << MetaStatusCode_Name(ret);
+        return ret;
     }
 
-    MetaStatusCode ret =
-        inodeStorage_->Delete(Key4Inode(fsId, inodeId), logIndex);
+    ret = inodeStorage_->Delete(Key4Inode(fsId, inodeId), logIndex);
     if (ret != MetaStatusCode::OK) {
         LOG(ERROR) << "DeleteInode fail, fsId = " << fsId
                    << ", inodeId = " << inodeId
                    << ", ret = " << MetaStatusCode_Name(ret);
         return ret;
     }
 
-    if (retGetAttr == MetaStatusCode::OK) {
+    // if nlink is 0 means this inode is already in trash
+    if (attr.nlink() != 0) {
         // get attr success
         --(*type2InodeNum_)[attr.type()];
+    } else {
+        // delete trash item
+        trash_->Remove(inodeId);
     }
     VLOG(6) << "DeleteInode success, fsId = " << fsId
             << ", inodeId = " << inodeId;
@@ -353,8 +358,7 @@ MetaStatusCode InodeManager::UpdateInode(const UpdateInodeRequest& request,
 
     if (request.has_nlink()) {
         if (old.nlink() != 0 && request.nlink() == 0) {
-            uint32_t now = TimeUtility::GetTimeofDaySec();
-            old.set_dtime(now);
+            old.set_dtime(TimeUtility::GetTimeofDaySec());
             needAddTrash = true;
         }
         VLOG(9) << "update inode nlink, from " << old.nlink() << " to "
@@ -373,7 +377,6 @@ MetaStatusCode InodeManager::UpdateInode(const UpdateInodeRequest& request,
 
     bool fileNeedDeallocate =
         (needAddTrash && (FsFileType::TYPE_FILE == old.type()));
-    bool s3NeedTrash = (needAddTrash && (FsFileType::TYPE_S3 == old.type()));
 
     std::shared_ptr<storage::StorageTransaction> txn;
     if (needUpdate) {
@@ -388,8 +391,8 @@ MetaStatusCode InodeManager::UpdateInode(const UpdateInodeRequest& request,
         }
     }
 
-    if (s3NeedTrash) {
-        trash_->Add(old.fsid(), old.inodeid(), old.dtime());
+    if (needAddTrash) {
+        trash_->Add(old.inodeid(), old.dtime());
         --(*type2InodeNum_)[old.type()];
     }
 
@@ -610,25 +613,6 @@ MetaStatusCode InodeManager::UpdateInodeWhenCreateOrRemoveSubNode(
     return MetaStatusCode::OK;
 }
 
-MetaStatusCode InodeManager::InsertInode(const Inode& inode, int64_t logIndex) {
-    CHECK_APPLIED();
-    VLOG(6) << "InsertInode, " << inode.ShortDebugString();
-
-    // 2. insert inode
-    MetaStatusCode ret = inodeStorage_->Insert(inode, logIndex);
-    if (ret != MetaStatusCode::OK) {
-        LOG(ERROR) << "InsertInode fail, " << inode.ShortDebugString()
-                   << ", ret = " << MetaStatusCode_Name(ret);
-        return ret;
-    }
-
-    if (inode.nlink() == 0) {
-        trash_->Add(inode.fsid(), inode.inodeid(), inode.dtime());
-    }
-
-    return MetaStatusCode::OK;
-}
-
 bool InodeManager::GetInodeIdList(std::list<uint64_t>* inodeIdList) {
     return inodeStorage_->GetAllInodeId(inodeIdList);
 }

diff --git a/curvefs/src/metaserver/inode_manager.h b/curvefs/src/metaserver/inode_manager.h
@@ -108,8 +108,6 @@ class InodeManager {
     MetaStatusCode UpdateInodeWhenCreateOrRemoveSubNode(
         const Dentry& dentry, const Time& tm, bool isCreate, int64_t logIndex);
 
-    MetaStatusCode InsertInode(const Inode& inode, int64_t logIndex);
-
     bool GetInodeIdList(std::list<uint64_t>* inodeIdList);
 
     // Update one or more volume extent slice

diff --git a/curvefs/src/metaserver/partition.cpp b/curvefs/src/metaserver/partition.cpp
@@ -69,7 +69,9 @@ Partition::Partition(PartitionInfo partition,
     dentryStorage_ =
         std::make_shared<DentryStorage>(kvStorage_, nameGen_, nDentry);
 
-    auto trash = std::make_shared<TrashImpl>(inodeStorage_);
+    auto trash = std::make_shared<TrashImpl>(inodeStorage_,
+        partitionInfo_.fsid(), partitionInfo_.poolid(),
+        partitionInfo_.copysetid(), partitionInfo_.partitionid());
     inodeManager_ = std::make_shared<InodeManager>(
         inodeStorage_, trash, partitionInfo_.mutable_filetype2inodenum());
     txManager_ = std::make_shared<TxManager>(dentryStorage_, partitionInfo_);
@@ -359,15 +361,6 @@ MetaStatusCode Partition::PaddingInodeS3ChunkInfo(int32_t fsId,
     return inodeManager_->PaddingInodeS3ChunkInfo(fsId, inodeId, m, limit);
 }
 
-MetaStatusCode Partition::InsertInode(const Inode& inode, int64_t logIndex) {
-    PRECHECK(inode.fsid(), inode.inodeid());
-    auto ret = inodeManager_->InsertInode(inode, logIndex);
-    if (ret == MetaStatusCode::IDEMPOTENCE_OK) {
-        ret = MetaStatusCode::OK;
-    }
-    return ret;
-}
-
 bool Partition::GetInodeIdList(std::list<uint64_t>* InodeIdList) {
     return inodeManager_->GetInodeIdList(InodeIdList);
 }
@@ -470,12 +463,12 @@ uint64_t Partition::GetNewInodeId() {
     return newInodeId;
 }
 
-uint32_t Partition::GetInodeNum() {
-    return static_cast<uint32_t>(inodeStorage_->Size());
+uint64_t Partition::GetInodeNum() {
+    return inodeStorage_->Size();
 }
 
-uint32_t Partition::GetDentryNum() {
-    return static_cast<uint32_t>(dentryStorage_->Size());
+uint64_t Partition::GetDentryNum() {
+    return dentryStorage_->Size();
 }
 
 bool Partition::EmptyInodeStorage() { return inodeStorage_->Empty(); }

diff --git a/curvefs/src/metaserver/partition.h b/curvefs/src/metaserver/partition.h
@@ -138,8 +138,6 @@ class Partition {
     virtual MetaStatusCode GetAllBlockGroup(
         std::vector<DeallocatableBlockGroup>* deallocatableBlockGroupVec);
 
-    MetaStatusCode InsertInode(const Inode& inode, int64_t logIndex);
-
     bool GetInodeIdList(std::list<uint64_t>* InodeIdList);
 
     // if partition has no inode or no dentry, it is deletable
@@ -167,9 +165,9 @@ class Partition {
     // if no available inode id in this partiton ,return UINT64_MAX
     uint64_t GetNewInodeId();
 
-    uint32_t GetInodeNum();
+    uint64_t GetInodeNum();
 
-    uint32_t GetDentryNum();
+    uint64_t GetDentryNum();
 
     bool EmptyInodeStorage();