From 73b928750ca92504c4a9e5841485a0abe6716d30 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 18 Nov 2017 12:18:01 -0800 Subject: [PATCH] objectcache: Run preemptive WAN cache refreshes post-send This keeps HTTP request time consistent in case of expensive keys Change-Id: I0746fde29a6e2f27d1b92f1af599c741d5972f46 --- includes/libs/objectcache/WANObjectCache.php | 33 +++++++++++-- includes/objectcache/ObjectCache.php | 6 +++ .../libs/objectcache/WANObjectCacheTest.php | 49 +++++++++++++++++-- 3 files changed, 79 insertions(+), 9 deletions(-) diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index 74ec7b941f..3e66f3c82c 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -93,6 +93,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { protected $stats; /** @var bool Whether to use "interim" caching while keys are tombstoned */ protected $useInterimHoldOffCaching = true; + /** @var callable|null Function that takes a WAN cache callback and runs it later */ + protected $asyncHandler; /** @var int ERR_* constant for the "last error" registry */ protected $lastRelayError = self::ERR_NONE; @@ -191,6 +193,13 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * - relayers : Map of (action => EventRelayer object). Actions include "purge". * - logger : LoggerInterface object * - stats : LoggerInterface object + * - asyncHandler : A function that takes a callback and runs it later. If supplied, + * whenever a preemptive refresh would be triggered in getWithSetCallback(), the + * current cache value is still used instead. However, the async-handler function + * receives a WAN cache callback that, when run, will execute the value generation + * callback supplied by the getWithSetCallback() caller. The result will be saved + * as normal. The handler is expected to call the WAN cache callback at an opportune + * time (e.g. HTTP post-send), though generally within a few 100ms. [optional] */ public function __construct( array $params ) { $this->cache = $params['cache']; @@ -202,6 +211,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { : new EventRelayerNull( [] ); $this->setLogger( isset( $params['logger'] ) ? $params['logger'] : new NullLogger() ); $this->stats = isset( $params['stats'] ) ? $params['stats'] : new NullStatsdDataFactory(); + $this->asyncHandler = isset( $params['asyncHandler'] ) ? $params['asyncHandler'] : null; } public function setLogger( LoggerInterface $logger ) { @@ -1002,12 +1012,27 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { if ( $value !== false && $this->isAliveOrInGracePeriod( $curTTL, $graceTTL ) && $this->isValid( $value, $versioned, $asOf, $minTime ) - && !$this->worthRefreshExpiring( $curTTL, $lowTTL ) - && !$this->worthRefreshPopular( $asOf, $ageNew, $popWindow, $preCallbackTime ) ) { - $this->stats->increment( "wanobjectcache.$kClass.hit.good" ); + $preemptiveRefresh = ( + $this->worthRefreshExpiring( $curTTL, $lowTTL ) || + $this->worthRefreshPopular( $asOf, $ageNew, $popWindow, $preCallbackTime ) + ); - return $value; + if ( !$preemptiveRefresh ) { + $this->stats->increment( "wanobjectcache.$kClass.hit.good" ); + + return $value; + } elseif ( $this->asyncHandler ) { + // Update the cache value later, such during post-send of an HTTP request + $func = $this->asyncHandler; + $func( function () use ( $key, $ttl, $callback, $opts, $asOf ) { + $opts['minAsOf'] = INF; // force a refresh + $this->doGetWithSetCallback( $key, $ttl, $callback, $opts, $asOf ); + } ); + $this->stats->increment( "wanobjectcache.$kClass.hit.refresh" ); + + return $value; + } } // A deleted key with a negative TTL left must be tombstoned diff --git a/includes/objectcache/ObjectCache.php b/includes/objectcache/ObjectCache.php index 6d6d3459d9..07432c0b24 100644 --- a/includes/objectcache/ObjectCache.php +++ b/includes/objectcache/ObjectCache.php @@ -332,6 +332,8 @@ class ObjectCache { * @throws UnexpectedValueException */ public static function newWANCacheFromParams( array $params ) { + global $wgCommandLineMode; + $services = MediaWikiServices::getInstance(); $erGroup = $services->getEventRelayerGroup(); @@ -346,6 +348,10 @@ class ObjectCache { } else { $params['logger'] = LoggerFactory::getInstance( 'objectcache' ); } + // Let pre-emptive refreshes happen post-send on HTTP requests + if ( !$wgCommandLineMode ) { + $params['asyncHandler'] = [ DeferredUpdates::class, 'addCallableUpdate' ]; + } $class = $params['class']; return new $class( $params ); diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index c2be911f92..f586d03353 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -370,15 +370,15 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { public function testPreemtiveRefresh() { $value = 'KatCafe'; $wasSet = 0; - $func = function ( $old, &$ttl, &$opts, $asOf ) use ( &$wasSet, $value ) + $func = function ( $old, &$ttl, &$opts, $asOf ) use ( &$wasSet, &$value ) { ++$wasSet; return $value; }; $cache = new NearExpiringWANObjectCache( [ - 'cache' => new HashBagOStuff(), - 'pool' => 'empty' + 'cache' => new HashBagOStuff(), + 'pool' => 'empty', ] ); $wasSet = 0; @@ -399,12 +399,50 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $v = $cache->getWithSetCallback( $key, 30, $func, $opts ); $this->assertEquals( 1, $wasSet, "Value cached" ); + $asycList = []; + $asyncHandler = function ( $callback ) use ( &$asycList ) { + $asycList[] = $callback; + }; + $cache = new NearExpiringWANObjectCache( [ + 'cache' => new TimeAdjustableHashBagOStuff(), + 'pool' => 'empty', + 'asyncHandler' => $asyncHandler + ] ); + + $now = microtime( true ); // reference time + $cache->setTime( $now ); + + $wasSet = 0; + $key = wfRandomString(); + $checkKey = wfRandomString(); + $opts = [ 'lowTTL' => 100 ]; + $v = $cache->getWithSetCallback( $key, 300, $func, $opts ); + $this->assertEquals( $value, $v, "Value returned" ); + $this->assertEquals( 1, $wasSet, "Value calculated" ); + $v = $cache->getWithSetCallback( $key, 300, $func, $opts ); + $this->assertEquals( 1, $wasSet, "Cached value used" ); + $this->assertEquals( $v, $value, "Value cached" ); + + $cache->setTime( $now + 250 ); + $v = $cache->getWithSetCallback( $key, 300, $func, $opts ); + $this->assertEquals( $value, $v, "Value returned" ); + $this->assertEquals( 1, $wasSet, "Stale value used" ); + $this->assertEquals( 1, count( $asycList ), "Refresh deferred." ); + $value = 'NewCatsInTown'; // change callback return value + $asycList[0](); // run the refresh callback + $asycList = []; + $this->assertEquals( 2, $wasSet, "Value calculated at later time" ); + $this->assertEquals( 0, count( $asycList ), "No deferred refreshes added." ); + $v = $cache->getWithSetCallback( $key, 300, $func, $opts ); + $this->assertEquals( $value, $v, "New value stored" ); + $cache = new PopularityRefreshingWANObjectCache( [ - 'cache' => new HashBagOStuff(), + 'cache' => new TimeAdjustableHashBagOStuff(), 'pool' => 'empty' ] ); - $now = microtime( true ); // reference time + $cache->setTime( $now ); + $wasSet = 0; $key = wfRandomString(); $opts = [ 'hotTTR' => 900 ]; @@ -424,6 +462,7 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertEquals( 1, $wasSet, "Value calculated" ); $cache->setTime( $now + 30 ); $v = $cache->getWithSetCallback( $key, 60, $func, $opts ); + $this->assertEquals( $value, $v, "Value returned" ); $this->assertEquals( 2, $wasSet, "Value re-calculated" ); } -- 2.20.1