jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/393991 )
Change subject: objectcache: Make WANObjectCache interim caching not interfere
with ChronologyProtector
......................................................................
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
---
M includes/Setup.php
M includes/libs/objectcache/WANObjectCache.php
M tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
3 files changed, 94 insertions(+), 11 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/Setup.php b/includes/Setup.php
index 081ea68..d6f4b2f 100644
--- a/includes/Setup.php
+++ b/includes/Setup.php
@@ -736,17 +736,20 @@
// 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 db27e42..74ec7b9 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;
@@ -1104,6 +1106,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 ) ) {
@@ -1496,6 +1502,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..c2be911 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,58 @@
}
/**
+ * @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
* @covers WANObjectCache::getCheckKeyTime
--
To view, visit https://gerrit.wikimedia.org/r/393991
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ida1a923f779aaf8410da76643457d2200da6cb20
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Krinkle <krinklem...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
