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

MDSMonitor: only clog changes to active #18600

Merged
merged 1 commit into from
Nov 16, 2017
Merged

MDSMonitor: only clog changes to active #18600

merged 1 commit into from
Nov 16, 2017

Conversation

batrick
Copy link
Member

@batrick batrick commented Oct 27, 2017

Otherwise we get constant INFO messages that an MDS is active.

Fixes: http://tracker.ceph.com/issues/21959

Signed-off-by: Patrick Donnelly pdonnell@redhat.com

@batrick
Copy link
Member Author

batrick commented Oct 27, 2017

@jcsp PTAL.

Copy link
Member

@jecluis jecluis left a comment

Choose a reason for hiding this comment

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

makes sense to me, but feel free to wait for @jcsp's review if in doubt :)

@@ -738,7 +738,7 @@ bool MDSMonitor::prepare_beacon(MonOpRequestRef op)
info->state_seq = seq;
});

if (state == MDSMap::STATE_ACTIVE) {
if (info.state != MDSMap::STATE_ACTIVE && state == MDSMap::STATE_ACTIVE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

info is a reference that will have been modified in place by modify_daemon, so I don't think this will ever be true. Probably can just swap this block with the preceding one, or be even more explicit by adding an old_state temporarily for this check?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, now I'm not sure how this solution actually worked for me (probably my testing was not right). I'll fix this...

Otherwise we get constant INFO messages that an MDS is active.

Fixes: http://tracker.ceph.com/issues/21959

Signed-off-by: Patrick Donnelly <pdonnell@redhat.com>
@batrick
Copy link
Member Author

batrick commented Nov 6, 2017

@jcsp PTAL.

@jcsp
Copy link
Contributor

jcsp commented Nov 7, 2017

retest this please

@batrick batrick merged commit 997a688 into ceph:master Nov 16, 2017
batrick added a commit that referenced this pull request Nov 16, 2017
* refs/pull/18600/head:
	MDSMonitor: only clog changes to active

Reviewed-by: John Spray <john.spray@redhat.com>
Reviewed-by: João Eduardo Luís <joao@suse.de>
@batrick batrick deleted the i21959 branch December 14, 2017 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants