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/HealthMonitor: avoid sending unnecessary MMonHealthChecks to leader #16478

Merged
merged 1 commit into from Jul 22, 2017

Conversation

Projects
None yet
3 participants
@xiexingguo
Member

xiexingguo commented Jul 21, 2017

If there is no historic warnings and no new warnings is generated,
skip sending MMonHealthChecks(for peon) or updating quorum_checks(for leader).

Signed-off-by: xie xingguo xie.xingguo@zte.com.cn

@tchaikov

aside from the nit, lgtm.

}
if (p == quorum_checks.end() && next.empty()) {
// no historic warnings and nothing new
return false;

This comment has been minimized.

@tchaikov

tchaikov Jul 21, 2017

Contributor

yeah, no news is good news.

@@ -87,6 +87,9 @@ struct health_check_map_t {
void clear() {
checks.clear();
}
bool empty() {

This comment has been minimized.

@tchaikov

tchaikov Jul 21, 2017

Contributor

mark this method const

@xiexingguo

This comment has been minimized.

Member

xiexingguo commented Jul 21, 2017

Thanks for the quick reply. Repushed.

@tchaikov tchaikov added the needs-qa label Jul 21, 2017

@tchaikov tchaikov added this to the luminous milestone Jul 21, 2017

}
if ((p == quorum_checks.end() && next.empty()) ||
(p != quorum_checks.end() && p->second == next)) {
// nothing changed

This comment has been minimized.

@xiexingguo

xiexingguo Jul 21, 2017

Member

@tchaikov Sorry, kefu. Leaked another evaluation conditon here.

This comment has been minimized.

@tchaikov

tchaikov Jul 21, 2017

Contributor

thanks for catching this! now even better!

This comment has been minimized.

@tchaikov

tchaikov Jul 21, 2017

Contributor

@xiexingguo could also put this way

if (p == quorum_checks.end()) {
  if (next.empty()) {
    return false;
  }
} else {
  if (p->second == next) {
    return false;
  }
}

less concise but more readable imho.

This comment has been minimized.

@xiexingguo

xiexingguo Jul 21, 2017

Member

Agree and applied!

mon/HealthMonitor: avoid sending unnecessary MMonHealthChecks to leader
If there is no historic warnings and no new warnings is generated,
skip sending MMonHealthChecks(for peon) or updating quorum_checks(for leader).

Signed-off-by: xie xingguo <xie.xingguo@zte.com.cn>

@liewegas liewegas merged commit 4d4b0b4 into ceph:master Jul 22, 2017

3 of 4 checks passed

make check make check failed
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check (arm64) make check succeeded
Details

@xiexingguo xiexingguo deleted the xiexingguo:wip-drop-empty-message branch Jul 22, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment