Skip to content

Commit

Permalink
curvefs/client: fixed without link destination directory
Browse files Browse the repository at this point in the history 
and unlink source directory when source and destination
directory are different for rename() (opencurve#1153).
  • Loading branch information
@Wine93
Wine93 committed Mar 10, 2022
1 parent 05077ab commit 45e3a9a
Show file tree
Hide file tree
Showing 4 changed files with 160 additions and 26 deletions.
58 changes: 53 additions & 5 deletions curvefs/src/client/client_operator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,22 +209,70 @@ CURVEFS_ERROR RenameOperator::CommitTx() {
return CURVEFS_ERROR::OK;
}

void RenameOperator::UnlinkOldInode() {
if (oldInodeId_ == 0) {
return;
CURVEFS_ERROR RenameOperator::LinkInode(uint64_t inodeId) {
std::shared_ptr<InodeWrapper> inodeWrapper;
auto rc = inodeManager_->GetInode(inodeId, inodeWrapper);
if (rc != CURVEFS_ERROR::OK) {
LOG_ERROR("GetInode", rc);
return rc;
}

rc = inodeWrapper->LinkLocked();
if (rc != CURVEFS_ERROR::OK) {
LOG_ERROR("Link", rc);
return rc;
}

// CURVEFS_ERROR::OK
return rc;
}

CURVEFS_ERROR RenameOperator::UnLinkInode(uint64_t inodeId) {
std::shared_ptr<InodeWrapper> inodeWrapper;
auto rc = inodeManager_->GetInode(oldInodeId_, inodeWrapper);
auto rc = inodeManager_->GetInode(inodeId, inodeWrapper);
if (rc != CURVEFS_ERROR::OK) {
LOG_ERROR("GetInode", rc);
return;
return rc;
}

rc = inodeWrapper->UnLinkLocked();
if (rc != CURVEFS_ERROR::OK) {
LOG_ERROR("UnLink", rc);
return rc;
}

// CURVEFS_ERROR::OK
return rc;
}

CURVEFS_ERROR RenameOperator::LinkDestParentInode() {
// Link action is unnecessary when met one of the following 2 conditions:
// (1) source and destination under same directory
// (2) destination already exist
if (parentId_ == newParentId_ || oldInodeId_ != 0) {
return CURVEFS_ERROR::OK;
}
return LinkInode(newParentId_);
}

void RenameOperator::UnlinkSrcParentInode() {
// UnLink action is unnecessary when met the following 2 conditions:
// (1) source and destination under same directory
// (2) destination not exist
if (parentId_ == newParentId_ && oldInodeId_ == 0) {
return;
}
auto rc = UnLinkInode(parentId_);
LOG(INFO) << "Unlink source parent inode, retCode = " << rc;
}

void RenameOperator::UnlinkOldInode() {
if (oldInodeId_ == 0) {
return;
}

auto rc = UnLinkInode(oldInodeId_);
LOG(INFO) << "Unlink old inode, retCode = " << rc;
}

void RenameOperator::UpdateCache() {
Expand Down
6 changes: 6 additions & 0 deletions curvefs/src/client/client_operator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,10 @@ class RenameOperator {

CURVEFS_ERROR GetTxId();
CURVEFS_ERROR Precheck();
CURVEFS_ERROR LinkDestParentInode();
CURVEFS_ERROR PrepareTx();
CURVEFS_ERROR CommitTx();
void UnlinkSrcParentInode();
void UnlinkOldInode();
void UpdateCache();

Expand All @@ -69,6 +71,10 @@ class RenameOperator {

CURVEFS_ERROR PrepareRenameTx(const std::vector<Dentry>& dentrys);

CURVEFS_ERROR LinkInode(uint64_t inodeId);

CURVEFS_ERROR UnLinkInode(uint64_t inodeId);

private:
uint32_t fsId_;
uint64_t parentId_;
Expand Down
2 changes: 2 additions & 0 deletions curvefs/src/client/fuse_client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -631,8 +631,10 @@ CURVEFS_ERROR FuseClient::FuseOpRename(fuse_req_t req, fuse_ino_t parent,
CURVEFS_ERROR rc = CURVEFS_ERROR::OK;
RETURN_IF_UNSUCCESS(GetTxId);
RETURN_IF_UNSUCCESS(Precheck);
RETURN_IF_UNSUCCESS(LinkDestParentInode);
RETURN_IF_UNSUCCESS(PrepareTx);
RETURN_IF_UNSUCCESS(CommitTx);
renameOp.UnlinkSrcParentInode();
renameOp.UnlinkOldInode();
renameOp.UpdateCache();

Expand Down
120 changes: 99 additions & 21 deletions curvefs/test/client/test_fuse_client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -973,15 +973,32 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameBasic) {
.WillOnce(DoAll(SetArgPointee<2>(dstPartitionId),
SetArgPointee<3>(dstTxId), Return(MetaStatusCode::OK)));

// step2: precheck
// step2: link dest parent inode
Inode destParentInode;
destParentInode.set_inodeid(newparent);
destParentInode.set_nlink(2);
auto inodeWrapper =
std::make_shared<InodeWrapper>(destParentInode, metaClient_);
EXPECT_CALL(*inodeManager_, GetInode(newparent, _))
.WillOnce(DoAll(SetArgReferee<1>(inodeWrapper),
Return(CURVEFS_ERROR::OK)));
// include below unlink operate
EXPECT_CALL(*metaClient_, UpdateInode(_, _))
.Times(2)
.WillRepeatedly(Invoke([&](const Inode& inode,
InodeOpenStatusChange statusChange) {
return MetaStatusCode::OK;
}));

// step3: precheck
// dentry = { fsid, parentid, name, txid, inodeid, DELETE }
auto dentry = GenDentry(fsId, parent, name, srcTxId, inodeId, 0);
EXPECT_CALL(*dentryManager_, GetDentry(parent, name, _))
.WillOnce(DoAll(SetArgPointee<2>(dentry), Return(CURVEFS_ERROR::OK)));
EXPECT_CALL(*dentryManager_, GetDentry(newparent, newname, _))
.WillOnce(Return(CURVEFS_ERROR::NOTEXIST));

// step3: prepare tx
// step4: prepare tx
EXPECT_CALL(*metaClient_, PrepareRenameTx(_))
.WillOnce(Invoke([&](const std::vector<Dentry> &dentrys) {
auto srcDentry = GenDentry(fsId, parent, name, srcTxId + 1, inodeId,
Expand All @@ -1000,7 +1017,7 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameBasic) {
return MetaStatusCode::UNKNOWN_ERROR;
}));

// step4: commit tx
// step5: commit tx
EXPECT_CALL(*mdsClient_, CommitTx(_))
.WillOnce(Invoke([&](const std::vector<PartitionTxId> &txIds) {
if (txIds.size() == 2 && txIds[0].partitionid() == srcPartitionId &&
Expand All @@ -1012,7 +1029,16 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameBasic) {
return TopoStatusCode::TOPO_INTERNAL_ERROR;
}));

// step5: update cache
// step6: unlink source parent inode
Inode srcParentInode;
srcParentInode.set_inodeid(parent);
srcParentInode.set_nlink(3);
inodeWrapper = std::make_shared<InodeWrapper>(srcParentInode, metaClient_);
EXPECT_CALL(*inodeManager_, GetInode(parent, _))
.WillOnce(DoAll(SetArgReferee<1>(inodeWrapper),
Return(CURVEFS_ERROR::OK)));

// step7: update cache
EXPECT_CALL(*dentryManager_, DeleteCache(parent, name)).Times(1);
EXPECT_CALL(*dentryManager_, InsertOrReplaceCache(_))
.WillOnce(Invoke([&](const Dentry &dentry) {
Expand All @@ -1021,7 +1047,7 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameBasic) {
ASSERT_TRUE(dentry == dstDentry);
}));

// step6: set txid
// step8: set txid
EXPECT_CALL(*metaClient_, SetTxId(srcPartitionId, srcTxId + 1)).Times(1);
EXPECT_CALL(*metaClient_, SetTxId(dstPartitionId, dstTxId + 1)).Times(1);

Expand Down Expand Up @@ -1085,18 +1111,33 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameOverwrite) {
return TopoStatusCode::TOPO_INTERNAL_ERROR;
}));

// step5: unlink old inode
// step5: unlink source parent inode
Inode srcParentInode;
srcParentInode.set_inodeid(parent);
srcParentInode.set_nlink(3);
auto inodeWrapper =
std::make_shared<InodeWrapper>(srcParentInode, metaClient_);
EXPECT_CALL(*inodeManager_, GetInode(parent, _))
.WillOnce(DoAll(SetArgReferee<1>(inodeWrapper),
Return(CURVEFS_ERROR::OK)));
// include below unlink old inode
EXPECT_CALL(*metaClient_, UpdateInode(_, _))
.Times(2)
.WillRepeatedly(Invoke([&](const Inode& inode,
InodeOpenStatusChange statusChange) {
return MetaStatusCode::OK;
}));

// step6: unlink old inode
Inode inode;
inode.set_inodeid(oldInodeId);
inode.set_nlink(1);
auto inodeWrapper = std::make_shared<InodeWrapper>(inode, metaClient_);
inodeWrapper = std::make_shared<InodeWrapper>(inode, metaClient_);
EXPECT_CALL(*inodeManager_, GetInode(oldInodeId, _))
.WillOnce(
DoAll(SetArgReferee<1>(inodeWrapper), Return(CURVEFS_ERROR::OK)));
EXPECT_CALL(*metaClient_, UpdateInode(_, _))
.WillOnce(Return(MetaStatusCode::OK));
.WillOnce(DoAll(SetArgReferee<1>(inodeWrapper),
Return(CURVEFS_ERROR::OK)));

// step6: update cache
// step7: update cache
EXPECT_CALL(*dentryManager_, DeleteCache(parent, name)).Times(1);
EXPECT_CALL(*dentryManager_, InsertOrReplaceCache(_))
.WillOnce(Invoke([&](const Dentry &dentry) {
Expand All @@ -1105,7 +1146,7 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameOverwrite) {
ASSERT_TRUE(dentry == dstDentry);
}));

// step7: set txid
// step8: set txid
EXPECT_CALL(*metaClient_, SetTxId(partitionId, txId + 1)).Times(2);

auto rc = client_->FuseOpRename(req, parent, name.c_str(), newparent,
Expand Down Expand Up @@ -1179,33 +1220,70 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameNameTooLong) {
TEST_F(TestFuseVolumeClient, FuseOpRenameParallel) {
fuse_req_t req;
uint64_t txId = 0;
auto dentry = GenDentry(0, 0, "A", 0, 0, FILE);
auto dentry = GenDentry(1, 1, "A", 0, 10, FILE);
int nThread = 3;
int timesPerThread = 10000;
int times = nThread * timesPerThread;
volatile bool start = false;
bool success = true;

// step1: get txid
EXPECT_CALL(*metaClient_, GetTxId(_, _, _, _))
.Times(2 * times)
.WillRepeatedly(
DoAll(SetArgPointee<3>(txId), Return(MetaStatusCode::OK)));
.WillRepeatedly(DoAll(SetArgPointee<3>(txId),
Return(MetaStatusCode::OK)));

// step2: precheck
EXPECT_CALL(*dentryManager_, GetDentry(_, _, _))
.Times(2 * times)
.WillRepeatedly(
DoAll(SetArgPointee<2>(dentry), Return(CURVEFS_ERROR::OK)));
.WillRepeatedly(DoAll(SetArgPointee<2>(dentry),
Return(CURVEFS_ERROR::OK)));

// step3: prepare tx
EXPECT_CALL(*metaClient_, PrepareRenameTx(_))
.Times(times)
.WillRepeatedly(Return(MetaStatusCode::OK));

// step4: commit tx
EXPECT_CALL(*mdsClient_, CommitTx(_))
.Times(times)
.WillRepeatedly(Return(TopoStatusCode::TOPO_OK));

// step5: unlink source directory
Inode srcParentInode;
srcParentInode.set_inodeid(1);
srcParentInode.set_nlink(times + 2);
auto srcParentInodeWrapper =
std::make_shared<InodeWrapper>(srcParentInode, metaClient_);
EXPECT_CALL(*inodeManager_, GetInode(srcParentInode.inodeid(), _))
.Times(times)
.WillRepeatedly(DoAll(SetArgReferee<1>(srcParentInodeWrapper),
Return(CURVEFS_ERROR::OK)));
EXPECT_CALL(*metaClient_, UpdateInode(_, _))
.Times(times * 2) // include below operator which unlink old inode
.WillRepeatedly(Return(MetaStatusCode::OK));

// step6: unlink old inode
Inode inode;
inode.set_inodeid(10);
inode.set_nlink(times);
inode.set_type(FsFileType::TYPE_FILE);
auto inodeWrapper = std::make_shared<InodeWrapper>(inode, metaClient_);
EXPECT_CALL(*inodeManager_, GetInode(inode.inodeid(), _))
.Times(times)
.WillRepeatedly(DoAll(SetArgReferee<1>(inodeWrapper),
Return(CURVEFS_ERROR::OK)));

// step7: update cache
EXPECT_CALL(*dentryManager_, DeleteCache(_, _)).Times(times);
EXPECT_CALL(*dentryManager_, InsertOrReplaceCache(_)).Times(times);

// step8: set txid
EXPECT_CALL(*metaClient_, SetTxId(_, _))
.Times(2 * times)
.WillRepeatedly(
Invoke([&](uint32_t partitionId, uint64_t _) { txId = txId + 1; }));
.WillRepeatedly(Invoke([&](uint32_t partitionId, uint64_t _) {
txId = txId + 1;
}));

auto worker = [&](int count) {
while (!start) {
Expand All @@ -1230,7 +1308,7 @@ TEST_F(TestFuseVolumeClient, FuseOpRenameParallel) {
}

ASSERT_TRUE(success);
// in our caes, for each renema, we plus txid twice
// in our cases, for each renema, we plus txid twice
ASSERT_EQ(2 * times, txId);
}

Expand Down

0 comments on commit 45e3a9a

Please sign in to comment.