curvefs: support client mdsaddrs override #2892
Conversation
h0hmj
changed the title
|
cicheck |
curvefs/src/mds/fs_manager.cpp
Outdated
| @@ -1280,5 +1289,10 @@ bool FsManager::FillVolumeInfo(common::Volume* volume) { | |||
| return true; | |||
| } | |||
|
|
|||
| void FsManager::SetClientMdsAddrsOverride(const std::string& addrs) { | |||
| WriteLockGuard lock(clientMdsAddrsOverrideMutex_); | |||
| clientMdsAddrsOverride_ = addrs; | |||
There was a problem hiding this comment.
It's better to do some basic verification of the IP addresses, otherwise, it may cause client exception.
Another point is that it is better to update the addresses gradually, you cannot change all addresses directly. For example, you must change from (A,B,C) -> (A,B,C,D,E,F) -> (D, E, F).
There was a problem hiding this comment.
BTW, should you persist mds addresses into etcd? Becuase there's chance that mds exit before client get new addresses.
There was a problem hiding this comment.
It's better to do some basic verification of the IP addresses, otherwise, it may cause client exception.
Another point is that it is better to update the addresses gradually, you cannot change all addresses directly. For example, you must change from (A,B,C) -> (A,B,C,D,E,F) -> (D, E, F).
ok i will make sure active mds is in the override ones
There was a problem hiding this comment.
BTW, should you persist mds addresses into etcd? Becuase there's chance that mds exit before client get new addresses.
no. it's just a temporary online advanced setting.
| @@ -550,7 +552,17 @@ MdsClientImpl::RefreshSession(const std::vector<PartitionTxId> &txIds, | |||
| FsQuotaChecker::GetInstance().UpdateQuotaCache( | |||
| response.fscapacity(), response.fsusedbytes()); | |||
| } | |||
|
|
|||
| if (response.has_mdsaddrsoverride()) { | |||
There was a problem hiding this comment.
it's not a good idea to put heavy logic in here, MdsClientImpl is just used for communicate with MDS
| curve::common::SplitString(response.mdsaddrsoverride(), ",", | ||
| &mdsAddrsOverride); | ||
| if (!mdsAddrsOverride.empty()) { | ||
| mdsOpt_.rpcRetryOpt.addrs = mdsAddrsOverride; |
There was a problem hiding this comment.
ok, will add safe get&set for rpcexcutor_'s retryOption
src/client/mds_client.cpp
Outdated
| ReadLockGuard lock(retryOptLock_); | ||
| // happen when mds scaling down | ||
| if (mdsindex >= static_cast<int>(retryOpt_.addrs.size())) { | ||
| return -EHOSTDOWN; |
There was a problem hiding this comment.
return -brpc::ELOGOFF is better
Signed-off-by: h0hmj <[email protected]>
Signed-off-by: h0hmj <[email protected]>
Signed-off-by: h0hmj <[email protected]>
|
cicheck |
4 similar comments
|
cicheck |
|
cicheck |
|
cicheck |
|
cicheck |
|
New unit test cases are needed to verify the code you added. |
Signed-off-by: h0hmj <[email protected]>
|
cicheck |
yes. new uts have been added. |
|
cicheck |
we need to make client alive when migrate mds to new ones. so we add online client mds addrs override to broadcast by session.
tool -> send SetClientMdsAddrsOverride to mds, will keep active mds
mds -> refresh session send to client
client -> change mds addr to override ones
use case:
client -> mds A,B,C
change to client -> mds A,B,C,D,E,F
change to client -> mds D,E,F