objectcache: Run preemptive WAN cache refreshes post-send

This keeps HTTP request time consistent in case of expensive keys

Change-Id: I0746fde29a6e2f27d1b92f1af599c741d5972f46

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index 74ec7b9..3e66f3c 100644 (file)
--- 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 6d6d345..07432c0 100644 (file)
--- 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 c2be911..f586d03 100644 (file)
--- 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" );
        }