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(V3):reduce session timeout for lock command #8365

Closed
jiaxuanzhou opened this issue Aug 3, 2017 · 8 comments
Closed

etcdctl(V3):reduce session timeout for lock command #8365

jiaxuanzhou opened this issue Aug 3, 2017 · 8 comments

Comments

@jiaxuanzhou
Copy link
Contributor

jiaxuanzhou commented Aug 3, 2017

1,the default timeout of lock command is etcd\clientv3\concurrency\session.go: defaultSessionTTL = 60
in the production scenario, when there is an unexpected server crash of the client side(container scenario: kill the docker), the client process can not capture the signal of the syscall, then the other clients must wait 60s to gain the lock, the timeout is too long in production environment, i suggest to reduce the timeout to less than 10s.

2, suggest add another mechanism to ensure the lock will be released immediately when the client process who holds the lock exited.
Currently code just capture two kind of syscall signals to inform the process to close the communication link: signal.Notify(sigc, syscall.SIGINT, syscall.SIGTERM)

@heyitsanthony
Copy link
Contributor

i suggest to reduce the timeout to less than 10s.

How about a --ttl flag so it's configurable?

add another mechanism to ensure the lock will be released immediately when the client process who holds the lock exited.
Currently code just capture two kind of syscall signals to inform the process to close the communication link: signal.Notify(sigc, syscall.SIGINT, syscall.SIGTERM)

What other mechanism?

@jiaxuanzhou
Copy link
Contributor Author

@heyitsanthony
i agree with you if adding a flag to config the ttl, it can avoid the long waits when seesion expired and no other mechanism needed with a flag.

@xiang90
Copy link
Contributor

xiang90 commented Aug 4, 2017

@jiaxuanzhou do you want to work on this improvement?

@jiaxuanzhou
Copy link
Contributor Author

@xiang90 i can work on it ,will submit a patch soon.

@xiang90
Copy link
Contributor

xiang90 commented Aug 4, 2017

@jiaxuanzhou thanks!

@jiaxuanzhou
Copy link
Contributor Author

jiaxuanzhou commented Aug 5, 2017

@xiang90 @heyitsanthony i found NewSession(client *v3.Client, opts ...SessionOption) is called by elect cmd too, need to add the flag as a gloabal role or just as a sub-flag used by lock ?

@jiaxuanzhou
Copy link
Contributor Author

@heyitsanthony
related PR #8370 has been merged , the issue could be closed.

@heyitsanthony
Copy link
Contributor

ok, thanks!

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

No branches or pull requests

3 participants