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

etcdctl: fix health check condition #3697

Merged
merged 1 commit into from
Oct 18, 2015
Merged

Conversation

mqliang
Copy link
Contributor

@mqliang mqliang commented Oct 17, 2015

This PR helps to fix the condition to determine if a member is healthy.

BTW, when determine if the whole cluster if healthy, it assumes that cluster is healthy as long as there is _one_ member is healthy. Should we replace that condition to "iff _most_ of the members are healthy". If your answer is YES, I am glad to help fix this. (After all, I don't know the implementation of health endpoint, maybe all the member will response "false" if most of members are unhealthy)

@mqliang mqliang changed the title Fix health check condition client: Fix health check condition Oct 17, 2015
@mqliang mqliang changed the title client: Fix health check condition client: fix health check condition Oct 17, 2015
@xiang90
Copy link
Contributor

xiang90 commented Oct 17, 2015

BTW, when determine if the whole cluster if healthy, it assumes that cluster is healthy as long as there is one member is healthy

The health endpoint of a member indicates the cluster-wide heath. So the implementation is correct actually.

Thanks for your fix!

@xiang90
Copy link
Contributor

xiang90 commented Oct 17, 2015

@mqliang Can you please update the commit message to etcdctl: fix health check condition?

@mqliang mqliang changed the title client: fix health check condition etcdctl: fix health check condition Oct 17, 2015
@yichengq
Copy link
Contributor

@mqliang xiang means the git commit title. We always use this format: https://github.com/coreos/etcd/blob/master/CONTRIBUTING.md#format-of-the-commit-message

@raoofm
Copy link
Contributor

raoofm commented Oct 18, 2015

Will this be back-ported?

Does this mean that the current health check is not valid?

@xiang90
Copy link
Contributor

xiang90 commented Oct 18, 2015

@raoofm This does not affect any release version of etcd as far as I can see. I will double check.

@xiang90
Copy link
Contributor

xiang90 commented Oct 18, 2015

@raoofm But yes, we will back-port this anyway.

xiang90 added a commit that referenced this pull request Oct 18, 2015
etcdctl: fix health check condition
@xiang90 xiang90 merged commit f08d750 into etcd-io:master Oct 18, 2015
@mqliang mqliang deleted the cluster-health branch October 18, 2015 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants