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 2021/03/16 04:36:07 UTC
[GitHub] [trafficserver] masaori335 opened a new pull request #7605: Disable read from client when HttpTunnelConsumer for client is set up
masaori335 opened a new pull request #7605:
URL: https://github.com/apache/trafficserver/pull/7605
Fix #7603.
The read from the client is enabled to watch the client abort, but this makes ATS signals EOS event to the HttpSM and goes `HttpSM::state_watch_for_client_abort()` handler.
https://github.com/apache/trafficserver/blob/7af385faf5bfb644ae200682fb156d1433437683/proxy/http/HttpSM.cc#L899-L900
When HttpSM sets up a HttpTunnelConsumer for a client, we can disable read to avoid the issue because there is nothing to read from the client. The EOS is handled by write op and signaled to HttpTunnel 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 commented on pull request #7605: Disable read from client when HttpTunnelConsumer for client is set up
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7605:
URL: https://github.com/apache/trafficserver/pull/7605#issuecomment-809821254
It turns out #7641 is a better approach because of fewer side effects.
--
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 #7605: Disable read from client when HttpTunnelConsumer for client is set up
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7605:
URL: https://github.com/apache/trafficserver/pull/7605#issuecomment-803717642
> Is there a reason we don't handle the EOS appropriately?
The HttpSM is in the `state_watch_for_client_abort` state and it kills HttpTunnel on EOS if `ua_txn->allow_half_open()` returns `false`. Checking what HttpTunnel is doing might be another approach, but I'm not sure it's better than this.
https://github.com/apache/trafficserver/blob/42db8bb284d63e3f019e2d9d986e4d34f750f69a/proxy/http/HttpSM.cc#L975-L993
> Do we also disable reads from the client in the HTTP/2 case? If so, then how does that work for bidirectional communication with HTTP/2?
With HTTP/2, this change disables the read of Http2Stream (just resets `Http2Stream::read_vio` which is used for receiving a request from the client on the stream).
The read op of HTTP/2 *connection* is still available.
--
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 closed pull request #7605: Disable read from client when HttpTunnelConsumer for client is set up
Posted by GitBox <gi...@apache.org>.
masaori335 closed pull request #7605:
URL: https://github.com/apache/trafficserver/pull/7605
--
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 #7605: Disable read from client when HttpTunnelConsumer for client is set up
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7605:
URL: https://github.com/apache/trafficserver/pull/7605#issuecomment-809006633
We tried this patch (on top of 8.1.2) and observed
1. background fill is working well
2. fewer client aborts
3. +2K active client connections.
3. might be a side effect of disabling read. I'll try another approach.
--
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] bryancall commented on pull request #7605: Disable read from client when HttpTunnelConsumer for client is set up
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7605:
URL: https://github.com/apache/trafficserver/pull/7605#issuecomment-802983137
Is there a reason we don't handle the EOS appropriately? Do we also disable reads from the client in the HTTP/2 case? If so, then how does that work for bidirectional communication with HTTP/2?
--
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 edited a comment on pull request #7605: Disable read from client when HttpTunnelConsumer for client is set up
Posted by GitBox <gi...@apache.org>.
masaori335 edited a comment on pull request #7605:
URL: https://github.com/apache/trafficserver/pull/7605#issuecomment-809006633
We tried this patch (on top of 8.1.2) and observed
1. background fill is working well
2. fewer client aborts
3. +2K active client connections
The 3. might be a side effect of disabling read. I'll try another approach.
--
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