Fixes to RedisBagOStuff

* Avoid serializing scalars like the memcached client does.
  This means incr/decr can be fast and preserve the TTL.
* Properly call unwatch() in the cas() method.
* Made the expiry set on add() actually atomic.
* Also cleaned up the profiling calls.

bug: 56069
Change-Id: I3e0d1c4888062544c54aef32085f8ce608ff5423
This commit is contained in:
Aaron Schulz 2013-11-21 10:22:48 -08:00
parent 9d3c176743
commit 2ceda41c57

View file

@ -55,7 +55,7 @@ class RedisBagOStuff extends BagOStuff {
* flap, for example if it is in swap death.
*/
function __construct( $params ) {
$redisConf = array( 'serializer' => 'php' );
$redisConf = array( 'serializer' => 'none' ); // manage that in this class
foreach ( array( 'connectTimeout', 'persistent', 'password' ) as $opt ) {
if ( isset( $params[$opt] ) ) {
$redisConf[$opt] = $params[$opt];
@ -72,38 +72,38 @@ class RedisBagOStuff extends BagOStuff {
}
public function get( $key, &$casToken = null ) {
wfProfileIn( __METHOD__ );
$section = new ProfileSection( __METHOD__ );
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
wfProfileOut( __METHOD__ );
return false;
}
try {
$result = $conn->get( $key );
$result = $this->unserialize( $conn->get( $key ) );
} catch ( RedisException $e ) {
$result = false;
$this->handleException( $server, $conn, $e );
}
$casToken = $result;
$this->logRequest( 'get', $key, $server, $result );
wfProfileOut( __METHOD__ );
return $result;
}
public function set( $key, $value, $expiry = 0 ) {
wfProfileIn( __METHOD__ );
$section = new ProfileSection( __METHOD__ );
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
wfProfileOut( __METHOD__ );
return false;
}
$expiry = $this->convertToRelative( $expiry );
try {
if ( !$expiry ) {
// No expiry, that is very different from zero expiry in Redis
$result = $conn->set( $key, $value );
if ( $expiry ) {
$result = $conn->setex( $key, $expiry, $this->serialize( $value ) );
} else {
$result = $conn->setex( $key, $expiry, $value );
// No expiry, that is very different from zero expiry in Redis
$result = $conn->set( $key, $this->serialize( $value ) );
}
} catch ( RedisException $e ) {
$result = false;
@ -111,15 +111,14 @@ class RedisBagOStuff extends BagOStuff {
}
$this->logRequest( 'set', $key, $server, $result );
wfProfileOut( __METHOD__ );
return $result;
}
public function cas( $casToken, $key, $value, $expiry = 0 ) {
wfProfileIn( __METHOD__ );
$section = new ProfileSection( __METHOD__ );
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
wfProfileOut( __METHOD__ );
return false;
}
$expiry = $this->convertToRelative( $expiry );
@ -127,25 +126,18 @@ class RedisBagOStuff extends BagOStuff {
$conn->watch( $key );
if ( $this->get( $key ) !== $casToken ) {
wfProfileOut( __METHOD__ );
$conn->unwatch();
return false;
}
// multi()/exec() will fail atomically if the key changed since watch()
$conn->multi();
if ( !$expiry ) {
// No expiry, that is very different from zero expiry in Redis
$conn->set( $key, $value );
if ( $expiry ) {
$conn->setex( $key, $expiry, $this->serialize( $value ) );
} else {
$conn->setex( $key, $expiry, $value );
// No expiry, that is very different from zero expiry in Redis
$conn->set( $key, $this->serialize( $value ) );
}
/*
* multi()/exec() (transactional mode) allows multiple values to
* be set/get at once and will return an array of results, in
* the order they were set/get. In this case, we only set 1
* value, which should (in case of success) result in true.
*/
$result = ( $conn->exec() == array( true ) );
} catch ( RedisException $e ) {
$result = false;
@ -153,15 +145,14 @@ class RedisBagOStuff extends BagOStuff {
}
$this->logRequest( 'cas', $key, $server, $result );
wfProfileOut( __METHOD__ );
return $result;
}
public function delete( $key, $time = 0 ) {
wfProfileIn( __METHOD__ );
$section = new ProfileSection( __METHOD__ );
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
wfProfileOut( __METHOD__ );
return false;
}
try {
@ -172,13 +163,14 @@ class RedisBagOStuff extends BagOStuff {
$result = false;
$this->handleException( $server, $conn, $e );
}
$this->logRequest( 'delete', $key, $server, $result );
wfProfileOut( __METHOD__ );
return $result;
}
public function getMulti( array $keys ) {
wfProfileIn( __METHOD__ );
$section = new ProfileSection( __METHOD__ );
$batches = array();
$conns = array();
foreach ( $keys as $key ) {
@ -204,7 +196,7 @@ class RedisBagOStuff extends BagOStuff {
}
foreach ( $batchResult as $i => $value ) {
if ( $value !== false ) {
$result[$batchKeys[$i]] = $value;
$result[$batchKeys[$i]] = $this->unserialize( $value );
}
}
} catch ( RedisException $e ) {
@ -214,29 +206,32 @@ class RedisBagOStuff extends BagOStuff {
$this->debug( "getMulti for " . count( $keys ) . " keys " .
"returned " . count( $result ) . " results" );
wfProfileOut( __METHOD__ );
return $result;
}
public function add( $key, $value, $expiry = 0 ) {
wfProfileIn( __METHOD__ );
$section = new ProfileSection( __METHOD__ );
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
wfProfileOut( __METHOD__ );
return false;
}
$expiry = $this->convertToRelative( $expiry );
try {
$result = $conn->setnx( $key, $value );
if ( $result && $expiry ) {
if ( $expiry ) {
$conn->multi();
$conn->setnx( $key, $this->serialize( $value ) );
$conn->expire( $key, $expiry );
$result = ( $conn->exec() == array( true, true ) );
} else {
$result = $conn->setnx( $key, $this->serialize( $value ) );
}
} catch ( RedisException $e ) {
$result = false;
$this->handleException( $server, $conn, $e );
}
$this->logRequest( 'add', $key, $server, $result );
wfProfileOut( __METHOD__ );
return $result;
}
@ -245,23 +240,22 @@ class RedisBagOStuff extends BagOStuff {
* with WATCH or scripting, but this function is rarely used.
*/
public function replace( $key, $value, $expiry = 0 ) {
wfProfileIn( __METHOD__ );
$section = new ProfileSection( __METHOD__ );
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
wfProfileOut( __METHOD__ );
return false;
}
if ( !$conn->exists( $key ) ) {
wfProfileOut( __METHOD__ );
return false;
}
$expiry = $this->convertToRelative( $expiry );
try {
if ( !$expiry ) {
$result = $conn->set( $key, $value );
$result = $conn->set( $key, $this->serialize( $value ) );
} else {
$result = $conn->setex( $key, $expiry, $value );
$result = $conn->setex( $key, $expiry, $this->serialize( $value ) );
}
} catch ( RedisException $e ) {
$result = false;
@ -269,10 +263,57 @@ class RedisBagOStuff extends BagOStuff {
}
$this->logRequest( 'replace', $key, $server, $result );
wfProfileOut( __METHOD__ );
return $result;
}
/**
* Non-atomic implementation of incr().
*
* Probably all callers actually want incr() to atomically initialise
* values to zero if they don't exist, as provided by the Redis INCR
* command. But we are constrained by the memcached-like interface to
* return null in that case. Once the key exists, further increments are
* atomic.
*/
public function incr( $key, $value = 1 ) {
$section = new ProfileSection( __METHOD__ );
list( $server, $conn ) = $this->getConnection( $key );
if ( !$conn ) {
return false;
}
if ( !$conn->exists( $key ) ) {
return null;
}
try {
$result = $this->unserialize( $conn->incrBy( $key, $value ) );
} catch ( RedisException $e ) {
$result = false;
$this->handleException( $server, $conn, $e );
}
$this->logRequest( 'incr', $key, $server, $result );
return $result;
}
/**
* @param mixed $data
* @return string
*/
protected function serialize( $data ) {
// Ignore digit strings and ints so INCR/DECR work
return ( is_int( $data ) || ctype_digit( $data ) ) ? $data : serialize( $data );
}
/**
* @param string $data
* @return mixed
*/
protected function unserialize( $data ) {
// Ignore digit strings and ints so INCR/DECR work
return ( is_int( $data ) || ctype_digit( $data ) ) ? $data : unserialize( $data );
}
/**
* Get a Redis object with a connection suitable for fetching the specified key
* @return Array (server, RedisConnRef) or (false, false)