From 80208d68dc88848d42e2d4e5fc2a015f1d154f78 Mon Sep 17 00:00:00 2001 From: Brad Spencer Date: Tue, 15 Jan 2019 14:39:41 -0400 Subject: resolve: apply Happy Eyeballs philosophy to parallel c-ares queries Closes #3699 --- lib/asyn-ares.c | 95 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ lib/urldata.h | 1 + 2 files changed, 96 insertions(+) diff --git a/lib/asyn-ares.c b/lib/asyn-ares.c index 04a25b321..5e6ba3dd8 100644 --- a/lib/asyn-ares.c +++ b/lib/asyn-ares.c @@ -89,8 +89,20 @@ struct ResolverResults { int num_pending; /* number of ares_gethostbyname() requests */ Curl_addrinfo *temp_ai; /* intermediary result while fetching c-ares parts */ int last_status; + struct curltime happy_eyeballs_dns_time; /* when this timer started, or 0 */ }; +/* How long we are willing to wait for additional parallel responses after + obtaining a "definitive" one. + + This is intended to equal the c-ares default timeout. cURL always uses that + default value. Unfortunately, c-ares doesn't expose its default timeout in + its API, but it is officially documented as 5 seconds. + + See query_completed_cb() for an explanation of how this is used. + */ +#define HAPPY_EYEBALLS_DNS_TIMEOUT 5000 + /* * Curl_resolver_global_init() - the generic low-level asynchronous name * resolve API. Called from curl_global_init() to initialize global resolver @@ -347,6 +359,29 @@ CURLcode Curl_resolver_is_resolved(struct connectdata *conn, waitperform(conn, 0); + /* Now that we've checked for any last minute results above, see if there are + any responses still pending when the EXPIRE_HAPPY_EYEBALLS_DNS timer + expires. */ + if(res + && res->num_pending + /* This is only set to non-zero if the timer was started. */ + && (res->happy_eyeballs_dns_time.tv_sec + || res->happy_eyeballs_dns_time.tv_usec) + && (Curl_timediff(Curl_now(), res->happy_eyeballs_dns_time) + >= HAPPY_EYEBALLS_DNS_TIMEOUT)) { + /* Remember that the EXPIRE_HAPPY_EYEBALLS_DNS timer is no longer + running. */ + memset( + &res->happy_eyeballs_dns_time, 0, sizeof(res->happy_eyeballs_dns_time)); + + /* Cancel the raw c-ares request, which will fire query_completed_cb() with + ARES_ECANCELLED synchronously for all pending responses. This will + leave us with res->num_pending == 0, which is perfect for the next + block. */ + ares_cancel((ares_channel)data->state.resolver); + DEBUGASSERT(res->num_pending == 0); + } + if(res && !res->num_pending) { if(dns) { (void)Curl_addrinfo_callback(conn, res->last_status, res->temp_ai); @@ -517,6 +552,66 @@ static void query_completed_cb(void *arg, /* (struct connectdata *) */ /* A successful result overwrites any previous error */ if(res->last_status != ARES_SUCCESS) res->last_status = status; + + /* If there are responses still pending, we presume they must be the + complementary IPv4 or IPv6 lookups that we started in parallel in + Curl_resolver_getaddrinfo() (for Happy Eyeballs). If we've got a + "definitive" response from one of a set of parallel queries, we need to + think about how long we're willing to wait for more responses. */ + if(res->num_pending + /* Only these c-ares status values count as "definitive" for these + purposes. For example, ARES_ENODATA is what we expect when there is + no IPv6 entry for a domain name, and that's not a reason to get more + aggressive in our timeouts for the other response. Other errors are + either a result of bad input (which should affect all parallel + requests), local or network conditions, non-definitive server + responses, or us cancelling the request. */ + && (status == ARES_SUCCESS || status == ARES_ENOTFOUND)) { + /* Right now, there can only be up to two parallel queries, so don't + bother handling any other cases. */ + DEBUGASSERT(res->num_pending == 1); + + /* It's possible that one of these parallel queries could succeed + quickly, but the other could always fail or timeout (when we're + talking to a pool of DNS servers that can only successfully resolve + IPv4 address, for example). + + It's also possible that the other request could always just take + longer because it needs more time or only the second DNS server can + fulfill it successfully. But, to align with the philosophy of Happy + Eyeballs, we don't want to wait _too_ long or users will think + requests are slow when IPv6 lookups don't actually work (but IPv4 ones + do). + + So, now that we have a usable answer (some IPv4 addresses, some IPv6 + addresses, or "no such domain"), we start a timeout for the remaining + pending responses. Even though it is typical that this resolved + request came back quickly, that needn't be the case. It might be that + this completing request didn't get a result from the first DNS server + or even the first round of the whole DNS server pool. So it could + already be quite some time after we issued the DNS queries in the + first place. Without modifying c-ares, we can't know exactly where in + its retry cycle we are. We could guess based on how much time has + gone by, but it doesn't really matter. Happy Eyeballs tells us that, + given usable information in hand, we simply don't want to wait "too + much longer" after we get a result. + + We simply wait an additional amount of time equal to the default + c-ares query timeout. That is enough time for a typical parallel + response to arrive without being "too long". Even on a network + where one of the two types of queries is failing or timing out + constantly, this will usually mean we wait a total of the default + c-ares timeout (5 seconds) plus the round trip time for the successful + request, which seems bearable. The downside is that c-ares might race + with us to issue one more retry just before we give up, but it seems + better to "waste" that request instead of trying to guess the perfect + timeout to prevent it. After all, we don't even know where in the + c-ares retry cycle each request is. + */ + res->happy_eyeballs_dns_time = Curl_now(); + Curl_expire( + conn->data, HAPPY_EYEBALLS_DNS_TIMEOUT, EXPIRE_HAPPY_EYEBALLS_DNS); + } } } diff --git a/lib/urldata.h b/lib/urldata.h index 24187a4c4..a801459d3 100644 --- a/lib/urldata.h +++ b/lib/urldata.h @@ -1209,6 +1209,7 @@ typedef enum { EXPIRE_ASYNC_NAME, EXPIRE_CONNECTTIMEOUT, EXPIRE_DNS_PER_NAME, + EXPIRE_HAPPY_EYEBALLS_DNS, /* See asyn-ares.c */ EXPIRE_HAPPY_EYEBALLS, EXPIRE_MULTI_PENDING, EXPIRE_RUN_NOW, -- cgit v1.2.1