From 7fccf0ef9c2d8b19e21fc9fbb44ca924ef99b815 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 28 Apr 2017 13:56:20 -0700 Subject: [PATCH] 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 --- includes/config/EtcdConfig.php | 30 +++++++++++++++--------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/includes/config/EtcdConfig.php b/includes/config/EtcdConfig.php index 06d8dfb4cf..8f95e4ce1f 100644 --- 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 } -- 2.20.1