Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

objectcache: avoid using process cache in nested callbacks

Browse source 
Because the process cache can be lagged by virtue of blind TTL,
the HOLDOFF_TTL might not be enough to account for it, so avoid
using it when already inside a callback.

Also split of the tests from the MediaWiki test class, so this
does not require DB access anymore.

Change-Id: I743a1233a5efc7f036fad140a9ff8a30b32f8f27
This commit is contained in:
Aaron Schulz 2016-10-20 14:47:03 -07:00
parent 6d9704caef
commit 43ff2a83b5
2 changed files with 33 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
includes/libs/objectcache/WANObjectCache.php
View file

@ -88,6 +88,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
/** @var int ERR_* constant for the "last error" registry */
protected $lastRelayError = self::ERR_NONE;
/** @var integer Callback stack depth for getWithSetCallback() */
private $callbackDepth = 0;
/** @var mixed[] Temporary warm-up cache */
private $warmupCache = [];
@ -847,8 +849,10 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
final public function getWithSetCallback( $key, $ttl, $callback, array $opts = [] ) {
$pcTTL = isset( $opts['pcTTL'] ) ? $opts['pcTTL'] : self::TTL_UNCACHEABLE;
// Try the process cache if enabled
if ( $pcTTL >= 0 ) {
// Try the process cache if enabled and the cache callback is not within a cache callback.
// Process cache use in nested callbacks is not lag-safe with regard to HOLDOFF_TTL since
// the in-memory value is further lagged than the shared one since it uses a blind TTL.
if ( $pcTTL >= 0 && $this->callbackDepth == 0 ) {
$group = isset( $opts['pcGroup'] ) ? $opts['pcGroup'] : self::PC_PRIMARY;
$procCache = $this->getProcessCache( $group );
$value = $procCache->get( $key );
@ -995,7 +999,12 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
// Generate the new value from the callback...
$setOpts = [];
$value = call_user_func_array( $callback, [ $cValue, &$ttl, &$setOpts, $asOf ] );
++$this->callbackDepth;
try {
$value = call_user_func_array( $callback, [ $cValue, &$ttl, &$setOpts, $asOf ] );
} finally {
--$this->callbackDepth;
}
// When delete() is called, writes are write-holed by the tombstone,
// so use a special INTERIM key to pass the new value around threads.
if ( ( $isTombstone && $lockTSE > 0 ) && $value !== false && $ttl >= 0 ) {

35
tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
View file

@ -1,6 +1,6 @@
<?php
class WANObjectCacheTest extends MediaWikiTestCase {
class WANObjectCacheTest extends PHPUnit_Framework_TestCase {
/** @var WANObjectCache */
private $cache;
/**@var BagOStuff */
@ -9,17 +9,11 @@ class WANObjectCacheTest extends MediaWikiTestCase {
protected function setUp() {
parent::setUp();
if ( $this->getCliArg( 'use-wanobjectcache' ) ) {
$name = $this->getCliArg( 'use-wanobjectcache' );
$this->cache = ObjectCache::getWANInstance( $name );
} else {
$this->cache = new WANObjectCache( [
'cache' => new HashBagOStuff(),
'pool' => 'testcache-hash',
'relayer' => new EventRelayerNull( [] )
] );
}
$this->cache = new WANObjectCache( [
'cache' => new HashBagOStuff(),
'pool' => 'testcache-hash',
'relayer' => new EventRelayerNull( [] )
] );
$wanCache = TestingAccessWrapper::newFromObject( $this->cache );
/** @noinspection PhpUndefinedFieldInspection */
@ -147,6 +141,19 @@ class WANObjectCacheTest extends MediaWikiTestCase {
$key, 100, $callback, [ 'pcTTL' => 5, 'pcGroup' => $groups[$i] ] );
}
$this->assertEquals( 9, $hit, "Values evicted" );
$key = reset( $keys );
// Get into cache
$this->cache->getWithSetCallback( $key, 100, $callback, [ 'pcTTL' => 5 ] );
$this->cache->getWithSetCallback( $key, 100, $callback, [ 'pcTTL' => 5 ] );
$this->assertEquals( 10, $hit, "Value cached" );
$outerCallback = function () use ( &$callback, $key ) {
$v = $this->cache->getWithSetCallback( $key, 100, $callback, [ 'pcTTL' => 5 ] );
return 43 + $v;
};
$this->cache->getWithSetCallback( $key, 100, $outerCallback );
$this->assertEquals( 11, $hit, "Nested callback value process cache skipped" );
}
/**
@ -206,7 +213,7 @@ class WANObjectCacheTest extends MediaWikiTestCase {
$this->assertEquals( $value, $v, "Value returned" );
$this->assertEquals( 1, $wasSet, "Value regenerated due to check keys" );
$this->assertEquals( $value, $priorValue, "Has prior value" );
$this->assertType( 'float', $priorAsOf, "Has prior value" );
$this->assertInternalType( 'float', $priorAsOf, "Has prior value" );
$t1 = $cache->getCheckKeyTime( $cKey1 );
$this->assertGreaterThanOrEqual( $priorTime, $t1, 'Check keys generated on miss' );
$t2 = $cache->getCheckKeyTime( $cKey2 );
@ -316,7 +323,7 @@ class WANObjectCacheTest extends MediaWikiTestCase {
$this->assertEquals( $value, $v[$keyB], "Value returned" );
$this->assertEquals( 1, $wasSet, "Value regenerated due to check keys" );
$this->assertEquals( $value, $priorValue, "Has prior value" );
$this->assertType( 'float', $priorAsOf, "Has prior value" );
$this->assertInternalType( 'float', $priorAsOf, "Has prior value" );
$t1 = $cache->getCheckKeyTime( $cKey1 );
$this->assertGreaterThanOrEqual( $priorTime, $t1, 'Check keys generated on miss' );
$t2 = $cache->getCheckKeyTime( $cKey2 );