[fix]curvefs: enableSumInDir in multi mount situation #2491
Conversation
|
cicheck |
curvefs/src/mds/fs_manager.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| *fsinfo = wrapper.ProtoFsInfo(); |
There was a problem hiding this comment.
*fsinfo not initialized, here temporary variable fsinfo here is not necessary
response->set_enablesumindir(wrapper.ProtoFsInfo().enablesumindir()); or Add method GetEnableSumInDir in class FsInfoWrapper and can use directly like response->set_enablesumindir(wrapper.GetEnableSumInDir())
| @@ -532,6 +533,7 @@ MdsClientImpl::RefreshSession(const std::vector<PartitionTxId> &txIds, | |||
| LOG(INFO) << "RefreshSession need update partition txid list: " | |||
| << response.DebugString(); | |||
| } | |||
| *enableSumInDir = response.enablesumindir(); | |||
There was a problem hiding this comment.
There may be thread safety issues here.
|
cicheck |
1 similar comment
|
cicheck |
Thx!And i will update in new version later. |
| @@ -532,6 +533,9 @@ MdsClientImpl::RefreshSession(const std::vector<PartitionTxId> &txIds, | |||
| LOG(INFO) << "RefreshSession need update partition txid list: " | |||
| << response.DebugString(); | |||
| } | |||
| if (*enableSumInDir == true && response.enablesumindir() == false) { | |||
| *enableSumInDir = response.enablesumindir(); | |||
There was a problem hiding this comment.
use enableSumInDir->store(response.enablesumindir());
And change use enableSumInDir_ value to enableSumInDir_.load()
std::atomic::store and std::atomic::load is thread safety.
There was a problem hiding this comment.
Get it! I will change afternoon.
|
cicheck |
|
cicheck |
curvefs/src/client/fuse_client.cpp
Outdated
| @@ -1446,14 +1448,15 @@ FuseClient::SetMountStatus(const struct MountOption *mountOption) { | |||
| } | |||
| inodeManager_->SetFsId(fsInfo_->fsid()); | |||
| dentryManager_->SetFsId(fsInfo_->fsid()); | |||
| enableSumInDir_ = fsInfo_->enablesumindir() && !FLAGS_enableCto; | |||
| enableSumInDir_.store(fsInfo_->enablesumindir() && !FLAGS_enableCto); | |||
There was a problem hiding this comment.
enableSumInDir_.store(fsInfo_->enablesumindir());
FLAGS_enableCto is no longer needed here.
|
cicheck |
|
cicheck |
bit-dance
force-pushed
the
issue2094
branch
4 times, most recently
from
99200a3 to
9dea224
Compare
|
cicheck |
|
cicheck |
|
cicheck |
2 similar comments
|
cicheck |
|
cicheck |
|
LGTM |
| std::shared_ptr<MdsClient> mdsCli) | ||
| : opt_(opt), metaCache_(metaCache), mdsCli_(mdsCli) {} | ||
| std::shared_ptr<MdsClient> mdsCli, | ||
| std::atomic<bool>* enableSumInDir = nullptr) |
There was a problem hiding this comment.
Pass by reference if enableSumInDir is never null. You can use //NOLINT to avoid cpplint.
Signed-off-by: jbk <[email protected]> add_atomic Signed-off-by: jbk <[email protected]>
|
cicheck |
1 similar comment
|
cicheck |
What problem does this PR solve?
Issue Number: #2094
Problem Summary:
What is changed and how it works?
When the fs mount is mounted multiple times with different enableSumInDir switches, the xattr information recorded in the metadata will be inaccurate.
What's Changed:
check the mds flag in lease_executor and update for fuse client
How it Works:
just like above
Side effects(Breaking backward compatibility? Performance regression?):
Check List