Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check if system has support for sub-second connect timeouts (CURL_VERSION_ASYNCHDNS) #264

Open
TysonAndre opened this issue Feb 10, 2017 · 6 comments

Comments

@TysonAndre
Copy link
Contributor

Related to 4e1c18f. I ask for this because I want to have a 'timeout' of less than a second.

It seems like it should be possible to avoid using signals such as alarm() (via CURLOPT_NOSIGNAL), and to detect if there is an alternative dns resolver (via CURL_VERSION_ASYNCHDNS)

https://curl.haxx.se/libcurl/c/curl_version_info.html

features can have none, one or more bits set, and the currently defined bits are:
...
CURL_VERSION_ASYNCHDNS

libcurl was built with support for asynchronous name lookups, which allows more exact timeouts (even > on Windows) and less blocking when using the multi interface. (added in 7.10.7)

http://stackoverflow.com/questions/25998063/how-can-i-tell-if-the-libcurl-installed-has-asynchronous-dns-enabled also mentions this.

Also, a workaround (when ASYNCDNS is absent) might be to disable timeouts for dns lookup (if the 'connect' time is sub-millisecond), so that the request timeout can be set to less than one second. I'm not sure if that will introduce new bugs, e.g. if DNS is slow/unreliable.

http://www.dsm.fordham.edu/cgi-bin/man-cgi.pl?topic=CURLOPT_NOSIGNAL&ampsect=3

  If this option is set and libcurl has been built with the standard name
  resolver,  timeouts  will not occur while the name resolve takes place.
  Consider building libcurl with the c-ares or threaded resolver backends
  to  enable  asynchronous  DNS  lookups,  to  enable  timeouts  for name
  resolves without the use of signals.

On an unrelated note, is the minimum necessary if the host name is an IPV4 or IPV6 address (e.g. 127.0.0.1, etc.), and HTTP redirects are disabled? (may have to set CURLOPT_NOSIGNAL so that the "lookup" will still avoid timing out)

@rmccue
Copy link
Collaborator

rmccue commented Jun 9, 2017

Interesting, this constant must be new. I'm happy with allowing sub-second timeouts if ASYNCDNS is available.

Also, a workaround (when ASYNCDNS is absent) might be to disable timeouts for dns lookup (if the 'connect' time is sub-millisecond), so that the request timeout can be set to less than one second. I'm not sure if that will introduce new bugs, e.g. if DNS is slow/unreliable.

When I dug into the underlying code, it appeared that if any DNS resolution was attempted, cURL would wait at least one tick (i.e. 1s) before failing.

On an unrelated note, is the minimum necessary if the host name is an IPV4 or IPV6 address (e.g. 127.0.0.1, etc.), and HTTP redirects are disabled? (may have to set CURLOPT_NOSIGNAL so that the "lookup" will still avoid timing out)

Potentially not, but that's such an edge case I didn't think it was worth accounting for.

@TysonAndre
Copy link
Contributor Author

@rmccue - I switched to building libcurl with async DNS support a while back, in a fork of Requests. It ended up working, but I hit a weird edge case when trying to "resolve" IPs.

So this might introduce new bugs.

  • If I requested a timeout of 1 millisecond from requests for one of the timeouts with blocking => false(I forget which one, probably CURLOPT_TIMEOUT_MS), it would end up taking much, much longer to send data (Waiting for the request to complete)
  • If I bumped the timeout to 2 milliseconds (if ($timeoutMs > 0) { $timeoutMs = max(2, $timeoutMs)), delays went back to normal
  • Context: That was a hack to send "async" HTTP requests to another service (not ideal, rabbitmq or local daemon might have been a better architecture)

@rmccue
Copy link
Collaborator

rmccue commented Jun 9, 2017

Gnarly. Seems like cURL internally is probably doing something strange with these timeouts. For a nice round number, 10ms could be a good minimum?

@TysonAndre
Copy link
Contributor Author

As a soft minimum, 10 makes sense for the majority of use cases. Maybe add an option allow_extremely_low_timeout => true to confirm that's what the user is trying to do?
As a hard minimum, 2 is needed as a workaround for that bug. (Not sure if anyone else uses timeouts that low. 5ms might make sense for services on localhost?

@rmccue
Copy link
Collaborator

rmccue commented Jun 9, 2017

If you really want a specific timeout, you can use the curl.before_send hook to set the option on the cURL handle directly, so I wouldn't worry about adding an option for it.

@TysonAndre
Copy link
Contributor Author

TysonAndre commented Jun 13, 2017

This is the patch I'm using, but it's specific to php7.1 and a newer libcurl version (and certain libcurl build flags)

--- a/library/Requests/Transport/cURL.php
+++ b/library/Requests/Transport/cURL.php
@@ -357,12 +360,28 @@ class Requests_Transport_cURL implements Requests_Transport {
                // end, so we need to round up regardless of the supplied timeout.
                //
                // https://github.com/curl/curl/blob/4f45240bc84a9aa648c8f7243be7b79e9f9323a5/lib/hostip.c#L606-L609
-               $timeout = max($options['timeout'], 1);
+               //
+               // NOTE: optimization for ['blocking' => false]
+               // use if you have a fast DNS resolver or frequently use IP addresses instead of dns
+               // TODO: I'm not sure if this is a generic fix - What about DNS lookups in HTTP Redirects?
+               $timeout = $options['timeout'];
 
                if (is_int($timeout) || $this->version < self::CURL_7_16_2) {
                        curl_setopt($this->handle, CURLOPT_TIMEOUT, ceil($timeout));
                }
                else {
+                       // Make curl timeout work for sub-second values
+                       // See https://github.com/rmccue/Requests/issues/264
+                       // and http://www.dsm.fordham.edu/cgi-bin/man-cgi.pl?topic=CURLOPT_NOSIGNAL&ampsect=3
+                       curl_setopt($this->handle, CURLOPT_NOSIGNAL, 1);
+                       // check to support for upgraded curl and/or threaded async resolver.
+                       // (1 millisecond seems to be treated as a special value, and it ended up taking longer than 2 milliseconds)
+                       if ($timeout > 0) {
+                               $timeout = max($timeout, 0.002);
+                       }
                        curl_setopt($this->handle, CURLOPT_TIMEOUT_MS, round($timeout * 1000));
                }
 
@@ -370,6 +389,12 @@ class Requests_Transport_cURL implements Requests_Transport {
                        curl_setopt($this->handle, CURLOPT_CONNECTTIMEOUT, ceil($options['connect_timeout']));
                }
                else {
+                       // Make curl timeout work for sub-second values
+                       // See https://github.com/rmccue/Requests/issues/264
+                       // and http://www.dsm.fordham.edu/cgi-bin/man-cgi.pl?topic=CURLOPT_NOSIGNAL&ampsect=3
+                       curl_setopt($this->handle, CURLOPT_NOSIGNAL, 1);
                         curl_setopt($this->handle, CURLOPT_CONNECTTIMEOUT_MS, round($options['connect_timeout'] * 1000));
                }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants