From: Aaron Schulz Date: Wed, 7 Sep 2016 22:36:14 +0000 (-0700) Subject: objectcache: add "pcGroup" option to WANObjectCache::getWithSetCallback() X-Git-Tag: 1.31.0-rc.0~5713^2 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/password.php?a=commitdiff_plain;h=5316e7d70061457420dc27434d9e7230bc7f031d;p=lhc%2Fweb%2Fwiklou.git objectcache: add "pcGroup" option to WANObjectCache::getWithSetCallback() This lets callers have their own process cache but keep it managed in a central location, making it easier to reset when DB transaction snapshot are flushed or when unit tests want to clear caches between runs. Use this to replace the Revision text process cache. Change-Id: Ic61ee9140d4ce9836cc4650adb5bb75a291fea18 --- diff --git a/includes/Revision.php b/includes/Revision.php index bc760a3609..ef0f03df8f 100644 --- a/includes/Revision.php +++ b/includes/Revision.php @@ -92,6 +92,8 @@ class Revision implements IDBAccessObject { const FOR_THIS_USER = 2; const RAW = 3; + const TEXT_CACHE_GROUP = 'revisiontext:10'; // process cache name and max key count + /** * Load a page revision from a given revision ID number. * Returns null if no such revision can be found. @@ -1577,31 +1579,22 @@ class Revision implements IDBAccessObject { private function loadText() { // Caching may be beneficial for massive use of external storage global $wgRevisionCacheExpiry; - static $processCache = null; if ( !$wgRevisionCacheExpiry ) { return $this->fetchText(); } - if ( !$processCache ) { - $processCache = new MapCacheLRU( 10 ); - } - $cache = ObjectCache::getMainWANInstance(); - $key = $cache->makeKey( 'revisiontext', 'textid', $this->getTextId() ); // No negative caching; negative hits on text rows may be due to corrupted replica DBs - return $processCache->getWithSetCallback( $key, function () use ( $cache, $key ) { - global $wgRevisionCacheExpiry; - - return $cache->getWithSetCallback( - $key, - $wgRevisionCacheExpiry, - function () { - return $this->fetchText(); - } - ); - } ); + return $cache->getWithSetCallback( + $key = $cache->makeKey( 'revisiontext', 'textid', $this->getTextId() ), + $wgRevisionCacheExpiry, + function () { + return $this->fetchText(); + }, + [ 'pcGroup' => self::TEXT_CACHE_GROUP, 'pcTTL' => $cache::TTL_PROC_LONG ] + ); } private function fetchText() { diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index 5563b22225..af78f430fb 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -76,8 +76,8 @@ use Psr\Log\NullLogger; class WANObjectCache implements IExpiringStore, LoggerAwareInterface { /** @var BagOStuff The local datacenter cache */ protected $cache; - /** @var HashBagOStuff Script instance PHP cache */ - protected $procCache; + /** @var HashBagOStuff[] Map of group PHP instance caches */ + protected $processCaches = []; /** @var string Purge channel name */ protected $purgeChannel; /** @var EventRelayer Bus that handles purge broadcasts */ @@ -154,7 +154,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { const VFLD_DATA = 'WOC:d'; // key to the value of versioned data const VFLD_VERSION = 'WOC:v'; // key to the version of the value present - const MAX_PC_KEYS = 1000; // max keys to keep in process cache + const PC_PRIMARY = 'primary:1000'; // process cache name and max key count const DEFAULT_PURGE_CHANNEL = 'wancache-purge'; @@ -167,7 +167,6 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { */ public function __construct( array $params ) { $this->cache = $params['cache']; - $this->procCache = new HashBagOStuff( [ 'maxKeys' => self::MAX_PC_KEYS ] ); $this->purgeChannel = isset( $params['channels']['purge'] ) ? $params['channels']['purge'] : self::DEFAULT_PURGE_CHANNEL; @@ -793,6 +792,12 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * since the callback should use replica DBs and they may be lagged or have snapshot * isolation anyway, this should not typically matter. * Default: WANObjectCache::TTL_UNCACHEABLE. + * - pcGroup: Process cache group to use instead of the primary one. If set, this must be + * of the format :. Use this for storing large values, + * small but numerous values, or a few values with a high cost if they are evicted. + * It is generally preferable to use a class constant when setting this value. + * This has no effect unless pcTTL is used. + * Default: WANObjectCache::PC_PRIMARY. * - version: Integer version number. This allows for callers to make breaking changes to * how values are stored while maintaining compatability and correct cache purges. New * versions are stored alongside older versions concurrently. Avoid storing class objects @@ -816,7 +821,14 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $pcTTL = isset( $opts['pcTTL'] ) ? $opts['pcTTL'] : self::TTL_UNCACHEABLE; // Try the process cache if enabled - $value = ( $pcTTL >= 0 ) ? $this->procCache->get( $key ) : false; + if ( $pcTTL >= 0 ) { + $group = isset( $opts['pcGroup'] ) ? $opts['pcGroup'] : self::PC_PRIMARY; + $procCache = $this->getProcessCache( $group ); + $value = $procCache->get( $key ); + } else { + $procCache = false; + $value = false; + } if ( $value === false ) { unset( $opts['minTime'] ); // not a public feature @@ -865,8 +877,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { } // Update the process cache if enabled - if ( $pcTTL >= 0 && $value !== false ) { - $this->procCache->set( $key, $value, $pcTTL ); + if ( $procCache && $value !== false ) { + $procCache->set( $key, $value, $pcTTL ); } } @@ -1050,7 +1062,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * @since 1.27 */ public function clearProcessCache() { - $this->procCache->clear(); + $this->processCaches = []; } /** @@ -1340,4 +1352,17 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { protected function makePurgeValue( $timestamp, $holdoff ) { return self::PURGE_VAL_PREFIX . (float)$timestamp . ':' . (int)$holdoff; } + + /** + * @param string $group + * @return HashBagOStuff + */ + protected function getProcessCache( $group ) { + if ( !isset( $this->processCaches[$group] ) ) { + list( , $n ) = explode( ':', $group ); + $this->processCaches[$group] = new HashBagOStuff( [ 'maxKeys' => (int)$n ] ); + } + + return $this->processCaches[$group]; + } } diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index abef758a42..cef31dc460 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -105,6 +105,47 @@ class WANObjectCacheTest extends MediaWikiTestCase { $this->assertFalse( $this->cache->get( $key ), "Stale set() value ignored" ); } + public function testProcessCache() { + $hit = 0; + $callback = function () use ( &$hit ) { + ++$hit; + return 42; + }; + $keys = [ wfRandomString(), wfRandomString(), wfRandomString() ]; + $groups = [ 'thiscache:1', 'thatcache:1', 'somecache:1' ]; + + foreach ( $keys as $i => $key ) { + $this->cache->getWithSetCallback( + $key, 100, $callback, [ 'pcTTL' => 5, 'pcGroup' => $groups[$i] ] ); + } + $this->assertEquals( 3, $hit ); + + foreach ( $keys as $i => $key ) { + $this->cache->getWithSetCallback( + $key, 100, $callback, [ 'pcTTL' => 5, 'pcGroup' => $groups[$i] ] ); + } + $this->assertEquals( 3, $hit, "Values cached" ); + + foreach ( $keys as $i => $key ) { + $this->cache->getWithSetCallback( + "$key-2", 100, $callback, [ 'pcTTL' => 5, 'pcGroup' => $groups[$i] ] ); + } + $this->assertEquals( 6, $hit ); + + foreach ( $keys as $i => $key ) { + $this->cache->getWithSetCallback( + "$key-2", 100, $callback, [ 'pcTTL' => 5, 'pcGroup' => $groups[$i] ] ); + } + $this->assertEquals( 6, $hit, "New values cached" ); + + foreach ( $keys as $i => $key ) { + $this->cache->delete( $key ); + $this->cache->getWithSetCallback( + $key, 100, $callback, [ 'pcTTL' => 5, 'pcGroup' => $groups[$i] ] ); + } + $this->assertEquals( 9, $hit, "Values evicted" ); + } + /** * @dataProvider getWithSetCallback_provider * @covers WANObjectCache::getWithSetCallback()