sudheerv commented on pull request #7226:
URL: https://github.com/apache/trafficserver/pull/7226#issuecomment-700858725
The change does sound functionally correct, although, I wonder what happens
when active timeout isn't configured (ie set to "0"). It'd leave the VC
unmonitored because, there's neither the "active", nor the "inactive" timeout
on it now.
That said, we also generally are trying to remove the default inactive
timeout mechanism entirely (or set it to a super high value - e.g hours or even
days as opposed to the current 5 min value that we are forced to set in
production), which means we have to make sure we find all corner cases where
there's no specific timeout is monitoring a VC. I think the DNS lookup use case
you raised is a nice example of such scenario - as long as we enforce a timeout
on the SM for DNS lookup and clean up the VC associated on a timeout, we should
be good, but, I'm not sure if there are more such scenarios.
Coming back to the specific change in the PR, it might be worth changing it
to
{code}
if (ne->next_inactivity_timeout_at == 0 &&
nh.config.default_inactivity_timeout > 0 &&
(ne->read.enabled || ne->write.enabled || ne->active_timeout_in ==
0) {
{code}
----------------------------------------------------------------
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]
