Merge "Removed unused and poorly supported time argument to BagOStuff::delete"
authorjenkins-bot <jenkins-bot@gerrit.wikimedia.org>
Thu, 5 Feb 2015 02:28:01 +0000 (02:28 +0000)
committerGerrit Code Review <gerrit@wikimedia.org>
Thu, 5 Feb 2015 02:28:01 +0000 (02:28 +0000)
1  2 
includes/objectcache/BagOStuff.php
includes/objectcache/HashBagOStuff.php
includes/objectcache/MemcachedBagOStuff.php
includes/objectcache/MemcachedPeclBagOStuff.php
includes/objectcache/MultiWriteBagOStuff.php
includes/objectcache/RedisBagOStuff.php
includes/objectcache/SqlBagOStuff.php

   * @defgroup Cache Cache
   */
  
 +use Psr\Log\LoggerAwareInterface;
 +use Psr\Log\LoggerInterface;
 +use Psr\Log\NullLogger;
 +
  /**
   * interface is intended to be more or less compatible with
   * the PHP memcached client.
   *
   * @ingroup Cache
   */
 -abstract class BagOStuff {
 +abstract class BagOStuff implements LoggerAwareInterface {
        private $debugMode = false;
  
        protected $lastError = self::ERR_NONE;
  
 +      /**
 +       * @var LoggerInterface
 +       */
 +      protected $logger;
 +
        /** 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
  
 +      public function __construct( array $params = array() ) {
 +              if ( isset( $params['logger'] ) ) {
 +                      $this->setLogger( $params['logger'] );
 +              } else {
 +                      $this->setLogger( new NullLogger() );
 +              }
 +      }
 +
 +      /**
 +       * @param LoggerInterface $logger
 +       * @return null
 +       */
 +      public function setLogger( LoggerInterface $logger ) {
 +              $this->logger = $logger;
 +      }
 +
        /**
         * @param bool $bool
         */
@@@ -83,9 -58,6 +83,6 @@@
                $this->debugMode = $bool;
        }
  
-       /* *** THE GUTS OF THE OPERATION *** */
-       /* Override these with functional things in subclasses */
        /**
         * Get an item with the given key. Returns false if it does not exist.
         * @param string $key
        /**
         * Delete an item.
         * @param string $key
-        * @param int $time Amount of time to delay the operation (mostly memcached-specific)
         * @return bool True if the item was deleted or not found, false on failure
         */
-       abstract public function delete( $key, $time = 0 );
+       abstract public function delete( $key );
  
        /**
         * Merge changes into the existing cache value (possibly creating a new one).
                $locked = false; // lock acquired
                $attempts = 0; // failed attempts
                do {
 -                      if ( ++$attempts >= 3 && $sleep <= 1e6 ) {
 +                      if ( ++$attempts >= 3 && $sleep <= 5e5 ) {
                                // Exponentially back off after failed attempts to avoid network spam.
                                // About 2*$uRTT*(2^n-1) us of "sleep" happen for the next n attempts.
                                $sleep *= 2;
         */
        public function debug( $text ) {
                if ( $this->debugMode ) {
 -                      $class = get_class( $this );
 -                      wfDebug( "$class debug: $text\n" );
 +                      $this->logger->debug( "{class} debug: $text", array(
 +                              'class' => get_class( $this ),
 +                      ) );
                }
        }
  
@@@ -31,8 -31,7 +31,8 @@@ class HashBagOStuff extends BagOStuff 
        /** @var array */
        protected $bag;
  
 -      function __construct() {
 +      function __construct( $params = array() ) {
 +              parent::__construct( $params );
                $this->bag = array();
        }
  
  
        /**
         * @param string $key
-        * @param int $time
         * @return bool
         */
-       function delete( $key, $time = 0 ) {
+       function delete( $key ) {
                if ( !isset( $this->bag[$key] ) ) {
                        return false;
                }
@@@ -91,11 -91,10 +91,10 @@@ class MemcachedBagOStuff extends BagOSt
  
        /**
         * @param string $key
-        * @param int $time
         * @return bool
         */
-       public function delete( $key, $time = 0 ) {
-               return $this->client->delete( $this->encodeKey( $key ), $time );
+       public function delete( $key ) {
+               return $this->client->delete( $this->encodeKey( $key ) );
        }
  
        /**
         * @param string $text
         */
        protected function debugLog( $text ) {
 -              wfDebugLog( 'memcached', $text );
 +              $this->logger->debug( $text );
        }
  }
@@@ -46,7 -46,6 +46,7 @@@ class MemcachedPeclBagOStuff extends Me
         * @throws MWException
         */
        function __construct( $params ) {
 +              parent::__construct( $params );
                $params = $this->applyDefaultParams( $params );
  
                if ( $params['persistent'] ) {
@@@ -55,7 -54,7 +55,7 @@@
                        // We can only reuse a pool ID if we keep the config consistent.
                        $this->client = new Memcached( md5( serialize( $params ) ) );
                        if ( count( $this->client->getServerList() ) ) {
 -                              wfDebug( __METHOD__ . ": persistent Memcached object already loaded.\n" );
 +                              $this->logger->debug( __METHOD__ . ": persistent Memcached object already loaded." );
                                return; // already initialized; don't add duplicate servers
                        }
                } else {
  
        /**
         * @param string $key
-        * @param int $time
         * @return bool
         */
-       public function delete( $key, $time = 0 ) {
+       public function delete( $key ) {
                $this->debugLog( "delete($key)" );
-               $result = parent::delete( $key, $time );
+               $result = parent::delete( $key );
                if ( $result === false && $this->client->getResultCode() === Memcached::RES_NOTFOUND ) {
                        // "Not found" is counted as success in our interface
                        return true;
                                break;
                        default:
                                $msg = $this->client->getResultMessage();
 +                              $logCtx = array();
                                if ( $key !== false ) {
                                        $server = $this->client->getServerByKey( $key );
 -                                      $serverName = "{$server['host']}:{$server['port']}";
 -                                      $msg = "Memcached error for key \"$key\" on server \"$serverName\": $msg";
 +                                      $logCtx['memcached-server'] = "{$server['host']}:{$server['port']}";
 +                                      $logCtx['memcached-key'] = $key;
 +                                      $msg = "Memcached error for key \"{memcached-key}\" on server \"{memcached-server}\": $msg";
                                } else {
                                        $msg = "Memcached error: $msg";
                                }
 -                              wfDebugLog( 'memcached-serious', $msg );
 +                              $this->logger->error( $msg, $logCtx );
                                $this->setLastError( BagOStuff::ERR_UNEXPECTED );
                }
                return $result;
@@@ -43,7 -43,6 +43,7 @@@ class MultiWriteBagOStuff extends BagOS
         * @throws MWException
         */
        public function __construct( $params ) {
 +              parent::__construct( $params );
                if ( !isset( $params['caches'] ) ) {
                        throw new MWException( __METHOD__ . ': the caches parameter is required' );
                }
  
        /**
         * @param string $key
-        * @param int $time
         * @return bool
         */
-       public function delete( $key, $time = 0 ) {
-               return $this->doWrite( 'delete', $key, $time );
+       public function delete( $key ) {
+               return $this->doWrite( 'delete', $key );
        }
  
        /**
@@@ -56,7 -56,6 +56,7 @@@ class RedisBagOStuff extends BagOStuff 
         * @param array $params
         */
        function __construct( $params ) {
 +              parent::__construct( $params );
                $redisConf = array( 'serializer' => 'none' ); // manage that in this class
                foreach ( array( 'connectTimeout', 'persistent', 'password' ) as $opt ) {
                        if ( isset( $params[$opt] ) ) {
                return $result;
        }
  
-       public function delete( $key, $time = 0 ) {
+       public function delete( $key ) {
  
                list( $server, $conn ) = $this->getConnection( $key );
                if ( !$conn ) {
         * @param string $msg
         */
        protected function logError( $msg ) {
 -              wfDebugLog( 'redis', "Redis error: $msg" );
 +              $this->logger->error( "Redis error: $msg" );
        }
  
        /**
@@@ -88,7 -88,6 +88,7 @@@ class SqlBagOStuff extends BagOStuff 
         * @param array $params
         */
        public function __construct( $params ) {
 +              parent::__construct( $params );
                if ( isset( $params['servers'] ) ) {
                        $this->serverInfos = $params['servers'];
                        $this->numServers = count( $this->serverInfos );
                        # If server connection info was given, use that
                        if ( $this->serverInfos ) {
                                if ( $wgDebugDBTransactions ) {
 -                                      wfDebug( "Using provided serverInfo for SqlBagOStuff\n" );
 +                                      $this->logger->debug( "Using provided serverInfo for SqlBagOStuff" );
                                }
                                $info = $this->serverInfos[$serverIndex];
                                $type = isset( $info['type'] ) ? $info['type'] : 'mysql';
                                $host = isset( $info['host'] ) ? $info['host'] : '[unknown]';
 -                              wfDebug( __CLASS__ . ": connecting to $host\n" );
 +                              $this->logger->debug( __CLASS__ . ": connecting to $host" );
                                $db = DatabaseBase::factory( $type, $info );
                                $db->clearFlag( DBO_TRX );
                        } else {
                                }
                        }
                        if ( $wgDebugDBTransactions ) {
 -                              wfDebug( sprintf( "Connection %s will be used for SqlBagOStuff\n", $db ) );
 +                              $this->logger->debug( sprintf( "Connection %s will be used for SqlBagOStuff", $db ) );
                        }
                        $this->conns[$serverIndex] = $db;
                }
  
        /**
         * @param string $key
-        * @param int $time
         * @return bool
         */
-       public function delete( $key, $time = 0 ) {
+       public function delete( $key ) {
                list( $serverIndex, $tableName ) = $this->getTableByKey( $key );
                try {
                        $db = $this->getDB( $serverIndex );
                if ( $exception instanceof DBConnectionError ) {
                        $this->markServerDown( $exception, $serverIndex );
                }
 -              wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" );
 +              $this->logger->error( "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
                        $this->setLastError( BagOStuff::ERR_UNREACHABLE );
 -                      wfDebug( __METHOD__ . ": ignoring connection error\n" );
 +                      $this->logger->debug( __METHOD__ . ": ignoring connection error" );
                } else {
                        $this->setLastError( BagOStuff::ERR_UNEXPECTED );
 -                      wfDebug( __METHOD__ . ": ignoring query error\n" );
 +                      $this->logger->debug( __METHOD__ . ": ignoring query error" );
                }
        }
  
                        } catch ( DBError $e ) {
                        }
                }
 -              wfDebugLog( 'SQLBagOStuff', "DBError: {$exception->getMessage()}" );
 +
 +              $this->logger->error( "DBError: {$exception->getMessage()}" );
                if ( $exception instanceof DBConnectionError ) {
                        $this->setLastError( BagOStuff::ERR_UNREACHABLE );
 -                      wfDebug( __METHOD__ . ": ignoring connection error\n" );
 +                      $this->logger->debug( __METHOD__ . ": ignoring connection error" );
                } else {
                        $this->setLastError( BagOStuff::ERR_UNEXPECTED );
 -                      wfDebug( __METHOD__ . ": ignoring query error\n" );
 +                      $this->logger->debug( __METHOD__ . ": ignoring query error" );
                }
        }
  
                                unset( $this->connFailureTimes[$serverIndex] );
                                unset( $this->connFailureErrors[$serverIndex] );
                        } else {
 -                              wfDebug( __METHOD__ . ": Server #$serverIndex already down\n" );
 +                              $this->logger->debug( __METHOD__ . ": Server #$serverIndex already down" );
                                return;
                        }
                }
                $now = time();
 -              wfDebug( __METHOD__ . ": Server #$serverIndex down until " . ( $now + 60 ) . "\n" );
 +              $this->logger->info( __METHOD__ . ": Server #$serverIndex down until " . ( $now + 60 ) );
                $this->connFailureTimes[$serverIndex] = $now;
                $this->connFailureErrors[$serverIndex] = $exception;
        }