You are viewing a plain text version of this content. The canonical link for it is here.

Posted to github@trafficserver.apache.org by GitBox <gi...@apache.org> on 2020/10/11 23:39:03 UTC

[GitHub] [trafficserver] masaori335 opened a new pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

masaori335 opened a new pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267


   # Issue
   We intermittently faced that clients get truncated response from ATS (v8.1.0). This happens only if ATS is in graceful shutdown mode, response takes time, and 2 GOAWAY frames are sent. We found that ATS races A). writing data to the socket and B). closing connection.
   
   A sequence of the issue is below.
   
   1). ATS starts sending response on the stream (id=1) and sends GOAWAY (stream_id=2^31-1) to indicate ATS is in graceful shutdown mode
   2). After 2 seconds, ATS sends another GOAWAY (stream_id=1) and `shutdown_state` becomes `HTTP2_SHUTDOWN_IN_PROGRESS`
   3). ATS try to complete sending the response and close the stream (ATS write a DATA frame with the END_STREAM flag to the buffer of VConnection)
   4). ATS schedule `HTTP2_SESSION_EVENT_FINI` event
   5). ATS try to write (SSL_write) data in the buffer but can't for some reasons ( - e.g., MBIO or socket buffer is full )
   6). Scheduled `HTTP2_SESSION_EVENT_FINI` closes VConnection regardless of the status
   
   # Fix
   This change removes step 4). in the above. The HTTP/2 connection becomes inactive and follows the close sequence in regular mode.
   
   # Future work
   In the graceful shutdown mode, ATS can close the inactive connections aggressively ( an idea is closing the connection from `VC_EVENT_WRITE_COMPLETE` event ). But it seems not necessary. I'd like to do it as a part of #6877 (Refactoring closing HTTP/2 session).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] zizhong commented on pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

zizhong commented on pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267#issuecomment-707399683


   @masaori335 Is it possible to add a test and make sure the connection is closed correctly?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] masaori335 merged pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

masaori335 merged pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] masaori335 commented on pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

masaori335 commented on pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267#issuecomment-706786547


   @zizhong Could you take a look? (somehow I can't assign you reviews)


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] maskit commented on pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

maskit commented on pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267#issuecomment-707475928

There was a confusion on what "active" means. This is my understanding after talking with Masaori.

With this change, PING frames keep connections established until active timeout kicks in, even under graceful shutdown. Inactivity timeout does not kick in because there is activity (PING) on the connection. This is what I pointed out above.

PING frames do not make a connection (or session) active state because PING does not initiate a transaction. Masaori thinks it would be nice if ATS could close a connection that do not have any ongoing transactions on `VC_WRITE_COMPLETE` event. Masaori is going to make another change for this in the future as part of #6877, but not now because he thinks it's not necessary. Correct me if I'm wrong, @masaori335 .

I think waiting for `VC_WRITE_COMPLETE` is probably a right direction, however, keeping connections established under graceful shutdown does not sound like a good idea. The number of established TCP connections may not be 0, although you can check an ATS metric that indicates the number of connections that have ongoing transactions.

Since truncated response is a pretty critical issue, taking this option for now may make sense, but you should understand what it can cause. I'm not sure when the additional change will be made, to be honest. There was no activity on #6877 for 3 months.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] masaori335 commented on pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

masaori335 commented on pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267#issuecomment-707383002


   To be clear, I don't mean the connection is closed by "follows the close sequence in regular mode."
   Same as regular mode, if a client keeps sending PING frame, the connection is open.
   But the connection is "inactive". ( PING frame doesn't make the connection active. )
   It's nice to close these lingering inactive connections, but I don't think it's necessary.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] shinrich commented on pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

shinrich commented on pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267#issuecomment-707371796


   Seems reasonable, but concerning if the default pings will keep things open.  Perhaps the graceful shutdown should also turn off the pings?


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] zwoop commented on pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

zwoop commented on pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267#issuecomment-709695987


   Cherry-picked to 8.1.x
   Cherry-picked to v9.0.x branch.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [trafficserver] maskit commented on pull request #7267: Fix truncated reponse on HTTP/2 graceful shutdown

Posted by GitBox <gi...@apache.org>.

maskit commented on pull request #7267:
URL: https://github.com/apache/trafficserver/pull/7267#issuecomment-706806309


   > The HTTP/2 connection becomes inactive and follows the close sequence in regular mode.
   
   This is not true. You can extend the life of the connection until `http2.active_timeout_in` at maximum by keep sending PING. If it was set to 0 (default value) the connection would not be closed.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org