jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/392222 )
Change subject: objectcache: Run preemptive WAN cache refreshes post-send
......................................................................
objectcache: Run preemptive WAN cache refreshes post-send
This keeps HTTP request time consistent in case of expensive keys
Change-Id: I0746fde29a6e2f27d1b92f1af599c741d5972f46
---
M includes/libs/objectcache/WANObjectCache.php
M includes/objectcache/ObjectCache.php
M tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
3 files changed, 80 insertions(+), 10 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/libs/objectcache/WANObjectCache.php
b/includes/libs/objectcache/WANObjectCache.php
index 74ec7b9..3e66f3c 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -93,6 +93,8 @@
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 @@
* - 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 @@
: 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 @@
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
--- a/includes/objectcache/ObjectCache.php
+++ b/includes/objectcache/ObjectCache.php
@@ -332,6 +332,8 @@
* @throws UnexpectedValueException
*/
public static function newWANCacheFromParams( array $params ) {
+ global $wgCommandLineMode;
+
$services = MediaWikiServices::getInstance();
$erGroup = $services->getEventRelayerGroup();
@@ -346,6 +348,10 @@
} 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
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -370,15 +370,15 @@
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 @@
$v = $cache->getWithSetCallback( $key, 30, $func, $opts );
$this->assertEquals( 1, $wasSet, "Value cached" );
- $cache = new PopularityRefreshingWANObjectCache( [
- 'cache' => new HashBagOStuff(),
- 'pool' => 'empty'
+ $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 TimeAdjustableHashBagOStuff(),
+ 'pool' => 'empty'
+ ] );
+
+ $cache->setTime( $now );
+
$wasSet = 0;
$key = wfRandomString();
$opts = [ 'hotTTR' => 900 ];
@@ -424,6 +462,7 @@
$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" );
}
--
To view, visit https://gerrit.wikimedia.org/r/392222
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0746fde29a6e2f27d1b92f1af599c741d5972f46
Gerrit-PatchSet: 16
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
