[
https://issues.apache.org/jira/browse/TS-3487?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14496051#comment-14496051
]
Feifei Cai commented on TS-3487:
--------------------------------
Well, let’s go through some code of inactivity timeout mechanism. Assuming ATS
is compiled in default way - using InactivityCop with a timeout timestamp, as
[~bcall] explained.
https://github.com/apache/trafficserver/blob/master/iocore/net/UnixNetVConnection.cc
{code}
UnixNetVConnection::reenable/reenable_re(VIO *vio) {
…
UnixNetVConnection::set_enabled(VIO *vio) {
…
if (!next_inactivity_timeout_at && inactivity_timeout_in) {
next_inactivity_timeout_at = ink_get_hrtime() + inactivity_timeout_in;
}
…
}
…
}
{code}
https://github.com/apache/trafficserver/blob/master/iocore/net/P_UnixNet.h
{code}
read_disable/write_disable(NetHandler *nh, UnixNetVConnection *vc) {
…
vc->next_inactivity_timeout_at = 0;
…
}
{code}
As it shows, the timer's start operation is binding to
UnixNetVConnection::set_enabled in UnixNetVConnection::reenable/reenable_re,
the stop operation is binding to read_disable/write_disable, and the reset
operation is binding to net_activity in read_from_net/write_to_net. So, we do
not need to care about the implementation details in net IO. What we need do is
to call set_inactivity_timeout at appropriate point and
cancel_inactivity_timeout when necessary.
For GET requests, the inactive timer on the client vc is cancelled right before
the origin connection is opened:
{code}
HttpSM::set_next_state() {
...
case HttpTransact::SM_ACTION_ORIGIN_SERVER_OPEN:
ua_session->get_netvc()->cancel_inactivity_timeout();
...
}
{code}
which is inside call_transact_and_set_next_state():
{code}
HttpSM::state_read_client_request_header () {
...
case PARSE_DONE:
ua_entry->read_vio->nbytes = ua_entry->read_vio->ndone;
call_transact_and_set_next_state(HttpTransact::ModifyRequest);
return 0;
}
{code}
Be aware that before entering
call_transact_and_set_next_state(HttpTransact::ModifyRequest), ATS is before
TS_HTTP_READ_REQUEST_HDR_HOOK hook point; after exit
call_transact_and_set_next_state(HttpTransact::ModifyRequest), ATS is before
TS_HTTP_SEND_REQUEST_HDR_HOOK hook point. In this time window, ATS is in HttpSM
functions, and would not switch to IO read/write on client vc. It’s running
inside read_signal_and_update, which is in NetHandler::mainNetEvent, and would
not switch to InactivityCop::check_inactivity. So, the inactivity timeout event
would not happen in this time window. There are several hook points which we
can override the configurations, however, check_inactivity does not take effect
here, even ATS is blocked in it (which should not happen in normal case, here
ATS is not doing I/O with client), so we do not need to set_inactivity_timeout
with new value . What’s more, I think we can cancel_inactivity_timeout some
earlier, just after finish reading and parsing client request headers and
disable further I/O on client vc. Once we stop client I/O, we do not need
inactivity timeout control on client vc anymore. For GET requests, we only need
to set it again when sending back response to client. The current
implementation is just okay.
For POST requests, I agree we should set_inactivity_timeout some earlier. How
about let’s go through GET requests first, then I update the patch for POST
requests, because in my understanding, we do not need to change anything for
GET requests logic.
> cannot override proxy.config.http.transaction_no_activity_timeout_in per
> remap rule for POST methold
> ----------------------------------------------------------------------------------------------------
>
> Key: TS-3487
> URL: https://issues.apache.org/jira/browse/TS-3487
> Project: Traffic Server
> Issue Type: Bug
> Components: HTTP
> Affects Versions: 5.2.1
> Reporter: Feifei Cai
> Assignee: Bryan Call
> Labels: review
> Fix For: 6.0.0
>
> Attachments: TS-3487.diff
>
>
> The configuration and test are as follows:
> remap.config:
> {noformat}
> map /test1 http://httpbin.org
> map /test2 http://httpbin.org @plugin=conf_remap.so
> @pparam=proxy.config.http.transaction_no_activity_timeout_in=15
> {noformat}
> records.config:
> {noformat}
> CONFIG proxy.config.http.transaction_no_activity_timeout_in INT 5
> CONFIG proxy.config.diags.debug.enabled INT 1
> CONFIG proxy.config.diags.debug.tags STRING
> http_cs|http_ss|inactivity.*|socket
> {noformat}
> {code:title=test.py}
> import time
> import logging
> import socket
> log = logging.getLogger(__name__)
> logging.basicConfig(level=logging.INFO)
> import SocketServer
> url1 = 'POST /test1/post HTTP/1.1\r\n'
> url2 = 'POST /test2/post HTTP/1.1\r\n'
> header1 = 'Host: 127.0.0.1\r\n'
> # last header need additional '\r\n'
> header2 = 'Content-Length: 10\r\n\r\n'
> body1 = '12345'
> body2 = '67890'
> def get_socket():
> s = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
> s.connect(('127.0.0.1', 8080))
> return s
> def test_global_config():
> s = get_socket()
> log.info('start test global config...')
> try:
> # before remap
> s.send(url1)
> time.sleep(2) # < global config
> s.send(header1)
> time.sleep(3) # < global config
> s.send(header2)
> # after remap
> time.sleep(2) # < global config
> s.send(body1)
> time.sleep(4) # < global config
> s.send(body2)
> log.info('test global config: pass!')
> except IOError:
> log.info('test global config: fail!')
> response = s.recv(4096)
> print response
> def test_per_remap_config():
> s = get_socket()
> log.info('start test per remap config...')
> try:
> # before remap
> s.send(url2)
> time.sleep(2) # < global config
> s.send(header1)
> time.sleep(3) # < global config
> s.send(header2)
> # after remap
> time.sleep(11) # < per remap config
> s.send(body1)
> time.sleep(13) # < per remap config
> s.send(body2)
> log.info('test per remap config: pass!')
> except IOError:
> log.info('test per remap config: fail!')
> response = s.recv(4096)
> print response
> if __name__ == '__main__':
> test_global_config()
> test_per_remap_config()
> {code}
> {{test_global_config()}} would pass, but {{test_per_remap_config()}} fails.
> {{proxy.config.http.transaction_no_activity_timeout_in}} in per remap rule
> does not works.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
