jenkins-bot has submitted this change and it was merged.
Change subject: Expanded use of reauthenticateConnection() beyond just Lua calls
......................................................................
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
---
M includes/clientpool/RedisConnectionPool.php
1 file changed, 34 insertions(+), 3 deletions(-)
Approvals:
Tim Starling: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/clientpool/RedisConnectionPool.php
b/includes/clientpool/RedisConnectionPool.php
index c8e98a7..6cf7376 100644
--- a/includes/clientpool/RedisConnectionPool.php
+++ b/includes/clientpool/RedisConnectionPool.php
@@ -285,9 +285,16 @@
}
/**
- * 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 @@
protected $conn;
protected $server; // string
+ protected $lastError; // string
/**
* @param $pool RedisConnectionPool
@@ -329,8 +337,29 @@
$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 @@
wfDebugLog( 'redis', "Lua script error on server
$server: " . $conn->getLastError() );
}
+ $this->lastError = $conn->getLastError() ?: $this->lastError;
+
return $res;
}
--
To view, visit https://gerrit.wikimedia.org/r/94935
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I47d0f52e72b35ec5cb7b92b9cc3488f145b2d7a2
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Ori.livneh <o...@wikimedia.org>
Gerrit-Reviewer: Tim Starling <tstarl...@wikimedia.org>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
