Added simpler handleError() method in RedisConnectionPool

* Callers should not need to give $server since it is stored in RedisConnRef

Change-Id: I902d984d6a7f19dd0d8c71ee374cbed359de378e

diff --git a/includes/clientpool/RedisConnectionPool.php b/includes/clientpool/RedisConnectionPool.php
index 983d90a..9e702e3 100644 (file)
--- a/includes/clientpool/RedisConnectionPool.php
+++ b/includes/clientpool/RedisConnectionPool.php
@@ -277,9 +277,24 @@ class RedisConnectionPool {
         * @param string $server
         * @param RedisConnRef $cref
         * @param RedisException $e
+        * @deprecated 1.23
         */
        public function handleException( $server, RedisConnRef $cref, RedisException $e ) {
-               wfDebugLog( 'redis', "Redis exception on server $server: " . $e->getMessage() );
+               return $this->handleError( $cref, $e );
+       }
+
+       /**
+        * The redis extension throws an exception in response to various read, write
+        * and protocol errors. Sometimes it also closes the connection, sometimes
+        * not. The safest response for us is to explicitly destroy the connection
+        * object and let it be reopened during the next request.
+        *
+        * @param RedisConnRef $cref
+        * @param RedisException $e
+        */
+       public function handleError( RedisConnRef $cref, RedisException $e ) {
+               $server = $cref->getServer();
+               wfDebugLog( 'redis', "Redis exception on server $server: " . $e->getMessage() . "\n" );
                foreach ( $this->connections[$server] as $key => $connection ) {
                        if ( $cref->isConnIdentical( $connection['conn'] ) ) {
                                $this->idlePoolSize -= $connection['free'] ? 1 : 0;

diff --git a/includes/filebackend/lockmanager/RedisLockManager.php b/includes/filebackend/lockmanager/RedisLockManager.php
index e37e567..ff4cba5 100644 (file)
--- a/includes/filebackend/lockmanager/RedisLockManager.php
+++ b/includes/filebackend/lockmanager/RedisLockManager.php
@@ -153,7 +153,7 @@ LUA;
                        );
                } catch ( RedisException $e ) {
                        $res = false;
-                       $this->redisPool->handleException( $server, $conn, $e );
+                       $this->redisPool->handleError( $conn, $e );
                }
 
                if ( $res === false ) {
@@ -221,7 +221,7 @@ LUA;
                        );
                } catch ( RedisException $e ) {
                        $res = false;
-                       $this->redisPool->handleException( $server, $conn, $e );
+                       $this->redisPool->handleError( $conn, $e );
                }
 
                if ( $res === false ) {

diff --git a/includes/job/JobQueueRedis.php b/includes/job/JobQueueRedis.php
index e0641b5..3422664 100644 (file)
--- a/includes/job/JobQueueRedis.php
+++ b/includes/job/JobQueueRedis.php
@@ -120,7 +120,7 @@ class JobQueueRedis extends JobQueue {
                try {
                        return $conn->lSize( $this->getQueueKey( 'l-unclaimed' ) );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -141,7 +141,7 @@ class JobQueueRedis extends JobQueue {
 
                        return array_sum( $conn->exec() );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -158,7 +158,7 @@ class JobQueueRedis extends JobQueue {
                try {
                        return $conn->zSize( $this->getQueueKey( 'z-delayed' ) );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -175,7 +175,7 @@ class JobQueueRedis extends JobQueue {
                try {
                        return $conn->zSize( $this->getQueueKey( 'z-abandoned' ) );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -229,7 +229,7 @@ class JobQueueRedis extends JobQueue {
                        JobQueue::incrStats( 'job-insert-duplicate', $this->type,
                                count( $items ) - $failed - $pushed );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
 
                return true;
@@ -330,7 +330,7 @@ LUA;
                                $job = $this->getJobFromFields( $item ); // may be false
                        } while ( !$job ); // job may be false if invalid
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
 
                return $job;
@@ -442,7 +442,7 @@ LUA;
                                        return false;
                                }
                        } catch ( RedisException $e ) {
-                               $this->throwRedisException( $this->server, $conn, $e );
+                               $this->throwRedisException( $conn, $e );
                        }
                }
 
@@ -473,7 +473,7 @@ LUA;
                        // Update the timestamp of the last root job started at the location...
                        return $conn->set( $key, $params['rootJobTimestamp'], self::ROOTJOB_TTL ); // 2 weeks
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -494,7 +494,7 @@ LUA;
                        // Get the last time this root job was enqueued
                        $timestamp = $conn->get( $this->getRootJobCacheKey( $params['rootJobSignature'] ) );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
 
                // Check if a new root job was started at the location after this one's...
@@ -519,7 +519,7 @@ LUA;
 
                        return ( $conn->delete( $keys ) !== false );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -542,7 +542,7 @@ LUA;
                                } )
                        );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -565,7 +565,7 @@ LUA;
                                } )
                        );
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -580,8 +580,8 @@ LUA;
        protected function doGetSiblingQueueSizes( array $types ) {
                $sizes = array(); // (type => size)
                $types = array_values( $types ); // reindex
+               $conn = $this->getConnection();
                try {
-                       $conn = $this->getConnection();
                        $conn->multi( Redis::PIPELINE );
                        foreach ( $types as $type ) {
                                $conn->lSize( $this->getQueueKey( 'l-unclaimed', $type ) );
@@ -593,7 +593,7 @@ LUA;
                                }
                        }
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
 
                return $sizes;
@@ -623,7 +623,7 @@ LUA;
 
                        return $job;
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
        }
 
@@ -707,7 +707,7 @@ LUA;
                                JobQueue::incrStats( 'job-abandon', $this->type, $abandoned );
                        }
                } catch ( RedisException $e ) {
-                       $this->throwRedisException( $this->server, $conn, $e );
+                       $this->throwRedisException( $conn, $e );
                }
 
                return $count;
@@ -822,13 +822,12 @@ LUA;
        }
 
        /**
-        * @param $server string
         * @param $conn RedisConnRef
         * @param $e RedisException
         * @throws JobQueueError
         */
-       protected function throwRedisException( $server, RedisConnRef $conn, $e ) {
-               $this->redisPool->handleException( $server, $conn, $e );
+       protected function throwRedisException( RedisConnRef $conn, $e ) {
+               $this->redisPool->handleError( $conn, $e );
                throw new JobQueueError( "Redis server error: {$e->getMessage()}\n" );
        }
 

diff --git a/includes/job/aggregator/JobQueueAggregatorRedis.php b/includes/job/aggregator/JobQueueAggregatorRedis.php
index c654933..2aec3c9 100644 (file)
--- a/includes/job/aggregator/JobQueueAggregatorRedis.php
+++ b/includes/job/aggregator/JobQueueAggregatorRedis.php
@@ -175,7 +175,7 @@ class JobQueueAggregatorRedis extends JobQueueAggregator {
         * @return void
         */
        protected function handleException( RedisConnRef $conn, $e ) {
-               $this->redisPool->handleException( $conn->getServer(), $conn, $e );
+               $this->redisPool->handleError( $conn, $e );
        }
 
        /**

diff --git a/includes/objectcache/RedisBagOStuff.php b/includes/objectcache/RedisBagOStuff.php
index 56f2128..427143c 100644 (file)
--- a/includes/objectcache/RedisBagOStuff.php
+++ b/includes/objectcache/RedisBagOStuff.php
@@ -84,7 +84,7 @@ class RedisBagOStuff extends BagOStuff {
                        $result = $this->unserialize( $value );
                } catch ( RedisException $e ) {
                        $result = false;
-                       $this->handleException( $server, $conn, $e );
+                       $this->handleException( $conn, $e );
                }
 
                $this->logRequest( 'get', $key, $server, $result );
@@ -108,7 +108,7 @@ class RedisBagOStuff extends BagOStuff {
                        }
                } catch ( RedisException $e ) {
                        $result = false;
-                       $this->handleException( $server, $conn, $e );
+                       $this->handleException( $conn, $e );
                }
 
                $this->logRequest( 'set', $key, $server, $result );
@@ -142,7 +142,7 @@ class RedisBagOStuff extends BagOStuff {
                        $result = ( $conn->exec() == array( true ) );
                } catch ( RedisException $e ) {
                        $result = false;
-                       $this->handleException( $server, $conn, $e );
+                       $this->handleException( $conn, $e );
                }
 
                $this->logRequest( 'cas', $key, $server, $result );
@@ -162,7 +162,7 @@ class RedisBagOStuff extends BagOStuff {
                        $result = true;
                } catch ( RedisException $e ) {
                        $result = false;
-                       $this->handleException( $server, $conn, $e );
+                       $this->handleException( $conn, $e );
                }
 
                $this->logRequest( 'delete', $key, $server, $result );
@@ -201,7 +201,7 @@ class RedisBagOStuff extends BagOStuff {
                                        }
                                }
                        } catch ( RedisException $e ) {
-                               $this->handleException( $server, $conn, $e );
+                               $this->handleException( $conn, $e );
                        }
                }
 
@@ -229,7 +229,7 @@ class RedisBagOStuff extends BagOStuff {
                        }
                } catch ( RedisException $e ) {
                        $result = false;
-                       $this->handleException( $server, $conn, $e );
+                       $this->handleException( $conn, $e );
                }
 
                $this->logRequest( 'add', $key, $server, $result );
@@ -260,7 +260,7 @@ class RedisBagOStuff extends BagOStuff {
                        }
                } catch ( RedisException $e ) {
                        $result = false;
-                       $this->handleException( $server, $conn, $e );
+                       $this->handleException( $conn, $e );
                }
 
                $this->logRequest( 'replace', $key, $server, $result );
@@ -290,7 +290,7 @@ class RedisBagOStuff extends BagOStuff {
                        $result = $this->unserialize( $conn->incrBy( $key, $value ) );
                } catch ( RedisException $e ) {
                        $result = false;
-                       $this->handleException( $server, $conn, $e );
+                       $this->handleException( $conn, $e );
                }
 
                $this->logRequest( 'incr', $key, $server, $result );
@@ -352,8 +352,8 @@ class RedisBagOStuff extends BagOStuff {
         * not. The safest response for us is to explicitly destroy the connection
         * object and let it be reopened during the next request.
         */
-       protected function handleException( $server, RedisConnRef $conn, $e ) {
-               $this->redisPool->handleException( $server, $conn, $e );
+       protected function handleException( RedisConnRef $conn, $e ) {
+               $this->redisPool->handleError( $conn, $e );
        }
 
        /**