Aaron Schulz has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/144422

Change subject: Lowered the default OPT_READ_TIMEOUT for Redis
......................................................................

Lowered the default OPT_READ_TIMEOUT for Redis

* This should probably be low for the case of a server that is
  quick to except connections but slow to do anything.
* Made the client class handle blocking operations sanely
  by automatically boosting/lowering the timeout value.

Change-Id: Idea083b843f7eb558d2daf249deea853c9ec43ae
---
M includes/clientpool/RedisConnectionPool.php
1 file changed, 34 insertions(+), 7 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/22/144422/1

diff --git a/includes/clientpool/RedisConnectionPool.php 
b/includes/clientpool/RedisConnectionPool.php
index 36d2731..d2c504a 100644
--- a/includes/clientpool/RedisConnectionPool.php
+++ b/includes/clientpool/RedisConnectionPool.php
@@ -101,7 +101,7 @@
                        $options['connectTimeout'] = 1;
                }
                if ( !isset( $options['readTimeout'] ) ) {
-                       $options['readTimeout'] = 31; // handles up to 30 
second blocking commands
+                       $options['readTimeout'] = 1;
                }
                if ( !isset( $options['persistent'] ) ) {
                        $options['persistent'] = false;
@@ -120,7 +120,7 @@
         *                      Optional, default is 1 second.
         *   - readTimeout    : The timeout for operation reads, in seconds.
         *                      Commands like BLPOP can fail if told to wait 
longer than this.
-        *                      Optional, default is 60 seconds.
+        *                      Optional, default is 1 second.
         *   - persistent     : Set this to true to allow connections to 
persist across
         *                      multiple web requests. False by default.
         *   - password       : The authentication password, will be sent to 
Redis in clear text.
@@ -343,6 +343,16 @@
        }
 
        /**
+        * Adjust or reset the connection handle read timeout value
+        *
+        * @param Redis $conn
+        * @param integer $timeout Optional
+        */
+       public function resetTimeout( Redis $conn, $timeout = null ) {
+               $conn->setOption( Redis::OPT_READ_TIMEOUT, $timeout ?: 
$this->readTimeout );
+       }
+
+       /**
         * Make sure connections are closed for sanity
         */
        function __destruct() {
@@ -401,17 +411,34 @@
        public function __call( $name, $arguments ) {
                $conn = $this->conn; // convenience
 
+               // Work around https://github.com/nicolasff/phpredis/issues/70
+               $lname = strtolower( $name );
+               if ( ( $lname === 'blpop' || $lname == 'brpop' )
+                       && is_array( $arguments[0] ) && isset( $arguments[1] )
+               ) {
+                       $this->pool->resetTimeout( $conn, $arguments[1] + 1 );
+               } elseif ( $lname === 'brpoplpush' && isset( $arguments[2] ) ) {
+                       $this->pool->resetTimeout( $conn, $arguments[2] + 1 );
+               }
+
                $conn->clearLastError();
-               $res = call_user_func_array( array( $conn, $name ), $arguments 
);
-               if ( preg_match( '/^ERR operation not permitted\b/', 
$conn->getLastError() ) ) {
-                       $this->pool->reauthenticateConnection( $this->server, 
$conn );
-                       $conn->clearLastError();
+               try {
                        $res = call_user_func_array( array( $conn, $name ), 
$arguments );
-                       wfDebugLog( 'redis', "Used automatic re-authentication 
for method '$name'." );
+                       if ( preg_match( '/^ERR operation not permitted\b/', 
$conn->getLastError() ) ) {
+                               $this->pool->reauthenticateConnection( 
$this->server, $conn );
+                               $conn->clearLastError();
+                               $res = call_user_func_array( array( $conn, 
$name ), $arguments );
+                               wfDebugLog( 'redis', "Used automatic 
re-authentication for method '$name'." );
+                       }
+               } catch ( RedisException $e ) {
+                       $this->pool->resetTimeout( $conn ); // restore
+                       throw $e;
                }
 
                $this->lastError = $conn->getLastError() ?: $this->lastError;
 
+               $this->pool->resetTimeout( $conn ); // restore
+
                return $res;
        }
 

-- 
To view, visit https://gerrit.wikimedia.org/r/144422
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Idea083b843f7eb558d2daf249deea853c9ec43ae
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to