Made BagOStuff fail fast in cas/lock on certain errors
authorAaron Schulz <aschulz@wikimedia.org>
Tue, 1 Apr 2014 20:25:42 +0000 (13:25 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Tue, 1 Apr 2014 20:26:44 +0000 (13:26 -0700)
* This avoids wasting time waiting on down servers

bug: 63058
Change-Id: Ia78a2036ffa7a20102ec9e8bf941cacb7fa63435

includes/objectcache/BagOStuff.php
includes/objectcache/MemcachedPeclBagOStuff.php
includes/objectcache/MultiWriteBagOStuff.php
includes/objectcache/RedisBagOStuff.php
includes/objectcache/SqlBagOStuff.php

index 217142c..ad3aa96 100644 (file)
 abstract class BagOStuff {
        private $debugMode = false;
 
+       protected $lastError = self::ERR_NONE;
+
+       /** Possible values for getLastError() */
+       const ERR_NONE        = 0; // no error
+       const ERR_NO_RESPONSE = 1; // no response
+       const ERR_UNREACHABLE = 2; // can't connect
+       const ERR_UNEXPECTED  = 3; // response gave some error
+
        /**
         * @param $bool bool
         */
@@ -169,9 +177,12 @@ abstract class BagOStuff {
         * @return bool success
         */
        public function lock( $key, $timeout = 6 ) {
+               $this->clearLastError();
                $timestamp = microtime( true ); // starting UNIX timestamp
                if ( $this->add( "{$key}:lock", 1, $timeout ) ) {
                        return true;
+               } elseif ( $this->getLastError() ) {
+                       return false;
                }
 
                $uRTT = ceil( 1e6 * ( microtime( true ) - $timestamp ) ); // estimate RTT (us)
@@ -186,7 +197,11 @@ abstract class BagOStuff {
                                $sleep *= 2;
                        }
                        usleep( $sleep ); // back off
+                       $this->clearLastError();
                        $locked = $this->add( "{$key}:lock", 1, $timeout );
+                       if ( $this->getLastError() ) {
+                               return false;
+                       }
                } while ( !$locked );
 
                return $locked;
@@ -292,6 +307,32 @@ abstract class BagOStuff {
                return $this->incr( $key, - $value );
        }
 
+       /**
+        * Get the "last error" registered; clearLastError() should be called manually
+        * @return integer ERR_* constant for the "last error" registry
+        * @since 1.23
+        */
+       public function getLastError() {
+               return $this->lastError;
+       }
+
+       /**
+        * Clear the "last error" registry
+        * @since 1.23
+        */
+       public function clearLastError() {
+               $this->lastError = self::ERR_NONE;
+       }
+
+       /**
+        * Set the "last error" registry
+        * @param $err integer ERR_* constant
+        * @since 1.23
+        */
+       protected function setLastError( $err ) {
+               $this->lastError = $err;
+       }
+
        /**
         * @param $text string
         */
index 18546d4..1c780b4 100644 (file)
@@ -231,6 +231,7 @@ class MemcachedPeclBagOStuff extends MemcachedBagOStuff {
                                        $msg = "Memcached error: $msg";
                                }
                                wfDebugLog( 'memcached-serious', $msg );
+                               $this->setLastError( BagOStuff::ERR_UNEXPECTED );
                }
                return $result;
        }
index e550c0d..b97410a 100644 (file)
@@ -179,6 +179,16 @@ class MultiWriteBagOStuff extends BagOStuff {
                return $this->doWrite( 'merge', $key, $callback, $exptime );
        }
 
+       public function getLastError() {
+               return isset( $this->caches[0] ) ? $this->caches[0]->getLastError() : self::ERR_NONE;
+       }
+
+       public function clearLastError() {
+               if ( isset( $this->caches[0] ) ) {
+                       $this->caches[0]->clearLastError();
+               }
+       }
+
        /**
         * @param $method string
         * @return bool
index f54726f..872af63 100644 (file)
@@ -305,6 +305,7 @@ class RedisBagOStuff extends BagOStuff {
                                return array( $server, $conn );
                        }
                }
+               $this->setLastError( BagOStuff::ERR_UNREACHABLE );
                return array( false, false );
        }
 
@@ -322,6 +323,7 @@ class RedisBagOStuff extends BagOStuff {
         * object and let it be reopened during the next request.
         */
        protected function handleException( RedisConnRef $conn, $e ) {
+               $this->setLastError( BagOStuff::ERR_UNEXPECTED );
                $this->redisPool->handleError( $conn, $e );
        }
 
index 2c90339..74b5de2 100644 (file)
@@ -626,8 +626,10 @@ class SqlBagOStuff extends BagOStuff {
                }
                wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
+                       $this->setLastError( BagOStuff::ERR_UNREACHABLE );
                        wfDebug( __METHOD__ . ": ignoring connection error\n" );
                } else {
+                       $this->setLastError( BagOStuff::ERR_UNEXPECTED );
                        wfDebug( __METHOD__ . ": ignoring query error\n" );
                }
        }
@@ -646,8 +648,10 @@ class SqlBagOStuff extends BagOStuff {
                }
                wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
+                       $this->setLastError( BagOStuff::ERR_UNREACHABLE );
                        wfDebug( __METHOD__ . ": ignoring connection error\n" );
                } else {
+                       $this->setLastError( BagOStuff::ERR_UNEXPECTED );
                        wfDebug( __METHOD__ . ": ignoring query error\n" );
                }
        }