Aaron Schulz has uploaded a new change for review. ( https://gerrit.wikimedia.org/r/393991 )
Change subject: Make WANObjectCache interim caching not interfere with ChronologyProtector ...................................................................... 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 --- M includes/Setup.php M includes/libs/objectcache/WANObjectCache.php M tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php 3 files changed, 89 insertions(+), 6 deletions(-) git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core refs/changes/91/393991/1 diff --git a/includes/Setup.php b/includes/Setup.php index 5da34f8..5827c4e 100644 --- a/includes/Setup.php +++ b/includes/Setup.php @@ -736,7 +736,7 @@ // 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 improvement purposes $cpPosTime = $wgRequest->getFloat( 'cpPosTime', isset( $_COOKIE['cpPosTime'] ) ? (float)$_COOKIE['cpPosTime'] : null @@ -747,6 +747,10 @@ 'ChronologyProtection' => $wgRequest->getHeader( 'ChronologyProtection' ), 'ChronologyPositionTime' => $cpPosTime ] ); +// Make sure that caching does not compromise the consistency improvements +if ( $cpPosTime ) { + MediaWikiServices::getInstance()->getMainWANObjectCache()->useInterimHoldOffCaching( false ); +} unset( $cpPosTime ); // Useful debug output diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index 727e3c1..9279927 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -91,6 +91,8 @@ 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; @@ -1101,6 +1103,10 @@ * @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 ) ) { @@ -1493,6 +1499,30 @@ } /** + * 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 * @since 1.28 diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index df8228d..96e9f65 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -769,8 +769,6 @@ $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 @@ $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 @@ $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)' ); } /** @@ -1188,6 +1186,57 @@ } /** + * @covers WANObjectCache::$useInterimHoldOffCaching + */ + 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 * @covers WANObjectCache::getCheckKeyTime -- To view, visit https://gerrit.wikimedia.org/r/393991 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Ida1a923f779aaf8410da76643457d2200da6cb20 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: master Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits