-
Notifications
You must be signed in to change notification settings - Fork 525
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
fix clear copyset creating flag when create copyset success #1417
Conversation
recheck |
0f24cd6
to
bfac19d
Compare
recheck |
1 similar comment
recheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@@ -866,6 +866,7 @@ TopoStatusCode TopologyManager::CreateCopyset( | |||
return ret; | |||
} | |||
|
|||
ClearCopysetCreating(copyset.poolId, copyset.copysetId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it's better to move this line of code before 856 and remove line 865, so you don't have to repeat it.
and should you add some unit test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
虽然都是去掉creating的标记,但是清理的时机是不一样的。如果都挪到856行,和原意还是有区别的。
Although the marking of creating is removed, the timing of cleaning is different. If it is moved to line 856, it is still different from the original intention.
bfac19d
to
a8f909c
Compare
What problem does this PR solve?
Issue Number: #1382
Problem Summary: Copyset is not automatically migrated after stopping metaserver
What is changed and how it works?
What's Changed: fix clear copyset creating flag when create copyset success
When copyset created success, it should clear the copyset creating flag. But the code miss this in code.