Expanded use of reauthenticateConnection() beyond just Lua calls

* Also made it so luaEval() does not clear any previous errors to callers.
  None of the native methods have that behavoir, so this is more consistent.

Bug: 56886
Change-Id: I47d0f52e72b35ec5cb7b92b9cc3488f145b2d7a2

diff --git a/includes/clientpool/RedisConnectionPool.php b/includes/clientpool/RedisConnectionPool.php
index c8e98a7..6cf7376 100644 (file)
--- a/includes/clientpool/RedisConnectionPool.php
+++ b/includes/clientpool/RedisConnectionPool.php
@@ -285,9 +285,16 @@ class RedisConnectionPool {
        }
 
        /**
-        * Resend an AUTH request to the redis server (useful after disconnects)
+        * Re-send an AUTH request to the redis server (useful after disconnects).
         *
-        * This method is for internal use only
+        * This works around an upstream bug in phpredis. phpredis hides disconnects by transparently
+        * reconnecting, but it neglects to re-authenticate the new connection. To the user of the
+        * phpredis client API this manifests as a seemingly random tendency of connections to lose
+        * their authentication status.
+        *
+        * This method is for internal use only.
+        *
+        * @see https://github.com/nicolasff/phpredis/issues/403
         *
         * @param string $server
         * @param Redis $conn
@@ -317,6 +324,7 @@ class RedisConnRef {
        protected $conn;
 
        protected $server; // string
+       protected $lastError; // string
 
        /**
         * @param $pool RedisConnectionPool
@@ -329,8 +337,29 @@ class RedisConnRef {
                $this->conn = $conn;
        }
 
+       public function getLastError() {
+               return $this->lastError;
+       }
+
+       public function clearLastError() {
+               $this->lastError = null;
+       }
+
        public function __call( $name, $arguments ) {
-               return call_user_func_array( array( $this->conn, $name ), $arguments );
+               $conn = $this->conn; // convenience
+
+               $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();
+                       $res = call_user_func_array( array( $conn, $name ), $arguments );
+                       wfDebugLog( 'redis', "Used automatic re-authentication for method '$name'." );
+               }
+
+               $this->lastError = $conn->getLastError() ?: $this->lastError;
+
+               return $res;
        }
 
        /**
@@ -369,6 +398,8 @@ class RedisConnRef {
                        wfDebugLog( 'redis', "Lua script error on server $server: " . $conn->getLastError() );
                }
 
+               $this->lastError = $conn->getLastError() ?: $this->lastError;
+
                return $res;
        }