PR 8178 says: > With this change adding a header-rewrite rule to add the Connection:closed header will still work to trigger the draining logic. But core logic to clean up HTTP/1.x connections will not trigger the draining logic.
It doesn't sound like the feature was taken out purposefully. I still think it was broken at some point if it doesn't work, and it should be restored. When the feature was added on 8.x we didn't have good tools to test it. We might be able to test it now with ProxyVerifier. Lack of tests should not be used as justification to silently kill features. I do see the complication, but that's inside the core. I don't see how adding the API, which requires plugins to check HTTP version and do different things for each version, can be easier for the plugins to use. The version number in the name should be removed at a minimum (i.e. the API should work for every HTTP version). Masakazu On Mon, Jun 17, 2024 at 8:52 PM Fei Deng <duke8...@gmail.com> wrote: > Because of this? > https://github.com/apache/trafficserver/pull/11046#discussion_r1583528415 > > The code in the HttpTransact.cc that checks for the header removes the > header and never adds it back if it is a http2 connection. > > In an internal discussion in Yahoo I mentioned we can probably find a exact > moment to set the header, such that it happens after the check for the > whole “connection: keep-alive” and “connection: close” in HttpTransact.cc, > but at the end it was deemed too complicated, and a simple flag set by an > api might be easier for the plugins to use. > > Do we have a test for that feature mentioned in the docs? > > > Regards, > Fei Deng > > Sent from my iPhone > > > On Mon, Jun 17, 2024 at 10:45 PM Masakazu Kitajo <mas...@apache.org> > wrote: > > > It's a documented feature. > > > > > https://docs.trafficserver.apache.org/en/9.2.x/admin-guide/plugins/header_rewrite.en.html#close-connections-for-draining > > > > > https://docs.trafficserver.apache.org/en/latest/admin-guide/plugins/header_rewrite.en.html#close-connections-for-draining > > > > On Mon, Jun 17, 2024 at 8:32 PM Masakazu Kitajo <mas...@apache.org> > wrote: > > > > > Purposefully? Where did it happen? I'd say it was broken. We have that > > use > > > case and need a FIX. > > > > > > Masakazu > > > > > > On Mon, Jun 17, 2024 at 6:47 PM Fei Deng <duke8...@apache.org> wrote: > > > > > >> Actually it’s not possible by setting the “Connection: close” header. > > That > > >> was the initial intention of PR #11046, but with all the discussions > it > > >> looks like that functionality was taken out purposefully. > > >> > > >> Regards, > > >> Fei Deng > > >> > > >> Sent from my iPhone > > >> > > >> > > >> On Mon, Jun 17, 2024 at 8:42 PM Masakazu Kitajo <mas...@apache.org> > > >> wrote: > > >> > > >> > It has been possible by setting "Connection: Close" header, which > > means > > >> > that a server wants no more requests on the connection and wants to > > >> close > > >> > it. > > >> > > > >> > If you want to close a connection on some conditions, you could > check > > >> the > > >> > conditions and set the header by header_rewrite (or any plugins). > And > > >> those > > >> > plugins that used to work for H1 do the same for H2 as well without > > any > > >> > changes (and probably for H3 as well, though it's not implemented > > yet). > > >> > > > >> > From plugins' perspective, everything on ATS is HTTP/1. Headers are > > >> > converted to H1 representation (e.g. ":authority" -> "Host"). It's > > >> > natural to use the H1 interface between ATS core and plugins. In > that > > >> way, > > >> > plugins don't even need to know/check the HTTP version. I don't > think > > >> > having something just for H2 is a right thing, unless it's truly an > H2 > > >> > specific thing (e.g. setting max H2 frame size). I didn't use "2" > even > > >> for > > >> > ServerPush because I knew H3 was already coming. > > >> > > > >> > Masakazu > > >> > > > >> > On Mon, Jun 17, 2024 at 9:34 AM Fei Deng <duke8...@apache.org> > wrote: > > >> > > > >> > > TSReturnCode TSHttp2GraceShutdown(TSHttpTxn txnp) > > >> > > > > >> > > With this new API, plugins can request a grace shutdown by sending > > >> GOAWAY > > >> > > frames (https://httpwg.org/specs/rfc7540.html#GOAWAY). > > >> > > > > >> > > This will also replace this PR > > >> > > https://github.com/apache/trafficserver/pull/11046 > > >> > > > > >> > > Fei Deng > > >> > > > > >> > > > >> > > > > > >
