-
Notifications
You must be signed in to change notification settings - Fork 844
Drain stop and drain improve #2962
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
Conversation
|
Thanks for making progress on drain feature. Some parts of the changes are written by me, but to be honest, I'm not familiar with this area. I just imitated code that is already there. I'd like traffic_manager / traffic_ctl experts to review this. @jpeach This includes what we discussed on the list long time ago. Please review it if you have time. |
|
[approve ci] |
|
The autest failure is not reachable now. Can you help to trigger another one? @maskit @bryancall @zwoop @scw00 ? Thanks! |
|
[approve ci] |
|
[approve ci debian] |
|
@zizhong This is getting close, can you squash the commits into one? |
Add maximum time limit for draining (cherry picked from commit a9fe3b0) Add traffic_ctl server stop --drain (cherry picked from commit 14ba9d6) Conflicts: proxy/Main.cc proxy/ProxyClientSession.cc proxy/ProxyClientSession.h 1. Adding `traffic server drain` subcommand 2. Adding default option, --no-new-connection option and --undo option (cherry picked from commit 139fe5f)
2fa4343 to
c398bde
Compare
|
@zwoop Thanks. Did the squash. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
As I commented on the previous PR, "--undo" looks a bit strange because of its lack of asymmetry. "start and stop" vs "drain and drain --undo". But I don't have better ideas.
Since I wrote some of the code, I can't approve this because it would be a kind of self approval.
This PR includes
Add maximum time limit for drainingfrom @maskit,Add traffic_ctl server stop --drainfrom @maskit andFixing #2906: improve drainingfrom me.For the
Fixing #2906: improve draining,traffic server drainsubcommandTODO:
is_server_idle_from_new_connection()@maskit @bryancall @zwoop @oknet @scw00 @masaori335 can you guys review it when you got time? Any ideas would be much appreciated!