Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

curvefs: fix create copyset when exist already. #1002

Merged
merged 1 commit into from
Jan 19, 2022

Conversation

SeanHai
Copy link
Contributor

@SeanHai SeanHai commented Jan 17, 2022

curvefs: fix create copyset when exist already. If peers are all the same will success, otherwise return exist.

What problem does this PR solve?

Issue Number: close #xxx
#998

Problem Summary:

What is changed and how it works?

What's Changed:

How it Works:

Side effects(Breaking backward compatibility? Performance regression?):

Check List

  • Relevant documentation/comments is changed or added
  • I acknowledge that all my contributions will be made under the project's license

@SeanHai
Copy link
Contributor Author

SeanHai commented Jan 17, 2022

recheck

@@ -57,7 +57,7 @@ class CopysetNodeManager {

virtual CopysetNode* GetCopysetNode(PoolId poolId, CopysetId copysetId);

bool IsCopysetNodeExist(PoolId poolId, CopysetId copysetId);
int IsCopysetNodeExist(const CreateCopysetRequest::Copyset& copyset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add meaning of return value

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add meaning of return value

fixed

} else {
auto copysetNode = iter->second.get();
std::vector<Peer> peers;
copysetNode->ListPeers(&peers);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compare peer num

peers [1, 2, 3] peers[1, 2, 3, 4] is not same

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

compare peer num

peers [1, 2, 3] peers[1, 2, 3, 4] is not same

fixed

bool exists =
manager_->IsCopysetNodeExist(copyset.poolid(), copyset.copysetid());
if (exists) {
int exists = manager_->IsCopysetNodeExist(copyset);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

another problem is above CreateCopysetNode will stop directly when failed to create one copyset, so you can find whether there's a good reason to continue creating all copysets and return status for each copyset.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants