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/08/24 16:49:48 UTC
[GitHub] [trafficserver] shinrich opened a new pull request #7134: Do not clear inactivity timeout on disable
shinrich opened a new pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134
Saw this when trying to override the inactivity timeouts for a tunnel_route. We could override the proxy.config.http.transaction_no_activity_timeout_in and proxy.config.http.transaction_no_activity_timeout_out to 600 seconds (from our default of 30 and 90). But our test stalled tunnel timed out at 300 seconds because our proxy.config.net.default_inactivity_timeout was set to 300, and the inactivity cop resets the timeout to proxy.config.net.default_inactivity_timeout if the next_inactivity_timeout is set to 0.
However, for the tunnel case, there is not an opportunity to reset the timeout to the original value. With this code change, the tunnel timeout works as expected without tripping up on the default_inactivity_timeout. This chance also does not seem to impact regular traffic.
I am not sure why we are clearing the inactivity timeout if the netvc is both read and write disabled. Looking through the git blames it seems that this logic has been present since the code moved into github in 2009.
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich merged pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-680187419
I just pushed a less extreme change. Rather than removing the inactivity logic completely from write_disable/read_disable, I reverted how the next_inactivity_timeout_at was cleared to 0 that variable directly rather than calling set_inactivity_timeout() which will also clear inactivity_timeout_in. The call to set_inactivity_timeout() was added in the NetEvent restructuring I think. Our ATS7 and the code @oknet references identifies the code that directly clears only net_inactivity_timeout_at.
I verified that this code works for my tunnel timeout case too. UnixNetVConnection::set_enabled is called well before inactivity_cop triggers, so the original inactivity_timeout is used to recompute next_inactivity_timeout_in.
I think UnixNetVConnection::set_default_inactivity_timeout should also be updated to not clear inactivity_timeout_in, but I will leave that to another 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] shinrich commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-684934054
Looking at the code a bit more, I think this default takeover of the inactivity timeout could occur for regular (non blind tunneled) connections as well. The code in question that sets the inactivity timeout in InactivityCop::check_inactivity was added somewhat recently by PR #6755. The code in question
```
// set a default inactivity timeout if one is not set
if (ne->next_inactivity_timeout_at == 0 && nh.config.default_inactivity_timeout > 0) {
Debug("inactivity_cop", "vc: %p inactivity timeout not set, setting a default of %d", ne,
nh.config.default_inactivity_timeout);
ne->set_default_inactivity_timeout(HRTIME_SECONDS(nh.config.default_inactivity_timeout));
NET_INCREMENT_DYN_STAT(default_inactivity_timeout_applied_stat);
}
```
Though if in fact next_inactivity_timeout_at and inactivity_timeout_in are cleared by write_disable/read_disable, I don't see a way to get those reset without the HttpSM level logic explicitly resetting it. And most of the read/write_disable seems to occur at the NetVC level.
@sudheerv since you last touched the default_inactivity_timeout logic, do you have any thoughts here?
----------------------------------------------------------------
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] oknet commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
oknet commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-690938262
@shinrich Did you try to comment the codes which set default inactivity timeout within `InactivityCop`? Is it useful for your tunnel timeout case?
----------------------------------------------------------------
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] oknet commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
oknet commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-690938262
----------------------------------------------------------------
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 edited a comment on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich edited a comment on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-680187419
I just pushed a less extreme change. Rather than removing the inactivity logic completely from write_disable/read_disable, I reverted how the next_inactivity_timeout_at was cleared to 0 that variable directly rather than calling set_inactivity_timeout() which will also clear inactivity_timeout_in. The call to set_inactivity_timeout() was added in the NetEvent restructuring I think. Our ATS7 and the code @oknet references identifies the code that directly clears only next_inactivity_timeout_at.
I verified that this code works for my tunnel timeout case too. UnixNetVConnection::set_enabled is called well before inactivity_cop triggers, so the original inactivity_timeout is used to recompute next_inactivity_timeout_in.
I think UnixNetVConnection::set_default_inactivity_timeout should also be updated to not clear inactivity_timeout_in, but I will leave that to another 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] oknet commented on pull request #7134: Do not clear inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
oknet commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-679708907
@shinrich
`git blame 6da43e2b75efe23524bf14394a07bca3fc8bf957 P_UnixNet.h` shows that this logic has been present from the beginning.
To enable `InactivityCop`, we do not define `INACTIVITY_TIMEOUT`.
```
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 450) read_disable(NetHandler * nh, UnixNetVConnection * vc)
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 451) {
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 452) #ifdef INACTIVITY_TIMEOUT
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 453) if (vc->inactivity_timeout) {
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 454) if (!vc->write.enabled) {
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 455) vc->inactivity_timeout->cancel_action();
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 456) vc->inactivity_timeout = NULL;
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 457) }
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 458) }
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 459) #else
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 460) if (vc->next_inactivity_timeout_at)
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 461) if (!vc->write.enabled)
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 462) vc->next_inactivity_timeout_at = 0;
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 463) #endif
01a15380d5 (John Bradley Plevyak 2009-12-09 23:01:29 +0000 464) vc->read.enabled = 0;
01a15380d5 (John Bradley Plevyak 2009-12-09 23:01:29 +0000 465) nh->read_ready_list.remove(vc);
a5345da86f (George Paul 2010-02-24 18:48:42 +0000 466) vc->ep.modify(-EVENTIO_READ);
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 467) }
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 468)
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 469) static inline void
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 470) write_disable(NetHandler * nh, UnixNetVConnection * vc)
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 471) {
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 472) #ifdef INACTIVITY_TIMEOUT
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 473) if (vc->inactivity_timeout) {
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 474) if (!vc->read.enabled) {
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 475) vc->inactivity_timeout->cancel_action();
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 476) vc->inactivity_timeout = NULL;
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 477) }
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 478) }
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 479) #else
468058a255 (Mladen Turk 2010-05-13 07:09:39 +0000 480) if (vc->next_inactivity_timeout_at)
01a15380d5 (John Bradley Plevyak 2009-12-09 23:01:29 +0000 481) if (!vc->read.enabled)
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 482) vc->next_inactivity_timeout_at = 0;
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 483) #endif
01a15380d5 (John Bradley Plevyak 2009-12-09 23:01:29 +0000 484) vc->write.enabled = 0;
01a15380d5 (John Bradley Plevyak 2009-12-09 23:01:29 +0000 485) nh->write_ready_list.remove(vc);
a5345da86f (George Paul 2010-02-24 18:48:42 +0000 486) vc->ep.modify(-EVENTIO_WRITE);
a1651345d1 (Andrew Hsu 2009-10-29 23:01:48 +0000 487) }
```
----------------------------------------------------------------
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 #7134: Do not clear inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-679286710
[approve ci docs]
----------------------------------------------------------------
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 edited a comment on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich edited a comment on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-680187419
I just pushed a less extreme change. Rather than removing the inactivity logic completely from write_disable/read_disable, I reverted how the next_inactivity_timeout_at was cleared to 0 that variable directly rather than calling set_inactivity_timeout() which will also clear inactivity_timeout_in. The call to set_inactivity_timeout() was added in the NetEvent restructuring I think. Our ATS7 and the code @oknet references identifies the code that directly clears only next_inactivity_timeout_at.
I verified that this code works for my tunnel timeout case too. UnixNetVConnection::set_enabled is called well before inactivity_cop triggers, so the original inactivity_timeout_in is used to recompute next_inactivity_timeout_in.
I think UnixNetVConnection::set_default_inactivity_timeout should also be updated to not clear inactivity_timeout_in, but I will leave that to another 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] sudheerv commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
sudheerv commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-691357987
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-680187512
[approve ci docs]
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-683969467
@oknet I think having the netactivity revert to the original inactivity timeout is desirable behavior, isn't it? Presumably it is the netactivity() that is fixing my tunnel timeout problem.
----------------------------------------------------------------
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 pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
sudheerv commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-691357987
@shinrich The change in your PR looks reasonable to me. This is how the code looked in 6.2.x as well.
As you said, Inactivity cop's take over of inactive timeout when no timeout is set is by design, and we recently tried to analyze why we'd even have situations where such guard/catch-all is required. I think we sort of found most (probably all such) cases, one example was a missing SSL Handshake timeout and confirmed by adding explicit metrics to see how often is the Inactivity Cop tries to take over the inactive timeout with default timeout, and of those, how often it actually _applies_ the default timeout. We found that the former is significant but the latter is 0 in prod after fixing some of the gaps (example below from a prod box).
{code}
[svinukon@ltx1-app42432 i001]$ sudo ./chroot-exec traffic_ctl metric match default
proxy.process.net.default_inactivity_timeout_applied 31593
proxy.process.net.default_inactivity_timeout_count 0
{code}
Also, I think the logic in inactivity cop taking over the VC's inactivity by setting default timeout is not actually introduced by PR 6755. PR 6755 merely did some refactoring to be able to detect default vs non-default inactivity timeout being set. The takeover logic has always been there AFAIR (at least, from 5.x?). I think @bryancall added this guard because we were seeing lots of connection leaks without that back in 5.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] oknet commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
oknet commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-684819524
@shinrich The `Inactivity Timeout` is used to set a timeout for read/write transaction. The `Inactivity Timeout` should be disabled if both read and write are disabled.
The `next_inactivity_timeout == 0` means inactivity timeout disabled, but the `InactivityCop` consider that it is default inactivity timeout.
To resolve your issue, IMO, it is better to clarify how to set default inactivity timeout within `InactivityCop`.
----------------------------------------------------------------
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] oknet commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
oknet commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-690938262
@shinrich Did you try to comment the codes which set default inactivity timeout within `InactivityCop`? Is it useful for your tunnel timeout case?
----------------------------------------------------------------
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 #7134: Do not clear inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-680104630
Yes, this logic has been present since the code was placed into gihub in 2009. I would assume that the timeout for the tunneling logic has never really worked appropriately either. The default value of proxy.config.net.default_inactivity_timeout is quite high 86400, so it normally wouldn't cause a conflict as long as at least one of the netVC retains a valid timeout.
But stepping back, why do we do this clearing of the timeout when a netvc is both read and write disabled? Is this something that used to make sense but does no longer?
----------------------------------------------------------------
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 pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
sudheerv commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-691357987
@shinrich The change in your PR looks reasonable to me. This is how the code looked in 6.2.x as well.
As you said, Inactivity cop's take over of inactive timeout when no timeout is set is by design, and we recently tried to analyze why we'd even have situations where such guard/catch-all is required. I think we sort of found most (probably all such) cases, one example was a missing SSL Handshake timeout and confirmed by adding explicit metrics to see how often is the Inactivity Cop tries to take over the inactive timeout with default timeout, and of those, how often it actually _applies_ the default timeout. We found that the former is significant but the latter is 0 in prod after fixing some of the gaps (example below from a prod box).
{code}
[svinukon@ltx1-app42432 i001]$ sudo ./chroot-exec traffic_ctl metric match default
proxy.process.net.default_inactivity_timeout_applied 31593
proxy.process.net.default_inactivity_timeout_count 0
{code}
Also, I think the logic in inactivity cop taking over the VC's inactivity by setting default timeout is not actually introduced by PR 6755. PR 6755 merely did some refactoring to be able to detect default vs non-default inactivity timeout being set. The takeover logic has always been there AFAIR (at least, from 5.x?). I think @bryancall added this guard because we were seeing lots of connection leaks without that back in 5.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] sudheerv commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
sudheerv commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-691357987
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-691127185
----------------------------------------------------------------
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] oknet commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
oknet commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-683252794
@shinrich
The `InactivityCop` will set default inactivity timeout if `next_inactivity_timeout_at` was set to `0`
https://github.com/apache/trafficserver/blob/c8c9540b04b08d04bd44f6b8c7aa5d75d4a9e1a8/iocore/net/UnixNet.cc#L72-L78
The `next_inactivity_timeout_at` will be reset with `net_activity()` if you do not clear `inactivity_timeout_in`.
https://github.com/apache/trafficserver/blob/c8c9540b04b08d04bd44f6b8c7aa5d75d4a9e1a8/iocore/net/UnixNetVConnection.cc#L63-L73
----------------------------------------------------------------
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] oknet commented on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
oknet commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-690938262
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-691127185
I didn't try that. That would probably solve my specific tunnel case since only one end is being fully deactivated and having its inactivity timeout cleared, so the inactivity timeout of the other netvc would prevail. But that is probably not a good solution, since that would leave connections with no inactivity timeout set, which is the whole purpose of the default inactivity timeout setting.
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-691127185
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-684925701
In the tunnel case, it seems that disabling both write and read on the netvc is a transient operation. With the current code, we need to explicitly reset the inactivity timeout when the netvc is reenabled. I'll. look around about how to do that at the HttpSM level.
I assume that netactivity() won't trigger until the netvc has been reenabled for read or write. If that is the case, then having the netactivity reset the previously set inactivity timeout seems reasonable.
----------------------------------------------------------------
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 #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
zwoop commented on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-692843749
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] shinrich edited a comment on pull request #7134: Do not lose original inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich edited a comment on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-684925701
In the tunnel case, it seems that disabling both write and read on the netvc is a transient operation. With the current code, we need to explicitly reset the inactivity timeout when the netvc is reenabled. I'll look around about how to do that at the HttpSM level.
I assume that net_activity() won't trigger until the netvc has been reenabled for read or write. If that is the case, then having the net_activity reset the previously set inactivity timeout seems reasonable.
I just looked through the code, and indeed it appears that net_activity will only be called if the netvc is in fact enabled, so having net_activity() reset the previously set inactivity timeout seems like a good thing.
----------------------------------------------------------------
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 edited a comment on pull request #7134: Do not clear inactivity timeout on disable
Posted by GitBox <gi...@apache.org>.
shinrich edited a comment on pull request #7134:
URL: https://github.com/apache/trafficserver/pull/7134#issuecomment-680104630
Yes, this logic has been present since the code was placed into gihub in 2009. I would assume that the timeout for the tunneling logic has never really worked appropriately either. The default value of proxy.config.net.default_inactivity_timeout is quite high 86400, so it normally wouldn't cause a conflict as long as at least one of the netVC retains a valid timeout. We somewhat recently started running with proxy.config.net.default_inactivity_timeout set to 500.
But stepping back, why do we do this clearing of the timeout when a netvc is both read and write disabled? Is this something that used to make sense but does no longer?
----------------------------------------------------------------
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