lzx404243 commented on code in PR #9620:
URL: https://github.com/apache/trafficserver/pull/9620#discussion_r1182734532
##########
proxy/http/HttpTransact.cc:
##########
@@ -3641,12 +3641,12 @@ HttpTransact::handle_response_from_parent(State *s)
return;
}
- if (s->current.attempts.get() < s->txn_conf->parent_connect_attempts) {
Review Comment:
Good point. I think the confusion arises from the naming.
`s->current.attempts.get()` is actually the retry count, so when
`s->current.attempts.get() = 2`, it has retried for 2 times already(3 attempts
in total).
The naming confusion is addressed in another PR:
https://github.com/apache/trafficserver/pull/9655 .
##########
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:
> even when we hit the max retries for the current parent, don't your new
changes in `configured_connect_attempts_max_retries()` account for the next
parent?
`configured_connect_attempts_max_retries()` normally would be increment to
the next multiple of `ppca`(accounts for the next parent), except for the case
where the attempt/(retry + 1) is already a multiple of ppca(current parent hits
max retries), in which case the calculation basically returns the same
attempt/retry count as before.
> im trying to find the edge case when this assert fails, could you provide
an example
The above logic make sense when we call `maximize()` with the
`configured_connect_attempts_max_retries()` to disable further retries, since
we already hit the max retries for the current parent, we are not setting it to
anything larger.
However, it will fail the assert in `increment()` with the
`configured_connect_attempts_max_retries()`, since `n =
configured_connect_attempts_max_retries(n).`
--
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]
