lzx404243 commented on code in PR #9620:
URL: https://github.com/apache/trafficserver/pull/9620#discussion_r1181810533


##########
proxy/http/HttpTransact.h:
##########
@@ -605,10 +605,9 @@ class HttpTransact
       }
 
       void
-      increment(MgmtInt configured_connect_attempts_max_retries)
+      increment()
       {
         ++_v;
-        ink_assert(_v <= configured_connect_attempts_max_retries);

Review Comment:
   This is a great question. The check before the increment ensures that 
`attempts - 1 < the global parent_max_retries`. The problem with this assert is 
that the argument passed in `configured_connect_attempts_max_retries` may not 
be the global parent max retry, but the max retry count for the current parent. 
Currently, the argument is set to the the output of 
`s->configured_connect_attempts_max_retries()`, which is returning:
   
   - if connecting to the OS, returns the 
`txn_conf->connect_attempts_max_retries` 
   - if connecting to a parent proxy, returns the max retries for the current 
parent(bounded by the global max retry for all parents)
   In the second case, the assert can fail as after the increment, it went over 
the max for the current parent(which should be allowed as we switch to the next 
parent). 



-- 
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.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to