All subclasses are now using the CAS variant, which respects $attempts.
Change-Id: Ia7ec6a7f3337cabe95c54c1142c3c5464c1794e7
* @throws InvalidArgumentException
*/
public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
- return $this->mergeViaLock( $key, $callback, $exptime, $attempts, $flags );
+ return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
}
/**
// Derive the new value from the old value
$value = call_user_func( $callback, $this, $key, $currentValue, $exptime );
+ $hadNoCurrentValue = ( $currentValue === false );
+ unset( $currentValue ); // free RAM in case the value is large
$this->clearLastError();
if ( $value === false ) {
$success = true; // do nothing
- } elseif ( $currentValue === false ) {
+ } elseif ( $hadNoCurrentValue ) {
// Try to create the key, failing if it gets created in the meantime
$success = $this->add( $key, $value, $exptime, $flags );
} else {
return $success;
}
- /**
- * @see BagOStuff::merge()
- *
- * @param string $key
- * @param callable $callback Callback method to be executed
- * @param int $exptime Either an interval in seconds or a unix timestamp for expiry
- * @param int $attempts The amount of times to attempt a merge in case of failure
- * @param int $flags Bitfield of BagOStuff::WRITE_* constants
- * @return bool Success
- */
- protected function mergeViaLock( $key, $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
- if ( $attempts <= 1 ) {
- $timeout = 0; // clearly intended to be "non-blocking"
- } else {
- $timeout = 3;
- }
-
- if ( !$this->lock( $key, $timeout ) ) {
- return false;
- }
-
- $this->clearLastError();
- $reportDupes = $this->reportDupes;
- $this->reportDupes = false;
- $currentValue = $this->get( $key, self::READ_LATEST );
- $this->reportDupes = $reportDupes;
-
- if ( $this->getLastError() ) {
- $this->logger->warning(
- __METHOD__ . ' failed due to I/O error on get() for {key}.',
- [ 'key' => $key ]
- );
-
- $success = false;
- } else {
- // Derive the new value from the old value
- $value = call_user_func( $callback, $this, $key, $currentValue, $exptime );
- if ( $value === false ) {
- $success = true; // do nothing
- } else {
- $success = $this->set( $key, $value, $exptime, $flags ); // set the new value
- }
- }
-
- if ( !$this->unlock( $key ) ) {
- // this should never happen
- trigger_error( "Could not release lock for key '$key'." );
- }
-
- return $success;
- }
-
/**
* Change the expiration on a key if it exists
*
if ( ( $flags & self::READ_LATEST ) == self::READ_LATEST ) {
// If the latest write was a delete(), we do NOT want to fallback
// to the other tiers and possibly see the old value. Also, this
- // is used by mergeViaLock(), which only needs to hit the primary.
+ // is used by merge(), which only needs to hit the primary.
return $this->caches[0]->get( $key, $flags );
}
$this->client->setLogger( $logger );
}
- /**
- * @param string $key
- * @param int $flags Bitfield of BagOStuff::READ_* constants [optional]
- * @return mixed Returns false on failure and if the item does not exist
- */
protected function doGet( $key, $flags = 0 ) {
+ $casToken = null;
+
+ return $this->getWithToken( $key, $casToken, $flags );
+ }
+
+ protected function getWithToken( $key, &$casToken, $flags = 0 ) {
+ $casToken = null;
+
$req = [
'method' => 'GET',
'url' => $this->url . rawurlencode( $key ),
list( $rcode, $rdesc, $rhdrs, $rbody, $rerr ) = $this->client->run( $req );
if ( $rcode === 200 ) {
if ( is_string( $rbody ) ) {
- return unserialize( $rbody );
+ $value = unserialize( $rbody );
+ /// @FIXME: use some kind of hash or UUID header as CAS token
+ $casToken = ( $value !== false ) ? $rbody : null;
+
+ return $value;
}
return false;
}
return false;
}
- /**
- * Handle storage error
- * @param string $msg Error message
- * @param int $rcode Error code from client
- * @param string $rerr Error message from client
- * @return false
- */
- protected function handleError( $msg, $rcode, $rerr ) {
- $this->logger->error( "$msg : ({code}) {error}", [
- 'code' => $rcode,
- 'error' => $rerr
- ] );
- $this->setLastError( $rcode === 0 ? self::ERR_UNREACHABLE : self::ERR_UNEXPECTED );
- return false;
- }
-
public function set( $key, $value, $exptime = 0, $flags = 0 ) {
// @TODO: respect WRITE_SYNC (e.g. EACH_QUORUM)
// @TODO: respect $exptime
return false;
}
+
+ public function merge( $key, callable $callback, $exptime = 0, $attempts = 10, $flags = 0 ) {
+ return $this->mergeViaCas( $key, $callback, $exptime, $attempts, $flags );
+ }
+
+ /**
+ * Handle storage error
+ * @param string $msg Error message
+ * @param int $rcode Error code from client
+ * @param string $rerr Error message from client
+ * @return false
+ */
+ protected function handleError( $msg, $rcode, $rerr ) {
+ $this->logger->error( "$msg : ({code}) {error}", [
+ 'code' => $rcode,
+ 'error' => $rerr
+ ] );
+ $this->setLastError( $rcode === 0 ? self::ERR_UNREACHABLE : self::ERR_UNEXPECTED );
+ return false;
+ }
}
/**
* @covers BagOStuff::merge
- * @covers BagOStuff::mergeViaLock
* @covers BagOStuff::mergeViaCas
*/
public function testMerge() {
$key = $this->cache->makeKey( self::TEST_KEY );
- $locks = false;
- $checkLockingCallback = function ( BagOStuff $cache, $key, $oldVal ) use ( &$locks ) {
- $locks = $cache->get( "$key:lock" );
-
- return false;
- };
-
- $this->cache->merge( $key, $checkLockingCallback, 5 );
- $this->assertFalse( $this->cache->get( $key ) );
$calls = 0;
$casRace = false; // emulate a race
$this->assertEquals( 'mergedmerged', $this->cache->get( $key ) );
$calls = 0;
- if ( $locks ) {
- // merge were something else already was merging (e.g. had the lock)
- $this->cache->lock( $key );
- $this->assertFalse(
- $this->cache->merge( $key, $callback, 5, 1 ),
- 'Non-blocking merge (locking)'
- );
- $this->cache->unlock( $key );
- $this->assertEquals( 0, $calls );
- } else {
- $casRace = true;
- $this->assertFalse(
- $this->cache->merge( $key, $callback, 5, 1 ),
- 'Non-blocking merge (CAS)'
- );
- $this->assertEquals( 1, $calls );
- }
+ $casRace = true;
+ $this->assertFalse(
+ $this->cache->merge( $key, $callback, 5, 1 ),
+ 'Non-blocking merge (CAS)'
+ );
+ $this->assertEquals( 1, $calls );
}
/**
* @covers BagOStuff::merge
- * @covers BagOStuff::mergeViaLock
* @dataProvider provideTestMerge_fork
*/
- public function testMerge_fork( $exists, $winsLocking, $resLocking, $resCAS ) {
+ public function testMerge_fork( $exists, $childWins, $resCAS ) {
$key = $this->cache->makeKey( self::TEST_KEY );
$pCallback = function ( BagOStuff $cache, $key, $oldVal ) {
return ( $oldVal === false ) ? 'init-parent' : $oldVal . '-merged-parent';
$fork &= !$this->cache instanceof MultiWriteBagOStuff;
if ( $fork ) {
$pid = null;
- $locked = false;
// Function to start merge(), run another merge() midway through, then finish
- $func = function ( BagOStuff $cache, $key, $cur )
- use ( $pCallback, $cCallback, &$pid, &$locked )
- {
+ $func = function ( $cache, $key, $cur ) use ( $pCallback, $cCallback, &$pid ) {
$pid = pcntl_fork();
if ( $pid == -1 ) {
return false;
} elseif ( $pid ) {
- $locked = $cache->get( "$key:lock" ); // parent has lock?
pcntl_wait( $status );
return $pCallback( $cache, $key, $cur );
return; // can't fork, ignore this test...
}
- if ( $locked ) {
- // merge succeed since child was locked out
- $this->assertEquals( $winsLocking, $merged );
- $this->assertEquals( $this->cache->get( $key ), $resLocking );
- } else {
- // merge has failed because child process was merging (and we only attempted once)
- $this->assertEquals( !$winsLocking, $merged );
- $this->assertEquals( $this->cache->get( $key ), $resCAS );
- }
+ // merge has failed because child process was merging (and we only attempted once)
+ $this->assertEquals( !$childWins, $merged );
+ $this->assertEquals( $this->cache->get( $key ), $resCAS );
} else {
$this->markTestSkipped( 'No pcntl methods available' );
}
function provideTestMerge_fork() {
return [
- // (already exists, parent wins if locking, result if locking, result if CAS)
- [ false, true, 'init-parent', 'init-child' ],
- [ true, true, 'x-merged-parent', 'x-merged-child' ]
+ // (already exists, child wins CAS, result of CAS)
+ [ false, true, 'init-child' ],
+ [ true, true, 'x-merged-child' ]
];
}
dépôts / lhc / web / wiklou.git / commitdiff