From e90eafdf6107d4e49c9c20f6310d1d6ee5f5c47f Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Tue, 28 Nov 2017 20:39:47 -0800 Subject: [PATCH] objectcache: Make WANObjectCache interim caching not interfere with ChronologyProtector Also removed useless line from testLockTSE(). That would have needed to be using $this->internalCache and those locks are freed immediately. Bug: T180035 Change-Id: Ida1a923f779aaf8410da76643457d2200da6cb20 --- includes/Setup.php | 15 +++-- includes/libs/objectcache/WANObjectCache.php | 30 ++++++++++ .../libs/objectcache/WANObjectCacheTest.php | 60 +++++++++++++++++-- 3 files changed, 94 insertions(+), 11 deletions(-) diff --git a/includes/Setup.php b/includes/Setup.php index 081ea68363..d6f4b2fe4c 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -736,17 +736,20 @@ if ( !$wgDBerrorLogTZ ) { // Initialize the request object in $wgRequest $wgRequest = RequestContext::getMain()->getRequest(); // BackCompat -// Set user IP/agent information for causal consistency purposes +// Set user IP/agent information for causal consistency purposes. +// The cpPosTime cookie has no prefix and is set by MediaWiki::preOutputCommit(). +$cpPosTime = $wgRequest->getFloat( 'cpPosTime', $wgRequest->getCookie( 'cpPosTime', '' ) ); MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->setRequestInfo( [ 'IPAddress' => $wgRequest->getIP(), 'UserAgent' => $wgRequest->getHeader( 'User-Agent' ), 'ChronologyProtection' => $wgRequest->getHeader( 'ChronologyProtection' ), - // The cpPosTime cookie has no prefix and is set by MediaWiki::preOutputCommit() - 'ChronologyPositionTime' => $wgRequest->getFloat( - 'cpPosTime', - $wgRequest->getCookie( 'cpPosTime', '' ) - ) + 'ChronologyPositionTime' => $cpPosTime ] ); +// Make sure that caching does not compromise the consistency improvements +if ( $cpPosTime ) { + MediaWikiServices::getInstance()->getMainWANObjectCache()->useInterimHoldOffCaching( false ); +} +unset( $cpPosTime ); // Useful debug output if ( $wgCommandLineMode ) { diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index db27e42e1e..74ec7b941f 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -91,6 +91,8 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { protected $logger; /** @var StatsdDataFactoryInterface */ protected $stats; + /** @var bool Whether to use "interim" caching while keys are tombstoned */ + protected $useInterimHoldOffCaching = true; /** @var int ERR_* constant for the "last error" registry */ protected $lastRelayError = self::ERR_NONE; @@ -1104,6 +1106,10 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { * @return mixed */ protected function getInterimValue( $key, $versioned, $minTime, &$asOf ) { + if ( !$this->useInterimHoldOffCaching ) { + return false; // disabled + } + $wrapped = $this->cache->get( self::INTERIM_KEY_PREFIX . $key ); list( $value ) = $this->unwrap( $wrapped, $this->getCurrentTime() ); if ( $value !== false && $this->isValid( $value, $versioned, $asOf, $minTime ) ) { @@ -1495,6 +1501,30 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface { $this->processCaches = []; } + /** + * Disable the use of brief caching for tombstoned keys + * + * When a key is purged via delete(), there normally is a period where caching + * is hold-off limited to an extremely short time. This method will disable that + * caching, forcing the callback to run for any of: + * - WANObjectCache::getWithSetCallback() + * - WANObjectCache::getMultiWithSetCallback() + * - WANObjectCache::getMultiWithUnionSetCallback() + * + * This is useful when both: + * - a) the database used by the callback is known to be up-to-date enough + * for some particular purpose (e.g. replica DB has applied transaction X) + * - b) the caller needs to exploit that fact, and therefore needs to avoid the + * use of inherently volatile and possibly stale interim keys + * + * @see WANObjectCache::delete() + * @param bool $enabled Whether to enable interim caching + * @since 1.31 + */ + public function useInterimHoldOffCaching( $enabled ) { + $this->useInterimHoldOffCaching = $enabled; + } + /** * @param int $flag ATTR_* class constant * @return int QOS_* class constant diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index df8228dcb0..c2be911f92 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -769,8 +769,6 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $calls = 0; $func = function () use ( &$calls, $value, $cache, $key ) { ++$calls; - // Immediately kill any mutex rather than waiting a second - $cache->delete( $cache::MUTEX_KEY_PREFIX . $key ); return $value; }; @@ -778,7 +776,7 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $this->assertEquals( $value, $ret ); $this->assertEquals( 1, $calls, 'Value was populated' ); - // Acquire a lock to verify that getWithSetCallback uses lockTSE properly + // Acquire the mutex to verify that getWithSetCallback uses lockTSE properly $this->internalCache->add( $cache::MUTEX_KEY_PREFIX . $key, 1, 0 ); $checkKeys = [ wfRandomString() ]; // new check keys => force misses @@ -795,8 +793,8 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { $ret = $cache->getWithSetCallback( $key, 30, $func, [ 'lockTSE' => 5, 'checkKeys' => $checkKeys ] ); - $this->assertEquals( $value, $ret, 'Callback was not used; used interim' ); - $this->assertEquals( 2, $calls, 'Callback was not used; used interim' ); + $this->assertEquals( $value, $ret, 'Callback was not used; used interim (mutex failed)' ); + $this->assertEquals( 2, $calls, 'Callback was not used; used interim (mutex failed)' ); } /** @@ -1187,6 +1185,58 @@ class WANObjectCacheTest extends PHPUnit_Framework_TestCase { ]; } + /** + * @covers WANObjectCache::useInterimHoldOffCaching + * @covers WANObjectCache::getInterimValue + */ + public function testInterimHoldOffCaching() { + $cache = $this->cache; + + $value = 'CRL-40-940'; + $wasCalled = 0; + $func = function () use ( &$wasCalled, $value ) { + $wasCalled++; + + return $value; + }; + + $cache->useInterimHoldOffCaching( true ); + + $key = wfRandomString( 32 ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 1, $wasCalled, 'Value cached' ); + $cache->delete( $key ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 2, $wasCalled, 'Value regenerated (got mutex)' ); // sets interim + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 3, $wasCalled, 'Value regenerated (got mutex)' ); // sets interim + // Lock up the mutex so interim cache is used + $this->internalCache->add( $cache::MUTEX_KEY_PREFIX . $key, 1, 0 ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 3, $wasCalled, 'Value interim cached (failed mutex)' ); + $this->internalCache->delete( $cache::MUTEX_KEY_PREFIX . $key ); + + $cache->useInterimHoldOffCaching( false ); + + $wasCalled = 0; + $key = wfRandomString( 32 ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 1, $wasCalled, 'Value cached' ); + $cache->delete( $key ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 2, $wasCalled, 'Value regenerated (got mutex)' ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 3, $wasCalled, 'Value still regenerated (got mutex)' ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 4, $wasCalled, 'Value still regenerated (got mutex)' ); + // Lock up the mutex so interim cache is used + $this->internalCache->add( $cache::MUTEX_KEY_PREFIX . $key, 1, 0 ); + $v = $cache->getWithSetCallback( $key, 60, $func ); + $this->assertEquals( 5, $wasCalled, 'Value still regenerated (failed mutex)' ); + } + /** * @covers WANObjectCache::touchCheckKey * @covers WANObjectCache::resetCheckKey -- 2.20.1