lzx404243 commented on code in PR #9620:
URL: https://github.com/apache/trafficserver/pull/9620#discussion_r1176721909
##########
proxy/http/HttpTransact.h:
##########
@@ -944,7 +944,22 @@ class HttpTransact
MgmtInt
configured_connect_attempts_max_retries() const
{
- return txn_conf->connect_attempts_max_retries;
+ if (dns_info.looking_up != ResolveInfo::PARENT_PROXY) {
+ return txn_conf->connect_attempts_max_retries;
+ }
+ // For parent proxy, return the maximum attempt count for the current
+ // parent intead of the global max retries for the whole parent group.
+ // The max attempt count for the current parent is calculated by rounding
+ // the current attempt up to next multiple of ppca.
+ auto ppca = txn_conf->per_parent_connect_attempts;
+ auto cur_attempts = current.attempts.get();
+ auto cur_parent_max_attempts = ((cur_attempts + ppca - 1) / ppca) * ppca;
+ if (cur_attempts == cur_parent_max_attempts) {
+ // If the current attempt is already a multiple of ppca, get the next
+ // multiple.
+ cur_parent_max_attempts += ppca;
+ }
Review Comment:
> This call:
>
>
https://github.com/apache/trafficserver/blob/6e8933f35ebf5a50211e3b06aa52749ac0ec3c3b/proxy/http/HttpTransact.cc#L3646
>
> can't cause an assert
To clarify, the `SDK_API_HttpParentProxySet_Fail` regression test failed at
this assert without the `if` statement. The problem doesn't seem to be
`current.attempts` being too large, but the value from
`configured_connect_attempts_max_retries()` being too small. Consider the
following case:
- `current.attempts` : 4
- `per_parent_connect_attempts`: 2
- `parent_proxy.total_connect_attempts`: 100
in this case, without the `if` block, the
`configured_connect_attempts_max_retries()` would return `4` and pass that into
`increment()`. Assertion in the `increment()` as after the increment,
`current.attempts` goes pass the input "max".
--
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]
