Skip to content

Commit

Permalink
curvefs/client: fix bug of support summary info in dir xattr, the par…
Browse files Browse the repository at this point in the history 
…ent relationship is not correct.
  • Loading branch information
@SeanHai
SeanHai committed Mar 11, 2022
1 parent d0544e2 commit ab608d4
Show file tree
Hide file tree
Showing 12 changed files with 106 additions and 207 deletions.
2 changes: 1 addition & 1 deletion curvefs/conf/tools.conf
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,5 +26,5 @@ s3.endpoint=endpoint
s3.bucket_name=bucket
s3.blocksize=4194304
s3.chunksize=67108864
# statistic info in xattr
# statistic info in xattr, hardlink will not be supported when enable
enableSumInDir=false
2 changes: 1 addition & 1 deletion curvefs/src/client/curve_fuse_op.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ void FuseReplyErrByErrCode(fuse_req_t req, CURVEFS_ERROR errcode) {
fuse_reply_err(req, ENOTEMPTY);
break;
case CURVEFS_ERROR::NOTSUPPORT:
fuse_reply_err(req, ENOSYS);
fuse_reply_err(req, EOPNOTSUPP);
break;
case CURVEFS_ERROR::NAMETOOLONG:
fuse_reply_err(req, ENAMETOOLONG);
Expand Down
124 changes: 57 additions & 67 deletions curvefs/src/client/fuse_client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include <vector>
#include <stack>
#include <set>
#include <unordered_map>

#include "curvefs/proto/mds.pb.h"
#include "curvefs/src/client/fuse_common.h"
Expand Down Expand Up @@ -308,9 +309,9 @@ CURVEFS_ERROR FuseClient::FuseOpOpen(fuse_req_t req, fuse_ino_t ino,
XAttr xattr;
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(length)});
std::list<uint64_t> parentIds;
if (inodeManager_->GetParent(inode->inodeid(), &parentIds)) {
ret = UpdateParentInodeXattr(parentIds, xattr, false);
uint64_t parentId;
if (inodeManager_->GetParent(inode->inodeid(), &parentId)) {
ret = UpdateParentInodeXattr(parentId, xattr, false);
} else {
LOG(ERROR) << "inodeManager getParent failed, inodeId = "
<< inode;
Expand Down Expand Up @@ -417,8 +418,7 @@ CURVEFS_ERROR FuseClient::MakeNode(fuse_req_t req, fuse_ino_t parent,
}
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(inodeWrapper->GetLength())});
ret = UpdateParentInodeXattr(
std::list<uint64_t>({parent}), xattr, true);
ret = UpdateParentInodeXattr(parent, xattr, true);
}

GetDentryParamFromInode(inodeWrapper, e);
Expand Down Expand Up @@ -516,7 +516,7 @@ CURVEFS_ERROR FuseClient::RemoveNode(fuse_req_t req, fuse_ino_t parent,

if (enableSumInDir_) {
// remove parent relationshaip
inodeManager_->RemoveParent(ino, parent);
inodeManager_->ClearParent(ino);

// update parent summary info
XAttr xattr;
Expand All @@ -528,8 +528,7 @@ CURVEFS_ERROR FuseClient::RemoveNode(fuse_req_t req, fuse_ino_t parent,
}
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(inodeWrapper->GetLength())});
ret = UpdateParentInodeXattr(
std::list<uint64_t>({parent}), xattr, false);
ret = UpdateParentInodeXattr(parent, xattr, false);
}

inodeManager_->ClearInodeCache(ino);
Expand Down Expand Up @@ -669,17 +668,15 @@ CURVEFS_ERROR FuseClient::FuseOpRename(fuse_req_t req, fuse_ino_t parent,
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(inodeWrapper->GetLength())});

if (inodeManager_->UpdateParent(ino, parent, newparent)) {
if (inodeManager_->UpdateParent(ino, newparent)) {
// TODO(wanghai): deal with failure
rc = UpdateParentInodeXattr(
std::list<uint64_t>({parent}), xattr, false);
rc = UpdateParentInodeXattr(parent, xattr, false);
if (rc != CURVEFS_ERROR::OK) {
LOG(ERROR) << "UpdateParentInodeXattr failed, parentId = "
<< parent;
return rc;
}
rc = UpdateParentInodeXattr(
std::list<uint64_t>({newparent}), xattr, true);
rc = UpdateParentInodeXattr(newparent, xattr, true);
if (rc != CURVEFS_ERROR::OK) {
LOG(ERROR) << "UpdateParentInodeXattr failed, parentId = "
<< newparent;
Expand Down Expand Up @@ -782,10 +779,10 @@ CURVEFS_ERROR FuseClient::FuseOpSetAttr(fuse_req_t req, fuse_ino_t ino,
XAttr xattr;
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(std::abs(changeSize))});
std::list<uint64_t> parentIds;
if (inodeManager_->GetParent(ino, &parentIds)) {
uint64_t parentId;
if (inodeManager_->GetParent(ino, &parentId)) {
bool direction = changeSize > 0;
ret = UpdateParentInodeXattr(parentIds, xattr, direction);
ret = UpdateParentInodeXattr(parentId, xattr, direction);
} else {
LOG(ERROR) << "inodeManager getParent failed, inodeId = "
<< inode;
Expand Down Expand Up @@ -893,7 +890,8 @@ CURVEFS_ERROR FuseClient::CalOneLayerSumInfo(Inode *inode) {

CURVEFS_ERROR FuseClient::CalAllLayerSumInfo(Inode *inode) {
std::stack<uint64_t> iStack;
// use set can deal with hard link
// record hard link, <inodeId, need2minus>
std::unordered_map<uint64_t, uint64_t> hardLinkMap;
std::set<uint64_t> inodeIds;
std::list<InodeAttr> attrs;
auto ino = inode->inodeid();
Expand Down Expand Up @@ -935,6 +933,15 @@ CURVEFS_ERROR FuseClient::CalAllLayerSumInfo(Inode *inode) {
}
rentries++;
rfbytes += it.length();
// record hardlink
if (it.type() != FsFileType::TYPE_DIRECTORY &&
it.nlink() > 1) {
if (hardLinkMap.count(it.inodeid())) {
hardLinkMap[it.inodeid()] += it.length();
} else {
hardLinkMap.emplace(it.inodeid(), 0);
}
}
}
inodeIds.clear();
attrs.clear();
Expand All @@ -944,6 +951,11 @@ CURVEFS_ERROR FuseClient::CalAllLayerSumInfo(Inode *inode) {
}
}

// deal with hardlink
for (const auto &it : hardLinkMap) {
rfbytes -= it.second;
}

inode->mutable_xattr()->insert({XATTRRFILES,
std::to_string(rfiles)});
inode->mutable_xattr()->insert({XATTRRSUBDIRS,
Expand Down Expand Up @@ -1180,8 +1192,7 @@ CURVEFS_ERROR FuseClient::FuseOpSymlink(fuse_req_t req, const char *link,
xattr.mutable_xattrinfos()->insert({XATTRFILES, "1"});
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(inodeWrapper->GetLength())});
ret = UpdateParentInodeXattr(
std::list<uint64_t>({parent}), xattr, true);
ret = UpdateParentInodeXattr(parent, xattr, true);
}

GetDentryParamFromInode(inodeWrapper, e);
Expand All @@ -1194,6 +1205,12 @@ CURVEFS_ERROR FuseClient::FuseOpLink(fuse_req_t req, fuse_ino_t ino,
LOG(INFO) << "FuseOpLink, ino: " << ino
<< ", newparent: " << newparent
<< ", newname: " << newname;
if (enableSumInDir_) {
// don't support hardlink
LOG(ERROR) << "FuseOpLink doesn't support when enableSumInDir.";
return CURVEFS_ERROR::NOTSUPPORT;
}

if (strlen(newname) > option_.maxNameLength) {
return CURVEFS_ERROR::NAMETOOLONG;
}
Expand Down Expand Up @@ -1244,19 +1261,6 @@ CURVEFS_ERROR FuseClient::FuseOpLink(fuse_req_t req, fuse_ino_t ino,
return ret;
}

if (enableSumInDir_) {
// recore parent inodeId
inodeManager_->AddParent(inodeWrapper->GetInodeId(), newparent);
// update parent summary info
XAttr xattr;
xattr.mutable_xattrinfos()->insert({XATTRENTRIES, "1"});
xattr.mutable_xattrinfos()->insert({XATTRFILES, "1"});
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(inodeWrapper->GetLength())});
ret = UpdateParentInodeXattr(
std::list<uint64_t>({newparent}), xattr, true);
}

GetDentryParamFromInode(inodeWrapper, e);
return ret;
}
Expand Down Expand Up @@ -1297,10 +1301,6 @@ CURVEFS_ERROR FuseClient::FuseOpRelease(fuse_req_t req, fuse_ino_t ino,
return ret;
}

if (enableSumInDir_) {
// update parent relationship
inodeManager_->ClearParent(ino);
}
return ret;
}

Expand All @@ -1313,41 +1313,31 @@ void FuseClient::FlushAll() {
FlushInodeAll();
}

CURVEFS_ERROR FuseClient::UpdateParentInodeXattr(
const std::list<uint64_t> &parentIds,
CURVEFS_ERROR FuseClient::UpdateParentInodeXattr(uint64_t parentId,
const XAttr &xattr, bool direction) {
for (const auto &parentId : parentIds) {
VLOG(1) << "UpdateParentInodeXattr inodeId = " << parentId
<< ", \nxattr = " << xattr.DebugString();
std::shared_ptr<InodeWrapper> pInodeWrapper;
CURVEFS_ERROR ret = inodeManager_->GetInode(parentId, pInodeWrapper);
if (ret != CURVEFS_ERROR::OK) {
LOG(ERROR) << "UpdateParentInodeXattr get parent inode fail, ret = "
<< ret << ", inodeid = " << parentId;
return ret;
}
// if dirty, flush attr first
if (pInodeWrapper->Dirty()) {
ret = pInodeWrapper->SyncAttr();
if (ret != CURVEFS_ERROR::OK) {
LOG(ERROR) << "sync parent inode attr failed when"
<< " UpdateParentInodeXattr, inodeId = " << parentId;
return ret;
}
}
::curve::common::UniqueLock lgGuard = pInodeWrapper->GetUniqueLock();
auto inode = pInodeWrapper->GetMutableInodeUnlocked();
for (const auto &it : xattr.xattrinfos()) {
auto iter = inode->mutable_xattr()->find(it.first);
if (iter != inode->mutable_xattr()->end()) {
if (!AddUllStringToFirst(&(iter->second), it.second,
direction)) {
return CURVEFS_ERROR::INTERNAL;
}
VLOG(1) << "UpdateParentInodeXattr inodeId = " << parentId
<< ", direction = " << direction
<< ", \nxattr = " << xattr.DebugString();
std::shared_ptr<InodeWrapper> pInodeWrapper;
CURVEFS_ERROR ret = inodeManager_->GetInode(parentId, pInodeWrapper);
if (ret != CURVEFS_ERROR::OK) {
LOG(ERROR) << "UpdateParentInodeXattr get parent inode fail, ret = "
<< ret << ", inodeid = " << parentId;
return ret;
}

::curve::common::UniqueLock lgGuard = pInodeWrapper->GetUniqueLock();
auto inode = pInodeWrapper->GetMutableInodeUnlocked();
for (const auto &it : xattr.xattrinfos()) {
auto iter = inode->mutable_xattr()->find(it.first);
if (iter != inode->mutable_xattr()->end()) {
if (!AddUllStringToFirst(&(iter->second), it.second,
direction)) {
return CURVEFS_ERROR::INTERNAL;
}
}
pInodeWrapper->FlushXattrAsync();
}
pInodeWrapper->FlushXattrAsync();
return CURVEFS_ERROR::OK;
}

Expand Down
2 changes: 1 addition & 1 deletion curvefs/src/client/fuse_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ class FuseClient {
return 0;
}

CURVEFS_ERROR UpdateParentInodeXattr(const std::list<uint64_t> &parentIds,
CURVEFS_ERROR UpdateParentInodeXattr(uint64_t parentId,
const XAttr &xattr, bool direction);

private:
Expand Down
6 changes: 3 additions & 3 deletions curvefs/src/client/fuse_s3_client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,9 @@ CURVEFS_ERROR FuseS3Client::FuseOpWrite(fuse_req_t req, fuse_ino_t ino,
XAttr xattr;
xattr.mutable_xattrinfos()->insert({XATTRFBYTES,
std::to_string(*wSize)});
std::list<uint64_t> parentIds;
if (inodeManager_->GetParent(ino, &parentIds)) {
ret = UpdateParentInodeXattr(parentIds, xattr, true);
uint64_t parentId;
if (inodeManager_->GetParent(ino, &parentId)) {
ret = UpdateParentInodeXattr(parentId, xattr, true);
} else {
LOG(ERROR) << "inodeManager getParent failed, inodeId = " << ino;
return CURVEFS_ERROR::INTERNAL;
Expand Down
35 changes: 12 additions & 23 deletions curvefs/src/client/inode_cache_manager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -183,47 +183,36 @@ void InodeCacheManagerImpl::FlushInodeOnce() {
void InodeCacheManagerImpl::AddParent(uint64_t inodeId, uint64_t parentId) {
curve::common::LockGuard lg2(parentIdMapMutex_);
if (parentIdMap_.count(inodeId)) {
parentIdMap_[inodeId].emplace_back(parentId);
} else {
parentIdMap_.emplace(inodeId, std::list<uint64_t>({parentId}));
}
}

void InodeCacheManagerImpl::RemoveParent(uint64_t inodeId, uint64_t parentId) {
curve::common::LockGuard lg2(parentIdMapMutex_);
if (parentIdMap_.count(inodeId)) {
auto iter = std::find(parentIdMap_[inodeId].begin(),
parentIdMap_[inodeId].end(), parentId);
if (iter != parentIdMap_[inodeId].end()) {
parentIdMap_[inodeId].erase(iter);
}
LOG(WARNING) << "AddParent but exist, inodeId = " << inodeId
<< ", parentId = " << parentIdMap_[inodeId]
<< ", need2AddParentId = " << parentId;
}
parentIdMap_[inodeId] = parentId;
}

void InodeCacheManagerImpl::ClearParent(uint64_t inodeId) {
curve::common::LockGuard lg2(parentIdMapMutex_);
parentIdMap_.erase(inodeId);
VLOG(1) << "ClearParent inodeId = " << inodeId
<< ", after clear the parentmap size = "
<< parentIdMap_.size();
}

bool InodeCacheManagerImpl::UpdateParent(uint64_t inodeId, uint64_t oldParentId,
bool InodeCacheManagerImpl::UpdateParent(uint64_t inodeId,
uint64_t newParentId) {
curve::common::LockGuard lg2(parentIdMapMutex_);
if (parentIdMap_.count(inodeId)) {
auto iter = std::find(parentIdMap_[inodeId].begin(),
parentIdMap_[inodeId].end(), oldParentId);
if (iter != parentIdMap_[inodeId].end()) {
*iter = newParentId;
return true;
}
parentIdMap_[inodeId] = newParentId;
return true;
}
return false;
}

bool InodeCacheManagerImpl::GetParent(uint64_t inodeId,
std::list<uint64_t> *parentIds) {
uint64_t *parentId) {
curve::common::LockGuard lg2(parentIdMapMutex_);
if (parentIdMap_.count(inodeId)) {
*parentIds = parentIdMap_[inodeId];
*parentId = parentIdMap_[inodeId];
return true;
}
return false;
Expand Down
19 changes: 6 additions & 13 deletions curvefs/src/client/inode_cache_manager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,15 +89,11 @@ class InodeCacheManager {

virtual void AddParent(uint64_t inodeId, uint64_t parentId) = 0;

virtual void RemoveParent(uint64_t inodeId, uint64_t parentId) = 0;

virtual void ClearParent(uint64_t inodeId) = 0;

virtual bool GetParent(uint64_t inodeId,
std::list<uint64_t> *parentIds) = 0;
virtual bool GetParent(uint64_t inodeId, uint64_t *parentId) = 0;

virtual bool UpdateParent(uint64_t inodeId, uint64_t oldParentId,
uint64_t newParentId) = 0;
virtual bool UpdateParent(uint64_t inodeId, uint64_t newParentId) = 0;

protected:
uint32_t fsId_;
Expand Down Expand Up @@ -151,14 +147,11 @@ class InodeCacheManagerImpl : public InodeCacheManager {

void AddParent(uint64_t inodeId, uint64_t parentId) override;

void RemoveParent(uint64_t inodeId, uint64_t parentId) override;

void ClearParent(uint64_t inodeId) override;

bool GetParent(uint64_t inodeId, std::list<uint64_t> *parentIds) override;
bool GetParent(uint64_t inodeId, uint64_t *parentId) override;

bool UpdateParent(uint64_t inodeId, uint64_t oldParentId,
uint64_t newParentId) override;
bool UpdateParent(uint64_t inodeId, uint64_t newParentId) override;

private:
std::shared_ptr<MetaServerClient> metaClient_;
Expand All @@ -168,8 +161,8 @@ class InodeCacheManagerImpl : public InodeCacheManager {
std::map<uint64_t, std::shared_ptr<InodeWrapper>> dirtyMap_;
curve::common::Mutex dirtyMapMutex_;

// inodeid to parent inodeid, may have more parent at hard link
std::map<uint64_t, std::list<uint64_t>> parentIdMap_;
// inodeid to parent inodeid
std::map<uint64_t, uint64_t> parentIdMap_;
curve::common::Mutex parentIdMapMutex_;

curve::common::GenericNameLock<Mutex> nameLock_;
Expand Down
9 changes: 3 additions & 6 deletions curvefs/src/client/inode_wrapper.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,9 @@ void InodeWrapper::FlushAttrAsync() {
}

void InodeWrapper::FlushXattrAsync() {
if (dirty_) {
LockSyncingXattr();
auto *done = new UpdateXattrAsyncDone(shared_from_this());
metaClient_->UpdateXattrAsync(inode_, done);
dirty_ = false;
}
LockSyncingXattr();
auto *done = new UpdateXattrAsyncDone(shared_from_this());
metaClient_->UpdateXattrAsync(inode_, done);
}

void InodeWrapper::FlushS3ChunkInfoAsync() {
Expand Down
Loading

0 comments on commit ab608d4

Please sign in to comment.