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:
[email protected]
