EtcdConfig: Fix infinite timeout bug, and reduce timeout
authorTim Starling <tstarling@wikimedia.org>
Thu, 4 May 2017 05:07:11 +0000 (15:07 +1000)
committerTim Starling <tstarling@wikimedia.org>
Thu, 4 May 2017 05:07:11 +0000 (15:07 +1000)
removeServer() returns the modified array, rather than passing by
reference, so you have to use the return value to avoid an infinite loop
when a server is down.

Tune the timeout downwards, to 2s. With three servers in the SRV pool,
if they are all unreachable, this will mean an overall request time of
6s, which is conveniently less than the APC lock time and the cache
TTL (9-10s). If the APC lock time is significantly shorter than the time
it takes to do the HTTP requests, then additional threads join in
waiting for the server. This could have stability consequences if the
maximum HHVM worker count is exceeded.

Change-Id: I3176aa41b8833c0ba0b668859e59911cd4392250

includes/config/EtcdConfig.php

index fd5c3f7..880cf9f 100644 (file)
@@ -73,7 +73,7 @@ class EtcdConfig implements Config, LoggerAwareInterface {
                        'encoding' => 'JSON',
                        'cacheTTL' => 10,
                        'skewTTL' => 1,
-                       'timeout' => 10
+                       'timeout' => 2
                ];
 
                $this->host = $params['host'];
@@ -215,7 +215,7 @@ class EtcdConfig implements Config, LoggerAwareInterface {
                        }
 
                        // Avoid the server next time if that failed
-                       $dsd->removeServer( $server, $servers );
+                       $servers = $dsd->removeServer( $server, $servers );
                } while ( $servers );
 
                return [ $config, $error, $retry ];