Improve timeouts in MultiHttpClient
authorTim Starling <tstarling@wikimedia.org>
Fri, 15 Jun 2018 10:31:02 +0000 (20:31 +1000)
committerTim Starling <tstarling@wikimedia.org>
Fri, 15 Jun 2018 11:04:05 +0000 (21:04 +1000)
* PHP 7 only checks for request/connection timeout after the timeout
  passed to curl_multi_select() expires. So it's necessary to use a
  select timeout which is much shorter than 10s. I filed
  https://bugs.php.net/bug.php?id=76480 for this.
* Use millisecond resolution timeout parameters. These have been
  available since libcurl 7.16.2, released Jan 2008, available in
  Debian 7, Ubuntu 14.04 and CentOS 6.

Change-Id: Ia07b824dde179b33e14b81a76d580ce547bca315

includes/libs/MultiHttpClient.php

index cb60b01..20ddf72 100644 (file)
@@ -50,9 +50,9 @@ class MultiHttpClient implements LoggerAwareInterface {
        protected $multiHandle = null; // curl_multi handle
        /** @var string|null SSL certificates path */
        protected $caBundlePath;
        protected $multiHandle = null; // curl_multi handle
        /** @var string|null SSL certificates path */
        protected $caBundlePath;
-       /** @var int */
+       /** @var float */
        protected $connTimeout = 10;
        protected $connTimeout = 10;
-       /** @var int */
+       /** @var float */
        protected $reqTimeout = 300;
        /** @var bool */
        protected $usePipelining = false;
        protected $reqTimeout = 300;
        /** @var bool */
        protected $usePipelining = false;
@@ -65,6 +65,11 @@ class MultiHttpClient implements LoggerAwareInterface {
        /** @var LoggerInterface */
        protected $logger;
 
        /** @var LoggerInterface */
        protected $logger;
 
+       // In PHP 7 due to https://bugs.php.net/bug.php?id=76480 the request/connect
+       // timeouts are periodically polled instead of being accurately respected.
+       // The select timeout is set to the minimum timeout multiplied by this factor.
+       const TIMEOUT_ACCURACY_FACTOR = 0.1;
+
        /**
         * @param array $options
         *   - connTimeout     : default connection timeout (seconds)
        /**
         * @param array $options
         *   - connTimeout     : default connection timeout (seconds)
@@ -148,6 +153,8 @@ class MultiHttpClient implements LoggerAwareInterface {
        public function runMulti( array $reqs, array $opts = [] ) {
                $chm = $this->getCurlMulti();
 
        public function runMulti( array $reqs, array $opts = [] ) {
                $chm = $this->getCurlMulti();
 
+               $selectTimeout = $this->getSelectTimeout( $opts );
+
                // Normalize $reqs and add all of the required cURL handles...
                $handles = [];
                foreach ( $reqs as $index => &$req ) {
                // Normalize $reqs and add all of the required cURL handles...
                $handles = [];
                foreach ( $reqs as $index => &$req ) {
@@ -224,7 +231,7 @@ class MultiHttpClient implements LoggerAwareInterface {
                                } while ( $mrc == CURLM_CALL_MULTI_PERFORM );
                                // Wait (if possible) for available work...
                                if ( $active > 0 && $mrc == CURLM_OK ) {
                                } while ( $mrc == CURLM_CALL_MULTI_PERFORM );
                                // Wait (if possible) for available work...
                                if ( $active > 0 && $mrc == CURLM_OK ) {
-                                       if ( curl_multi_select( $chm, 10 ) == -1 ) {
+                                       if ( curl_multi_select( $chm, $selectTimeout ) == -1 ) {
                                                // PHP bug 63411; https://curl.haxx.se/libcurl/c/curl_multi_fdset.html
                                                usleep( 5000 ); // 5ms
                                        }
                                                // PHP bug 63411; https://curl.haxx.se/libcurl/c/curl_multi_fdset.html
                                                usleep( 5000 ); // 5ms
                                        }
@@ -285,11 +292,11 @@ class MultiHttpClient implements LoggerAwareInterface {
        protected function getCurlHandle( array &$req, array $opts = [] ) {
                $ch = curl_init();
 
        protected function getCurlHandle( array &$req, array $opts = [] ) {
                $ch = curl_init();
 
-               curl_setopt( $ch, CURLOPT_CONNECTTIMEOUT,
-                       $opts['connTimeout'] ?? $this->connTimeout );
+               curl_setopt( $ch, CURLOPT_CONNECTTIMEOUT_MS,
+                       ( $opts['connTimeout'] ?? $this->connTimeout ) * 1000 );
                curl_setopt( $ch, CURLOPT_PROXY, $req['proxy'] ?? $this->proxy );
                curl_setopt( $ch, CURLOPT_PROXY, $req['proxy'] ?? $this->proxy );
-               curl_setopt( $ch, CURLOPT_TIMEOUT,
-                       $opts['reqTimeout'] ?? $this->reqTimeout );
+               curl_setopt( $ch, CURLOPT_TIMEOUT_MS,
+                       ( $opts['reqTimeout'] ?? $this->reqTimeout ) * 1000 );
                curl_setopt( $ch, CURLOPT_FOLLOWLOCATION, 1 );
                curl_setopt( $ch, CURLOPT_MAXREDIRS, 4 );
                curl_setopt( $ch, CURLOPT_HEADER, 0 );
                curl_setopt( $ch, CURLOPT_FOLLOWLOCATION, 1 );
                curl_setopt( $ch, CURLOPT_MAXREDIRS, 4 );
                curl_setopt( $ch, CURLOPT_HEADER, 0 );
@@ -410,6 +417,28 @@ class MultiHttpClient implements LoggerAwareInterface {
                return $ch;
        }
 
                return $ch;
        }
 
+       /**
+        * Get a suitable select timeout for the given options.
+        *
+        * @param array $opts
+        * @return float
+        */
+       private function getSelectTimeout( $opts ) {
+               $connTimeout = $opts['connTimeout'] ?? $this->connTimeout;
+               $reqTimeout = $opts['reqTimeout'] ?? $this->reqTimeout;
+               $timeouts = array_filter( [ $connTimeout, $reqTimeout ] );
+               if ( count( $timeouts ) === 0 ) {
+                       return 1;
+               }
+
+               $selectTimeout = min( $timeouts ) * self::TIMEOUT_ACCURACY_FACTOR;
+               // Minimum 10us for sanity
+               if ( $selectTimeout < 10e-6 ) {
+                       $selectTimeout = 10e-6;
+               }
+               return $selectTimeout;
+       }
+
        /**
         * @return resource
         * @throws Exception
        /**
         * @return resource
         * @throws Exception