curvefs/client: fix update nlink error #1901
Conversation
|
recheck |
curvefs/src/client/inode_wrapper.h
Outdated
| @@ -52,13 +52,11 @@ constexpr int kAccessTime = 1 << 0; | |||
| constexpr int kChangeTime = 1 << 1; | |||
| constexpr int kModifyTime = 1 << 2; | |||
|
|
|||
| #define REFRESH_NLINK_IF_NEED \ | |||
| #define REFRESH_NLINK \ | |||
There was a problem hiding this comment.
you can move this macro to source file, the header file doesn't directly use it.
curvefs/src/client/fuse_client.h
Outdated
| CURVEFS_ERROR UpdateParentInodeMCTimeAndInvalidNlink( | ||
| fuse_ino_t parent, FsFileType type); | ||
| CURVEFS_ERROR UpdateParentMCTimeAndNlink( | ||
| fuse_ino_t parent, FsFileType type, int32_t nlink); |
There was a problem hiding this comment.
As for the third argument nlink, I only saw you passed either 1 or -1, does other values exist, like 2/3 or -2/-3? And if not, a proper way is using an enum to represent 1 or -1.
| @@ -580,7 +580,7 @@ CURVEFS_ERROR FuseClient::UpdateParentInodeMCTimeAndInvalidNlink( | |||
| parentInodeWrapper->UpdateTimestampLocked(kModifyTime | kChangeTime); | |||
|
|
|||
| if (FsFileType::TYPE_DIRECTORY == type) { | |||
| parentInodeWrapper->InvalidateNlink(); | |||
| parentInodeWrapper->UpdateNlinkLocked(nlink); | |||
There was a problem hiding this comment.
Is there a bug here? we changed the timestamp and nlink of an inode, but we don't call ShipToFlush which means the updated fields don't persist to metaserver?
There was a problem hiding this comment.
The update of nlink is done in metaserver and client at same time, the nlink in client is just to show, it need refresh first from metaserver when update to metaserver.
The update of time will set inode to dirty and flush backgroud
curvefs/src/client/inode_wrapper.h
Outdated
| } | ||
|
|
||
| void UpdateNlinkLocked(uint32_t nlink) { | ||
| inode_.set_nlink(inode_.nlink() + nlink); |
There was a problem hiding this comment.
Should we mark inode dirty and also set corresponding field in dirtyAttr?
There was a problem hiding this comment.
The update of nlink is done in metaserver and client at same time, the nlink in client is just to show, it need refresh first from metaserver when update to metaserver.
Signed-off-by: wanghai01 <seanhaizi@163.com>
Signed-off-by: wanghai01 seanhaizi@163.com
What problem does this PR solve?
fix the update inode nlink logic, the previous logic will update nlink incorrect at:
The fixed logic is every update of nlink to metaserver should refresh from metaserver first.
Issue Number: #1904
Problem Summary:
What is changed and how it works?
What's Changed:
How it Works:
Side effects(Breaking backward compatibility? Performance regression?):
Check List