On Sun, Jun 11, 2017 at 1:27 PM, James Peach <jpe...@apache.org> wrote:
> > > On 9 May 2017, at 0:02, Masakazu Kitajo wrote: > > Hi, >> >> Don't worry guys. I'm working on it. >> >> I merged the PR to have a draining (shedding) logic for HTTP/2 (Double >> GOAWAY frames), and now I'm generalizing the flag so that we can invoke >> draining on both HTTP/1 and HTTP/2. I'm going to add "Connection: close" >> header automatically for HTTP/1. So, "proxy.config.stop.shutdown_timeout" >> will be available for both versions. The PR was step 1, and it will be >> step >> 2. >> > > “proxy.config.stop.shutdown_timeout” is still a mis-feature. What really > matters for shutting down is how much traffic you are still serving, which > is expressed by “proxy.config.restart.active_client_threshold”. I think > you might also want to express a maximum time you are willing to wait, but > just having a fixed time is not what we should be doing. I agree the current implementation is not perfect, but that part is what we haven't completed yet. Probably we need to use the both configurations because we can’t wait forever until traffic goes down, and we can use "proxy.config.stop.shutdown_timeout" as "proxy.config.restart.active_client_timeout”. I think their intention is the same and the difference is which process use it. > "traffic_ctl server stop --drain" is also in my mind. Once we have one >> generalized way for draining in traffic_server, we can invoke it from >> anywhere we want (e.g. from traffic_manager, TS API, metric). Currently it >> is invoked using the flag, but eventually, we should be able to decouple >> draining and shutdown, and should also be able to remove the flag and >> sleep(). >> >> Doing these all at once is heavy and the PR would be big, and it slows >> review and progress, IMO. You might want full featured graceful shutdown >> but this is a reason why I didn't ask to do it. >> > > I’m fine with (and even prefer) doing this in pieces, but we need to agree > on the direction and that the end result will be what we want. Ok, probably the step after PR#2106 will be how we invoke graceful shutdown and which process should wait for / determine the completion of draining. These should be the points we still need to discuss and agree on. Actually, I basically agree on your last suggestion. I'm not keeping the configuration and the flag, but just left them on the PR. > Thanks, >> Masakazu >> >> >> >> On Tue, May 9, 2017 at 2:40 PM, CrazyCow <zhangzizhong0...@gmail.com> >> wrote: >> >> I don't disagree. The reasons I chose this way are: >>> 1. We are using other stuff in my team instead of traffic_ctl to manage >>> the >>> process and do the upgrade. >>> 2. traffic_ctl --drain can only support HTTP and it can only be used when >>> restarting ATS. That makes it hardly useful in our use case. >>> >>> 2017-05-08 22:16 GMT-07:00 James Peach <jpe...@apache.org>: >>> >>> >>>> >>>> On 8 May 2017, at 21:48, Miles Libbey wrote: >>>> >>>> We'd also like a bit more fine grained control in the process -- we >>>> >>>>> frequently want to perform maintenance on a server (upgrading ATS; >>>>> upgrading the OS; performing hardware changes, etc) after draining but >>>>> before restarting ATS. I suppose this would mean allowing the --drain >>>>> option to apply to traffic_Ctl server stop. >>>>> >>>>> >>>> Yes I agree that draining make sense for stop as well as restart. >>>> >>>> >>>> >>>> miles >>>>> >>>>> On Mon, May 8, 2017 at 8:19 PM, James Peach <jpe...@apache.org> wrote: >>>>> >>>>> This patch adds another separate shutdown mechanism that only works for >>>>>> HTTP/2. I think that we really ought to have a single, well-defined >>>>>> graceful >>>>>> shutdown that works for all protocols. >>>>>> >>>>>> In HTTP/1.1 it works like this: >>>>>> - You (possibly dynamically) set >>>>>> proxy.config.restart.active_client_threshold >>>>>> - You run “traffic_ctl server restart —drain” >>>>>> >>>>>> Then, once client connections have been drained to the threshold, >>>>>> traffic_server restarts. Note that this assumes that you have some >>>>>> additional orchestration that triggers header_rewrite to inject a >>>>>> “Connection: close” header, and tell the GSLB to stop sending new >>>>>> connections. >>>>>> >>>>>> After this HTTP/2 change, there is a new graceful shutdown path >>>>>> - You (at startup only) set proxy.config.stop.shutdown_tim >>>>>> eout >>>>>> - You send a signal to traffic_server >>>>>> >>>>>> This flips a global variable to the “drain” state, then sleeps in the >>>>>> signal >>>>>> handler until the timeout is reached. In HTTP/2 only, any new >>>>>> >>>>> connections >>> >>>> will be accepted and then immediately closed. >>>>>> >>>>>> To rationalize these disparate approaches, I suggest that we go back >>>>>> to >>>>>> the >>>>>> traffic_ctl methodology, and enhance it so that it sends a message to >>>>>> traffic_server that puts it into a known draining state. This should >>>>>> be >>>>>> published in a metric and should be reversible so you can abort the >>>>>> drain. >>>>>> The metric can be observed by HTTP/1.1 and HTTP/2 to take appropriate >>>>>> action. We should also add a new setting >>>>>> “proxy.config.restart.active_client_timeout” (or something like that) >>>>>> >>>>> to >>> >>>> handle the maximum time to wait for traffic to drain. >>>>>> >>>>>> I’m not sure whether it is a good idea to close connections in HTTP/2 >>>>>> while >>>>>> we are in draining state. If there is a desire for this, I would like >>>>>> >>>>> it >>> >>>> to >>>>>> be configurable (defaulting to off). >>>>>> >>>>>> On 8 May 2017, at 18:17, Masakazu Kitajo wrote: >>>>>> >>>>>> Merged #1710. >>>>>> >>>>>>> >>>>>>> -- >>>>>>> You are receiving this because you are subscribed to this thread. >>>>>>> Reply to this email directly or view it on GitHub: >>>>>>> https://github.com/apache/trafficserver/pull/1710#event-1073784726 >>>>>>> >>>>>>> >>>>>> >>>>>> >>>>>> >>>>>> >>>
