delay DNS for origin server until it is required #1756
Conversation
|
clang format successful! https://ci.trafficserver.apache.org/job/clang-format-github/352/ |
|
RAT check successful! https://ci.trafficserver.apache.org/job/RAT-github/364/ |
|
FreeBSD11 build successful! https://ci.trafficserver.apache.org/job/freebsd-github/2047/ |
|
Intel CC build successful! https://ci.trafficserver.apache.org/job/icc-github/478/ |
|
Linux build successful! https://ci.trafficserver.apache.org/job/linux-github/1940/ |
|
AU check successful! https://ci.trafficserver.apache.org/job/autest-github/349/ |
|
clang-analyzer build successful! https://ci.trafficserver.apache.org/job/clang-analyzer-github/611/ |
proxy/http/HttpTransact.cc
Outdated
| s->current.attempts = 0; | ||
| // Do DNS as we are now going to Origin Server | ||
| TRANSACT_RETURN(SM_ACTION_DNS_LOOKUP, OSDNSLookup); | ||
| /*s->current.attempts = 0; |
There was a problem hiding this comment.
If this is dead code, either #ifdef it, or just remove it. These comments makes it difficult to see the intent here :).
There was a problem hiding this comment.
yeah, i think that should be removed.
There was a problem hiding this comment.
it should be removed
proxy/http/HttpTransact.cc
Outdated
| build_request(s, &s->hdr_info.client_request, &s->hdr_info.server_request, s->current.server->http_version); | ||
|
|
||
| s->next_action = how_to_open_connection(s); | ||
| // TODO doesn't make sense to set this here, find the right place |
There was a problem hiding this comment.
Is this so? If so, where do we put it? If we're keeping it here, then the comment probably should go ?
There was a problem hiding this comment.
The comment should go away. I forgot to remove the comment i added.
|
I don't see anything wrong with this after making the requested changes from @zwoop |
|
We've been running this in prod for ~24h, and pretty decent results (and no issues as far as we can tell). Attaching a graph showing the different with and without this patch. |
|
|
|
Marking this for 7.1.0 as well, once merged, I will cherry-pick. |
|
incorporated the review comments. |
|
clang format successful! https://ci.trafficserver.apache.org/job/clang-format-github/371/ |
|
RAT check successful! https://ci.trafficserver.apache.org/job/RAT-github/383/ |
|
AU check successful! https://ci.trafficserver.apache.org/job/autest-github/367/ |
|
FreeBSD11 build successful! https://ci.trafficserver.apache.org/job/freebsd-github/2066/ |
|
Intel CC build successful! https://ci.trafficserver.apache.org/job/icc-github/496/ |
|
Linux build successful! https://ci.trafficserver.apache.org/job/linux-github/1959/ |
|
zwoop let me know if the changes are okay, i will merge |
|
clang-analyzer build successful! https://ci.trafficserver.apache.org/job/clang-analyzer-github/629/ |
4 participants
There is no need to perform DNS for origin server under some scenarios. For example, request matches a parent and the parent handles the request or all the parents that matched the request died and the parent configuration does not allow to talk to origin server directly.