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/11/03 13:38:11 UTC
[GitHub] [trafficserver] shinrich opened a new pull request #7309: Disable client inactivity timeout while server is processing POST request
shinrich opened a new pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309
This PR makes changes to HttpSM.cc to disable the client inactivity timeout at the end of the POST tunnel, and it reenables the client inactivity timeout upon receiving a server response header.
It also adds an autest to exercise different cases of GET and POST against a origin that will delay for 4 seconds before returning a response. In the autest the client inactivity timeout is set to 2. When run against the current origin/master, the GET cases will succeed and the POST cases will fail. With this code change, all cases should succeed.
This closes #7308
----------------------------------------------------------------
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] ywkaras commented on a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
ywkaras commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r516204499
##########
File path: tests/gold_tests/timeout/inactive_client_post_timeout.test.py
##########
@@ -0,0 +1,108 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Test.Summary = 'Testing ATS client inactivity timeout'
+
+Test.SkipUnless(
+ Condition.HasCurlFeature('http2')
+)
+
+ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
+
+Test.ContinueOnFail = True
+
+Test.GetTcpPort("upstream_port1")
+Test.GetTcpPort("upstream_port2")
+Test.GetTcpPort("upstream_port3")
+Test.GetTcpPort("upstream_port4")
+Test.GetTcpPort("upstream_port5")
+Test.GetTcpPort("upstream_port6")
+
+ts.addSSLfile("../tls/ssl/server.pem")
+ts.addSSLfile("../tls/ssl/server.key")
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.url_remap.remap_required': 1,
+ 'proxy.config.http.transaction_no_activity_timeout_in': 2,
+})
+
+ts.Disk.remap_config.AddLines([
+ 'map /case1 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port1),
+ 'map /case2 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port2),
+ 'map /case3 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port3),
+ 'map /case4 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port4),
+ 'map /case5 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port5),
+ 'map /case6 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port6),
+])
+
+ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+)
+
+# Using netcat with explicit delays instead of the delay option with microserver because it appears
+# that microserver will not delay responses to POST requests
+# The delay-inactive-server.sh will deplay for 4 seconds before returning a response. This is more
+# than the 2 second proxy.config.http.transaction_no_activity_timeout_in
+# These tests exercise that the client inactivity timeout is disabled after the request and post body
+# are sent. So a slow to respond server will not trigger the client inactivity timeout.
+
+tr4 = Test.AddTestRun("tr")
+tr4.Processes.Default.StartBefore(ts, ready=When.PortOpen(ts.Variables.ssl_port))
+tr4.Setup.Copy('delay-inactive-server.sh')
+tr4.Setup.Copy('case-inactive4.sh')
+tr4.Processes.Default.ReturnCode = 0
+tr4.Processes.Default.Command = 'sh -x ./case-inactive4.sh {0} {1} case4'.format(
Review comment:
Nitpicks: Why use the Bourne shell instead of bash? (The differences look irrelevant here but seems like a potential gotcha if the scripts get more complicated in the future.). Also, the caseN parameters seem redundant.
----------------------------------------------------------------
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 #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-744543765
I still need a non-Verizon Media review.
----------------------------------------------------------------
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 a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r575885233
##########
File path: tests/gold_tests/timeout/inactive_client_post_timeout.test.py
##########
@@ -0,0 +1,108 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Test.Summary = 'Testing ATS client inactivity timeout'
+
+Test.SkipUnless(
+ Condition.HasCurlFeature('http2')
+)
+
+ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
+
+Test.ContinueOnFail = True
+
+Test.GetTcpPort("upstream_port1")
+Test.GetTcpPort("upstream_port2")
+Test.GetTcpPort("upstream_port3")
+Test.GetTcpPort("upstream_port4")
+Test.GetTcpPort("upstream_port5")
+Test.GetTcpPort("upstream_port6")
+
+ts.addSSLfile("../tls/ssl/server.pem")
+ts.addSSLfile("../tls/ssl/server.key")
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.url_remap.remap_required': 1,
+ 'proxy.config.http.transaction_no_activity_timeout_in': 2,
+})
+
+ts.Disk.remap_config.AddLines([
+ 'map /case1 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port1),
+ 'map /case2 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port2),
+ 'map /case3 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port3),
+ 'map /case4 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port4),
+ 'map /case5 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port5),
+ 'map /case6 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port6),
+])
+
+ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+)
+
+# Using netcat with explicit delays instead of the delay option with microserver because it appears
+# that microserver will not delay responses to POST requests
Review comment:
FYI, https://github.com/postmanlabs/httpbin/commit/36a3df2f822935634697c415309967a37af614c3 added "delay on httpbin" support, but it has not been released yet. It seems like the project is stalled.
We need to tweak our microserver or proxy verifier for cleanup.
----------------------------------------------------------------
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 #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-777837517
Good to know about the httpbin delay. I'll clean that up in a separate PR.
----------------------------------------------------------------
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 a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r571808935
##########
File path: tests/gold_tests/timeout/inactive_client_post_timeout.test.py
##########
@@ -0,0 +1,108 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Test.Summary = 'Testing ATS client inactivity timeout'
+
+Test.SkipUnless(
+ Condition.HasCurlFeature('http2')
+)
+
+ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
+
+Test.ContinueOnFail = True
+
+Test.GetTcpPort("upstream_port1")
+Test.GetTcpPort("upstream_port2")
+Test.GetTcpPort("upstream_port3")
+Test.GetTcpPort("upstream_port4")
+Test.GetTcpPort("upstream_port5")
+Test.GetTcpPort("upstream_port6")
+
+ts.addSSLfile("../tls/ssl/server.pem")
+ts.addSSLfile("../tls/ssl/server.key")
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.url_remap.remap_required': 1,
+ 'proxy.config.http.transaction_no_activity_timeout_in': 2,
+})
+
+ts.Disk.remap_config.AddLines([
+ 'map /case1 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port1),
+ 'map /case2 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port2),
+ 'map /case3 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port3),
+ 'map /case4 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port4),
+ 'map /case5 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port5),
+ 'map /case6 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port6),
+])
+
+ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+)
+
+# Using netcat with explicit delays instead of the delay option with microserver because it appears
+# that microserver will not delay responses to POST requests
Review comment:
IIRC, httpbin supports delay on POST requests.
----------------------------------------------------------------
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 #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-721887503
Confirmed that this change fixed the large post upload timeout problem we were seeing in production.
----------------------------------------------------------------
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 a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
masaori335 commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r571814762
##########
File path: proxy/http/HttpSM.cc
##########
@@ -2008,28 +2008,13 @@ HttpSM::state_read_server_response_header(int event, void *data)
// Now that we know that we have all of the origin server
// response headers, we can reset the client inactivity
- // timeout. This is unlikely to cause a recurrence of
- // old bug because there will be no more retries now that
- // the connection has been established. It is possible
- // however. We do not need to reset the inactivity timeout
- // if the request contains a body (noted by the
- // request_content_length field) because it was never
- // canceled.
- //
-
+ // timeout.
// we now reset the client inactivity timeout only
// when we are ready to send the response headers. In the
// case of transform plugin, this is after the transform
// outputs the 1st byte, which can take a long time if the
// plugin buffers the whole response.
- // Also, if the request contains a body, we cancel the timeout
- // when we read the 1st byte of the origin server response.
- /*
- if (ua_txn && !t_state.hdr_info.request_content_length) {
- ua_txn->get_netvc()->set_inactivity_timeout(HRTIME_SECONDS(
- HttpConfig::m_master.accept_no_activity_timeout));
- }
- */
+ ua_txn->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_no_activity_timeout_in));
Review comment:
It seems like the inactive timeout is already canceled before L1943 cancels it. Probably, we can get rid of it.
----------------------------------------------------------------
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 merged pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
shinrich merged pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309
----------------------------------------------------------------
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 #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-783523023
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] zwoop commented on pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-778275020
I assume we want this on 9.1.x as well, since it's marked for 9.0.x.
----------------------------------------------------------------
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 #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-773637149
@masaori335 is going to take a look at it.
----------------------------------------------------------------
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 a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r517539862
##########
File path: tests/gold_tests/timeout/inactive_client_post_timeout.test.py
##########
@@ -0,0 +1,108 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Test.Summary = 'Testing ATS client inactivity timeout'
+
+Test.SkipUnless(
+ Condition.HasCurlFeature('http2')
+)
+
+ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
+
+Test.ContinueOnFail = True
+
+Test.GetTcpPort("upstream_port1")
+Test.GetTcpPort("upstream_port2")
+Test.GetTcpPort("upstream_port3")
+Test.GetTcpPort("upstream_port4")
+Test.GetTcpPort("upstream_port5")
+Test.GetTcpPort("upstream_port6")
+
+ts.addSSLfile("../tls/ssl/server.pem")
+ts.addSSLfile("../tls/ssl/server.key")
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.url_remap.remap_required': 1,
+ 'proxy.config.http.transaction_no_activity_timeout_in': 2,
+})
+
+ts.Disk.remap_config.AddLines([
+ 'map /case1 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port1),
+ 'map /case2 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port2),
+ 'map /case3 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port3),
+ 'map /case4 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port4),
+ 'map /case5 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port5),
+ 'map /case6 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port6),
+])
+
+ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+)
+
+# Using netcat with explicit delays instead of the delay option with microserver because it appears
+# that microserver will not delay responses to POST requests
+# The delay-inactive-server.sh will deplay for 4 seconds before returning a response. This is more
+# than the 2 second proxy.config.http.transaction_no_activity_timeout_in
+# These tests exercise that the client inactivity timeout is disabled after the request and post body
+# are sent. So a slow to respond server will not trigger the client inactivity timeout.
+
+tr4 = Test.AddTestRun("tr")
+tr4.Processes.Default.StartBefore(ts, ready=When.PortOpen(ts.Variables.ssl_port))
+tr4.Setup.Copy('delay-inactive-server.sh')
+tr4.Setup.Copy('case-inactive4.sh')
+tr4.Processes.Default.ReturnCode = 0
+tr4.Processes.Default.Command = 'sh -x ./case-inactive4.sh {0} {1} case4'.format(
Review comment:
As it turns out. On Centos 7 it is bash
```
[shinrich@atslab02 http2]$ ls -l /usr/bin/sh
lrwxrwxrwx. 1 root root 4 Jun 13 2019 /usr/bin/sh -> bash
```
----------------------------------------------------------------
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] sudheerv commented on a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
sudheerv commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r518222672
##########
File path: proxy/http/HttpSM.cc
##########
@@ -2008,28 +2008,13 @@ HttpSM::state_read_server_response_header(int event, void *data)
// Now that we know that we have all of the origin server
// response headers, we can reset the client inactivity
- // timeout. This is unlikely to cause a recurrence of
- // old bug because there will be no more retries now that
- // the connection has been established. It is possible
- // however. We do not need to reset the inactivity timeout
- // if the request contains a body (noted by the
- // request_content_length field) because it was never
- // canceled.
- //
-
+ // timeout.
// we now reset the client inactivity timeout only
// when we are ready to send the response headers. In the
// case of transform plugin, this is after the transform
// outputs the 1st byte, which can take a long time if the
// plugin buffers the whole response.
- // Also, if the request contains a body, we cancel the timeout
- // when we read the 1st byte of the origin server response.
- /*
- if (ua_txn && !t_state.hdr_info.request_content_length) {
- ua_txn->get_netvc()->set_inactivity_timeout(HRTIME_SECONDS(
- HttpConfig::m_master.accept_no_activity_timeout));
- }
- */
+ ua_txn->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_no_activity_timeout_in));
Review comment:
Hmm, inactivity timeout for the ua_txn that has a non-zero CL is actually being cancelled in line #1943 above. Seems like we are re-enabling it and then canceling in post tunnel? I'm curious to know why we need to re-enable the UA inactivity timeout here?
FWIW, we are not seeing this specific issue with POSTs, but that may be because our Edge Proxies that are ATS6 always send a CL (because they buffer the request body) when talking to our DC Proxies that are ATS9. Are your issues for POSTs that are chunked encoding and not having a CL header?
----------------------------------------------------------------
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 #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
masaori335 commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-778888392
[approve ci autest]
----------------------------------------------------------------
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 #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-778275286
Cherry-picked to v9.1.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] bryancall commented on pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
bryancall commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-773637149
@masaori335 is going to take a look at it.
----------------------------------------------------------------
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 a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
shinrich commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r530705799
##########
File path: proxy/http/HttpSM.cc
##########
@@ -2008,28 +2008,13 @@ HttpSM::state_read_server_response_header(int event, void *data)
// Now that we know that we have all of the origin server
// response headers, we can reset the client inactivity
- // timeout. This is unlikely to cause a recurrence of
- // old bug because there will be no more retries now that
- // the connection has been established. It is possible
- // however. We do not need to reset the inactivity timeout
- // if the request contains a body (noted by the
- // request_content_length field) because it was never
- // canceled.
- //
-
+ // timeout.
// we now reset the client inactivity timeout only
// when we are ready to send the response headers. In the
// case of transform plugin, this is after the transform
// outputs the 1st byte, which can take a long time if the
// plugin buffers the whole response.
- // Also, if the request contains a body, we cancel the timeout
- // when we read the 1st byte of the origin server response.
- /*
- if (ua_txn && !t_state.hdr_info.request_content_length) {
- ua_txn->get_netvc()->set_inactivity_timeout(HRTIME_SECONDS(
- HttpConfig::m_master.accept_no_activity_timeout));
- }
- */
+ ua_txn->set_inactivity_timeout(HRTIME_SECONDS(t_state.txn_conf->transaction_no_activity_timeout_in));
Review comment:
The problem arises for us when the server delays before sending any data at all. So the HttpSM::state_read_server_response_header doesn't called until the origin client side timeout already triggered.
I would argue that the cancel timeout on line #1943 doesn't do anything.
----------------------------------------------------------------
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] djcarlin commented on pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
djcarlin commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-723298000
I've tried this out in production and it fixes problem, hasn't impacted other production traffic.
As a workaround, increasing proxy.config.http.transaction_no_activity_timeout_in increased the timeout for waiting on origin response during POST. We're using this until ATS build with this patch is rolled out.
----------------------------------------------------------------
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 removed a comment on pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
masaori335 removed a comment on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-778888392
[approve ci autest]
----------------------------------------------------------------
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] bneradt commented on a change in pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
bneradt commented on a change in pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#discussion_r578833805
##########
File path: tests/gold_tests/timeout/inactive_client_post_timeout.test.py
##########
@@ -0,0 +1,108 @@
+'''
+'''
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+Test.Summary = 'Testing ATS client inactivity timeout'
+
+Test.SkipUnless(
+ Condition.HasCurlFeature('http2')
+)
+
+ts = Test.MakeATSProcess("ts", select_ports=True, enable_tls=True)
+
+Test.ContinueOnFail = True
+
+Test.GetTcpPort("upstream_port1")
+Test.GetTcpPort("upstream_port2")
+Test.GetTcpPort("upstream_port3")
+Test.GetTcpPort("upstream_port4")
+Test.GetTcpPort("upstream_port5")
+Test.GetTcpPort("upstream_port6")
+
+ts.addSSLfile("../tls/ssl/server.pem")
+ts.addSSLfile("../tls/ssl/server.key")
+
+ts.Disk.records_config.update({
+ 'proxy.config.diags.debug.enabled': 1,
+ 'proxy.config.diags.debug.tags': 'http',
+ 'proxy.config.ssl.server.cert.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.ssl.server.private_key.path': '{0}'.format(ts.Variables.SSLDir),
+ 'proxy.config.url_remap.remap_required': 1,
+ 'proxy.config.http.transaction_no_activity_timeout_in': 2,
+})
+
+ts.Disk.remap_config.AddLines([
+ 'map /case1 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port1),
+ 'map /case2 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port2),
+ 'map /case3 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port3),
+ 'map /case4 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port4),
+ 'map /case5 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port5),
+ 'map /case6 http://127.0.0.1:{0}'.format(Test.Variables.upstream_port6),
+])
+
+ts.Disk.ssl_multicert_config.AddLine(
+ 'dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key'
+)
+
+# Using netcat with explicit delays instead of the delay option with microserver because it appears
+# that microserver will not delay responses to POST requests
Review comment:
I added server-side delay to Proxy Verifier. This PR (inactive_client_post_timeout.test.py) is now converted to use Proxy Verifier via this delay feature:
https://github.com/apache/trafficserver/pull/7535
----------------------------------------------------------------
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] bneradt commented on pull request #7309: Disable client inactivity timeout while server is processing POST request
Posted by GitBox <gi...@apache.org>.
bneradt commented on pull request #7309:
URL: https://github.com/apache/trafficserver/pull/7309#issuecomment-780052232
> FYI, postmanlabs/httpbin@36a3df2 added "delay on httpbin" support, but it has not been released yet. It seems like the project is stalled.
> We need to tweak our microserver or proxy verifier for cleanup.
Yes, I've considered this as well. Proxy Verifier has client-side delay but not server-side delay. It shouldn't be hard to add it to the server-side. I plan to work on that soon.
----------------------------------------------------------------
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