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

Does the user need to eat all keep alive messages? #8168

Closed
helinwang opened this issue Jun 23, 2017 · 4 comments
Closed

Does the user need to eat all keep alive messages? #8168

helinwang opened this issue Jun 23, 2017 · 4 comments

Comments

@helinwang
Copy link

When user keep alive a lease, the function KeepAlive returns a channel of keep alive messages. From the code in Session, it creates a net go-routine and eats all messages.
Is it necessary for the lease keep alive to work properly?

I am under impression that it does not have to
according to the code here, since the implementation just discard the message if the channel is full.

Just want to confirm if I need to add extra code to eat all the messages every time I create a lease and keep alive it.

@yudai
Copy link
Contributor

yudai commented Jun 23, 2017

AFIK, you virtually must drain messages from the channel returned by KeepAlive if the code has been not changed since #7446. Otherwise, keep alive messages are sent every 500ms to the server, as I reported in the issue.

@helinwang
Copy link
Author

helinwang commented Jun 23, 2017

@yudai Thanks for the response! The issue you created is very informative! Yes the code is the same.

A side question, do your application actually care the result coming from the keep alive response channel? I am curious about the use case, maybe I could learn something to improve the program that I am currently working.

Thanks!
Helin

@yudai
Copy link
Contributor

yudai commented Jun 23, 2017

In my code, there are some code like for range keepAliveCh {}.
I actually don't have any use case that requires to check the content of messages to this channel so far.

@xiang90
Copy link
Contributor

xiang90 commented Jun 27, 2017

i think the question is well answered. thanks @yudai. closing.

@xiang90 xiang90 closed this as completed Jun 27, 2017
luoyancn added a commit to luoyancn/dubhe that referenced this issue Aug 11, 2018
When use etcd keepalive method, we must consume all messages
from the queen it retured.Otherwise, the queen would be full.
See more detail, visit the follow links:

etcd-io/etcd#9952
etcd-io/etcd#8168
https://github.com/coreos/etcd/blob/master/clientv3/concurrency/session.go#L63
https://www.cnxct.com/etcd-lease-keepalive-debug-note/
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants