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

mon: avoid start election twice when quorum enter #10150

Merged
merged 1 commit into from Feb 7, 2017

Conversation

Projects
None yet
4 participants
@songbaisen

songbaisen commented Jul 6, 2016

mon: avoid start election twice when quorum enter

when quorum enter it is no meaning to start election twice

Signed-off-by: song baisen song.baisen@zte.com.cn

@songbaisen songbaisen changed the title from mon: free the RoutedRequest before clear it. to mon: avoid start election twice when quorum ente Jul 7, 2016

@songbaisen songbaisen changed the title from mon: avoid start election twice when quorum ente to mon: avoid start election twice when quorum enter Jul 7, 2016

@liewegas liewegas added the core label Jul 7, 2016

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 7, 2016

It looks like those are the only two callers for start_participating.. why not just change the implementation?

@songbaisen

This comment has been minimized.

songbaisen commented Jul 8, 2016

@ Hello sage! Because when we start election we will check whether we set participating true.So before start election we must be set the participating true. And the interface of start_participating can be use for later on.

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 8, 2016

I don't think we'll have another user, given that the only current user is the admin commands that are there just for debugging purposes anyway.

@songbaisen

This comment has been minimized.

songbaisen commented Jul 11, 2016

@liewegas Yes sir.Done it.

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 11, 2016

don't we still need to set teh participating flag = true or false (like your original methods did)?

@songbaisen

This comment has been minimized.

songbaisen commented Jul 12, 2016

@liewegas Yes sir.Done it.

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 13, 2016

This looks like the original patch. start_participating() now has no users. So we can also remove it. Or... just change the start_participating to do what your new begin_participating does. (Which gets us to the same point, except the name is different.)

Either way, let's get rid of the old begin_participating() implementation which is no longer needed?

@jecluis right?

@songbaisen

This comment has been minimized.

songbaisen commented Jul 13, 2016

@liewegas Oh I got it! Thank you sir!

@ghost

This comment has been minimized.

ghost commented Nov 22, 2016

jenkins test this please (jenkins hung)

@jecluis

This looks okay. I think we should keep the 'start_participating()' method, as it is consumed by the 'quorum enter' commands (both via network and admin socket).

There's however a nit that should be addressed, regarding the removal of the closing elector defgroup, which I presume was made by mistake.

@@ -425,9 +425,6 @@ class Elector {
* @post @p participating is true
*/
void start_participating();
/**

This comment has been minimized.

@jecluis

jecluis Nov 22, 2016

Member

why are you removing the closing tag for this doxygen defgroup? I'm assuming this was a mistake.

This comment has been minimized.

@songbaisen

songbaisen Nov 23, 2016

@jecluis 😄 Thank you for review sir.I think this note is no use.So I remove it.

songbaisen
mon: avoid start election twice when quorum enter
when quorum enter it is no meaning to start election twice

Signed-off-by: song baisen <song.baisen@zte.com.cn>
@songbaisen

This comment has been minimized.

songbaisen commented Nov 25, 2016

@jecluis Done it!

@liewegas liewegas merged commit 402b358 into ceph:master Feb 7, 2017

3 checks passed

Signed-off-by all commits in this PR are signed
Details
Unmodifed Submodules submodules for project are unmodified
Details
default Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment