Ori.livneh has submitted this change and it was merged.
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 accept 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(-)
Approvals:
Ori.livneh: Looks good to me, approved
jenkins-bot: Verified
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: merged
Gerrit-Change-Id: Idea083b843f7eb558d2daf249deea853c9ec43ae
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Ori.livneh <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
