From 968e28f8943699369601b395260e7a84ae142a14 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 7 Oct 2015 01:55:39 -0700 Subject: [PATCH] Add process cache support to WANObjectCache * Make getWithSetCallback() accept a TTL field for this * Make ChangeTag callers use this flag to avoid hundreds of duplicate queries at Special:Tags Change-Id: Ic1ed28294f5d557e02f39a7f20d36766244b9ded --- includes/changetags/ChangeTags.php | 23 ++++------ includes/libs/objectcache/WANObjectCache.php | 43 ++++++++++++++++++- .../objectcache/WANObjectCacheTest.php | 21 ++++++--- 3 files changed, 66 insertions(+), 21 deletions(-) diff --git a/includes/changetags/ChangeTags.php b/includes/changetags/ChangeTags.php index 55fc104083..5c70c995a2 100644 --- a/includes/changetags/ChangeTags.php +++ b/includes/changetags/ChangeTags.php @@ -1101,7 +1101,8 @@ class ChangeTags { }, array( 'checkKeys' => array( wfMemcKey( 'active-tags' ) ), - 'lockTSE' => INF + 'lockTSE' => INF, + 'pcTTL' => 30 ) ); } @@ -1146,7 +1147,8 @@ class ChangeTags { }, array( 'checkKeys' => array( wfMemcKey( 'valid-tags-db' ) ), - 'lockTSE' => INF + 'lockTSE' => INF, + 'pcTTL' => 30 ) ); } @@ -1173,7 +1175,8 @@ class ChangeTags { }, array( 'checkKeys' => array( wfMemcKey( 'valid-tags-hook' ) ), - 'lockTSE' => INF + 'lockTSE' => INF, + 'pcTTL' => 30 ) ); } @@ -1214,15 +1217,8 @@ class ChangeTags { * @return array Array of string => int */ public static function tagUsageStatistics() { - static $cachedStats = null; - - // Process cache to avoid I/O and repeated regens during holdoff - if ( $cachedStats !== null ) { - return $cachedStats; - } - $fname = __METHOD__; - $cachedStats = ObjectCache::getMainWANInstance()->getWithSetCallback( + return ObjectCache::getMainWANInstance()->getWithSetCallback( wfMemcKey( 'change-tag-statistics' ), 300, function ( $oldValue, &$ttl, array &$setOpts ) use ( $fname ) { @@ -1247,11 +1243,10 @@ class ChangeTags { }, array( 'checkKeys' => array( wfMemcKey( 'change-tag-statistics' ) ), - 'lockTSE' => INF + 'lockTSE' => INF, + 'pcTTL' => 30 ) ); - - return $cachedStats; } /** diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index 621ad42426..b0cb77513c 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -60,6 +60,8 @@ class WANObjectCache { /** @var BagOStuff The local datacenter cache */ protected $cache; + /** @var HashBagOStuff Script instance PHP cache */ + protected $procCache; /** @var string Cache pool name */ protected $pool; /** @var EventRelayer */ @@ -127,6 +129,7 @@ class WANObjectCache { $this->cache = $params['cache']; $this->pool = $params['pool']; $this->relayer = $params['relayer']; + $this->procCache = new HashBagOStuff(); } /** @@ -638,7 +641,15 @@ class WANObjectCache { * expiration is low, the assumption is that the key is hot and that a stampede is worth * avoiding. Setting this above WANObjectCache::HOLDOFF_TTL makes no difference. The * higher this is set, the higher the worst-case staleness can be. - * Use WANObjectCache::TSE_NONE to disable this logic. Default: WANObjectCache::TSE_NONE. + * Use WANObjectCache::TSE_NONE to disable this logic. + * Default: WANObjectCache::TSE_NONE. + * - pcTTL : process cache the value in this PHP instance with this TTL. This avoids + * network I/O when a key is read several times. This will not cache if the callback + * returns false however. Note that any purges will not be seen while process cached; + * since the callback should use slave DBs and they may be lagged or have snapshot + * isolation anyway, this should not matter much + * Default: WANObjectCache::TTL_UNCACHEABLE. + * @param array $oldOpts Unused (mentioned only to avoid PHPDoc warnings) * @return mixed Value to use for the key */ final public function getWithSetCallback( @@ -658,6 +669,36 @@ class WANObjectCache { $checkKeys = isset( $opts['checkKeys'] ) ? $opts['checkKeys'] : array(); } + $pcTTL = isset( $opts['pcTTL'] ) ? $opts['pcTTL'] : self::TTL_UNCACHEABLE; + + // Try the process cache if enabled + $value = ( $pcTTL >= 0 ) ? $this->procCache->get( $key ) : false; + + if ( $value === false ) { + // Fetch the value over the network + $value = $this->doGetWithSetCallback( $key, $ttl, $callback, $checkKeys, $opts ); + // Update the process cache if enabled + if ( $pcTTL >= 0 && $value !== false ) { + $this->procCache->set( $key, $value, $pcTTL ); + } + } + + return $value; + } + + /** + * @see WANObjectCache::getWithSetCallback() + * + * @param string $key + * @param integer $ttl + * @param callback $callback + * @param array $checkKeys + * @param array $opts + * @return mixed + */ + protected function doGetWithSetCallback( + $key, $ttl, $callback, array $checkKeys, array $opts + ) { $lowTTL = isset( $opts['lowTTL'] ) ? $opts['lowTTL'] : min( self::LOW_TTL, $ttl ); $lockTSE = isset( $opts['lockTSE'] ) ? $opts['lockTSE'] : self::TSE_NONE; diff --git a/tests/phpunit/includes/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/objectcache/WANObjectCacheTest.php index 4195216518..8981f2fb1a 100644 --- a/tests/phpunit/includes/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/objectcache/WANObjectCacheTest.php @@ -105,11 +105,11 @@ class WANObjectCacheTest extends MediaWikiTestCase { $wasSet = 0; $v = $cache->getWithSetCallback( $key, $func, 30, array(), array( 'lockTSE' => 5 ) ); - $this->assertEquals( $v, $value ); + $this->assertEquals( $value, $v, "Value returned" ); $this->assertEquals( 1, $wasSet, "Value regenerated" ); $curTTL = null; - $v = $cache->get( $key, $curTTL ); + $cache->get( $key, $curTTL ); $this->assertLessThanOrEqual( 20, $curTTL, 'Current TTL between 19-20 (overriden)' ); $this->assertGreaterThanOrEqual( 19, $curTTL, 'Current TTL between 19-20 (overriden)' ); @@ -118,14 +118,14 @@ class WANObjectCacheTest extends MediaWikiTestCase { 'lowTTL' => 0, 'lockTSE' => 5, ) ); - $this->assertEquals( $v, $value ); + $this->assertEquals( $value, $v, "Value returned" ); $this->assertEquals( 0, $wasSet, "Value not regenerated" ); $priorTime = microtime( true ); usleep( 1 ); $wasSet = 0; $v = $cache->getWithSetCallback( $key, $func, 30, array( $cKey1, $cKey2 ) ); - $this->assertEquals( $v, $value ); + $this->assertEquals( $value, $v, "Value returned" ); $this->assertEquals( 1, $wasSet, "Value regenerated due to check keys" ); $t1 = $cache->getCheckKeyTime( $cKey1 ); $this->assertGreaterThanOrEqual( $priorTime, $t1, 'Check keys generated on miss' ); @@ -135,7 +135,7 @@ class WANObjectCacheTest extends MediaWikiTestCase { $priorTime = microtime( true ); $wasSet = 0; $v = $cache->getWithSetCallback( $key, $func, 30, array( $cKey1, $cKey2 ) ); - $this->assertEquals( $v, $value ); + $this->assertEquals( $value, $v, "Value returned" ); $this->assertEquals( 1, $wasSet, "Value regenerated due to still-recent check keys" ); $t1 = $cache->getCheckKeyTime( $cKey1 ); $this->assertLessThanOrEqual( $priorTime, $t1, 'Check keys did not change again' ); @@ -144,8 +144,17 @@ class WANObjectCacheTest extends MediaWikiTestCase { $curTTL = null; $v = $cache->get( $key, $curTTL, array( $cKey1, $cKey2 ) ); - $this->assertEquals( $v, $value ); + $this->assertEquals( $value, $v, "Value returned" ); $this->assertLessThanOrEqual( 0, $curTTL, "Value has current TTL < 0 due to check keys" ); + + $wasSet = 0; + $key = wfRandomString(); + $v = $cache->getWithSetCallback( $key, $func, 30, array(), array( 'pcTTL' => 5 ) ); + $this->assertEquals( $value, $v, "Value returned" ); + $cache->delete( $key ); + $v = $cache->getWithSetCallback( $key, $func, 30, array(), array( 'pcTTL' => 5 ) ); + $this->assertEquals( $value, $v, "Value still returned after deleted" ); + $this->assertEquals( 1, $wasSet, "Value process cached while deleted" ); } /** -- 2.20.1