Improve EtcdConfig fallback logic

If the cache is stale and the lock keeps being acquired, but the re-fetch
fails each time, the method would end up failing after the timeout was
reached. Instead, use the stale cache if available.

Change-Id: Ieafc9de17e6c60d8eea7b937923b4ad548e99be8

diff --git a/includes/config/EtcdConfig.php b/includes/config/EtcdConfig.php
index 06d8dfb..8f95e4c 100644 (file)
--- a/includes/config/EtcdConfig.php
+++ b/includes/config/EtcdConfig.php
@@ -148,24 +148,24 @@ class EtcdConfig implements Config, LoggerAwareInterface {
                                if ( $this->srvCache->lock( $key, 0, $this->baseCacheTTL ) ) {
                                        try {
                                                list( $config, $error, $retry ) = $this->fetchAllFromEtcd();
-                                               if ( $config === null ) {
-                                                       $this->logger->error( "Failed to fetch configuration: $error" );
-                                                       // Fail fast if the error is likely to just keep happening
-                                                       return $retry
-                                                               ? WaitConditionLoop::CONDITION_CONTINUE
-                                                               : WaitConditionLoop::CONDITION_FAILED;
-                                               }
-
-                                               // Avoid having all servers expire cache keys at the same time
-                                               $expiry = microtime( true ) + $this->baseCacheTTL;
-                                               $expiry += mt_rand( 0, 1e6 ) / 1e6 * $this->skewCacheTTL;
+                                               if ( is_array( $config ) ) {
+                                                       // Avoid having all servers expire cache keys at the same time
+                                                       $expiry = microtime( true ) + $this->baseCacheTTL;
+                                                       $expiry += mt_rand( 0, 1e6 ) / 1e6 * $this->skewCacheTTL;
 
-                                               $data = [ 'config' => $config, 'expires' => $expiry ];
-                                               $this->srvCache->set( $key, $data, BagOStuff::TTL_INDEFINITE );
+                                                       $data = [ 'config' => $config, 'expires' => $expiry ];
+                                                       $this->srvCache->set( $key, $data, BagOStuff::TTL_INDEFINITE );
 
-                                               $this->logger->info( "Refreshed stale etcd configuration cache." );
+                                                       $this->logger->info( "Refreshed stale etcd configuration cache." );
 
-                                               return WaitConditionLoop::CONDITION_REACHED;
+                                                       return WaitConditionLoop::CONDITION_REACHED;
+                                               } else {
+                                                       $this->logger->error( "Failed to fetch configuration: $error" );
+                                                       if ( !$retry ) {
+                                                               // Fail fast since the error is likely to keep happening
+                                                               return WaitConditionLoop::CONDITION_FAILED;
+                                                       }
+                                               }
                                        } finally {
                                                $this->srvCache->unlock( $key ); // release mutex
                                        }