jenkins-bot has submitted this change and it was merged.
Change subject: objectcache: Support key versioning in WANObjectCache
......................................................................
objectcache: Support key versioning in WANObjectCache
* getWithSetCallback() takes a 'version' parameter.
* If the value at a key has a different version, then
getWithSetCallback() will automatically use a separate
key. Which value "wins" the main key does not matter.
* Purges are handled by using the main key as a sort of
check key (with no hold-off). Note that this key is always
purged on delete().
* Changed stash keys to track the same info as other keys
both for consistency and because this change needs the
generation timestamp. Renamed the stash prefix to avoid
corrupt results with Het Deploy.
* This is useful for things like the User class that use
versioning and have cross-wiki key access and purges.
Currently, bumps to version must be deployed to all wikis
at once, which this aims to avoid.
Change-Id: I26ae62f116e32b48bcf06bc13f8b9e79ae976745
---
M includes/libs/objectcache/WANObjectCache.php
M tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
2 files changed, 202 insertions(+), 31 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 470a38c..ad7bc08 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -127,10 +127,13 @@
const ERR_RELAY = 4; // relay broadcast failed
const VALUE_KEY_PREFIX = 'WANCache:v:';
- const STASH_KEY_PREFIX = 'WANCache:s:';
+ const INTERIM_KEY_PREFIX = 'WANCache:i:';
const TIME_KEY_PREFIX = 'WANCache:t:';
const PURGE_VAL_PREFIX = 'PURGED:';
+
+ const VFLD_DATA = 'WOC:d'; // key to the value of versioned data
+ const VFLD_VERSION = 'WOC:v'; // key to the version of the value present
const MAX_PC_KEYS = 1000; // max keys to keep in process cache
@@ -207,14 +210,17 @@
* That method has cache slam avoiding features for hot/expensive keys.
*
* @param string $key Cache key
- * @param mixed $curTTL Approximate TTL left on the key if present
[returned]
+ * @param mixed $curTTL Approximate TTL left on the key if
present/tombstoned [returned]
* @param array $checkKeys List of "check" keys
+ * @param float &$asOf UNIX timestamp of cached value; null on failure
[returned]
* @return mixed Value of cache key or false on failure
*/
- final public function get( $key, &$curTTL = null, array $checkKeys = []
) {
+ final public function get( $key, &$curTTL = null, array $checkKeys =
[], &$asOf = null ) {
$curTTLs = [];
- $values = $this->getMulti( [ $key ], $curTTLs, $checkKeys );
+ $asOfs = [];
+ $values = $this->getMulti( [ $key ], $curTTLs, $checkKeys,
$asOfs );
$curTTL = isset( $curTTLs[$key] ) ? $curTTLs[$key] : null;
+ $asOf = isset( $asOfs[$key] ) ? $asOfs[$key] : null;
return isset( $values[$key] ) ? $values[$key] : false;
}
@@ -228,13 +234,15 @@
* @param array $curTTLs Map of (key => approximate TTL left) for
existing keys [returned]
* @param array $checkKeys List of check keys to apply to all $keys.
May also apply "check"
* keys to specific cache keys only by using cache keys as keys in the
$checkKeys array.
+ * @param float[] &$asOfs Map of (key => UNIX timestamp of cached
value; null on failure)
* @return array Map of (key => value) for keys that exist
*/
final public function getMulti(
- array $keys, &$curTTLs = [], array $checkKeys = []
+ array $keys, &$curTTLs = [], array $checkKeys = [], array
&$asOfs = []
) {
$result = [];
$curTTLs = [];
+ $asOfs = [];
$vPrefixLen = strlen( self::VALUE_KEY_PREFIX );
$valueKeys = self::prefixCacheKeys( $keys,
self::VALUE_KEY_PREFIX );
@@ -297,6 +305,7 @@
}
}
$curTTLs[$key] = $curTTL;
+ $asOfs[$key] = ( $value !== false ) ?
$wrappedValues[$vKey][self::FLD_TIME] : null;
}
return $result;
@@ -761,13 +770,18 @@
* higher this is set, the higher the worst-case staleness can be.
* Use WANObjectCache::TSE_NONE to disable this logic.
* Default: WANObjectCache::TSE_NONE.
- * - pcTTL : process cache the value in this PHP instance with this
TTL. This avoids
+ * - pcTTL: Process cache the value in this PHP instance with this
TTL. This avoids
* network I/O when a key is read several times. This will not
cache if the callback
* returns false however. Note that any purges will not be seen
while process cached;
* since the callback should use slave DBs and they may be lagged
or have snapshot
* isolation anyway, this should not typically matter.
* Default: WANObjectCache::TTL_UNCACHEABLE.
- * @return mixed Value to use for the key
+ * - version: Integer version number. This allows for callers to make
breaking changes to
+ * how values are stored while maintaining compatability and
correct cache purges. New
+ * versions are stored alongside older versions concurrently.
Avoid storing class objects
+ * however, as this reduces compatibility (due to serialization).
+ * Default: null.
+ * @return mixed Value found or written to the key
*/
final public function getWithSetCallback( $key, $ttl, $callback, array
$opts = [] ) {
$pcTTL = isset( $opts['pcTTL'] ) ? $opts['pcTTL'] :
self::TTL_UNCACHEABLE;
@@ -776,8 +790,43 @@
$value = ( $pcTTL >= 0 ) ? $this->procCache->get( $key ) :
false;
if ( $value === false ) {
+ unset( $opts['minTime'] ); // not a public feature
+
// Fetch the value over the network
- $value = $this->doGetWithSetCallback( $key, $ttl,
$callback, $opts );
+ if ( isset( $opts['version'] ) ) {
+ $version = $opts['version'];
+ $asOf = null;
+ $cur = $this->doGetWithSetCallback(
+ $key,
+ $ttl,
+ function ( $oldValue, &$ttl, &$setOpts
) use ( $callback, $version ) {
+ $oldData = $oldValue ?
$oldValue[self::VFLD_DATA] : false;
+ return [
+ self::VFLD_DATA =>
$callback( $oldData, $ttl, $setOpts ),
+ self::VFLD_VERSION =>
$version
+ ];
+ },
+ $opts,
+ $asOf
+ );
+ if ( $cur[self::VFLD_VERSION] === $version ) {
+ // Value created or existed before with
version; use it
+ $value = $cur[self::VFLD_DATA];
+ } else {
+ // Value existed before with a
different version; use variant key.
+ // Reflect purges to $key by requiring
that this key value be newer.
+ $value = $this->doGetWithSetCallback(
+ 'cache-variant:' . md5( $key )
. ":$version",
+ $ttl,
+ $callback,
+ // Regenerate value if not
newer than $key
+ [ 'version' => null, 'minTime'
=> $asOf ] + $opts
+ );
+ }
+ } else {
+ $value = $this->doGetWithSetCallback( $key,
$ttl, $callback, $opts );
+ }
+
// Update the process cache if enabled
if ( $pcTTL >= 0 && $value !== false ) {
$this->procCache->set( $key, $value, $pcTTL );
@@ -795,21 +844,29 @@
* @param string $key
* @param integer $ttl
* @param callback $callback
- * @param array $opts
+ * @param array $opts Options map for getWithSetCallback() which also
includes:
+ * - minTime: Treat values older than this UNIX timestamp as not
existing. Default: null.
+ * @param float &$asOf Cache generation timestamp of returned value
[returned]
* @return mixed
*/
- protected function doGetWithSetCallback( $key, $ttl, $callback, array
$opts ) {
+ protected function doGetWithSetCallback( $key, $ttl, $callback, array
$opts, &$asOf = null ) {
$lowTTL = isset( $opts['lowTTL'] ) ? $opts['lowTTL'] : min(
self::LOW_TTL, $ttl );
$lockTSE = isset( $opts['lockTSE'] ) ? $opts['lockTSE'] :
self::TSE_NONE;
$checkKeys = isset( $opts['checkKeys'] ) ? $opts['checkKeys'] :
[];
+ $minTime = isset( $opts['minTime'] ) ? $opts['minTime'] : 0.0;
+ $versioned = isset( $opts['version'] );
// Get the current key value
$curTTL = null;
- $cValue = $this->get( $key, $curTTL, $checkKeys ); // current
value
+ $cValue = $this->get( $key, $curTTL, $checkKeys, $asOf ); //
current value
$value = $cValue; // return value
// Determine if a regeneration is desired
- if ( $value !== false && $curTTL > 0 && !$this->worthRefresh(
$curTTL, $lowTTL ) ) {
+ if ( $value !== false
+ && $curTTL > 0
+ && $this->isValid( $value, $versioned, $asOf, $minTime )
+ && !$this->worthRefresh( $curTTL, $lowTTL )
+ ) {
return $value;
}
@@ -829,15 +886,18 @@
if ( $this->cache->lock( $key, 0, self::LOCK_TTL ) ) {
// Lock acquired; this thread should update the
key
$lockAcquired = true;
- } elseif ( $value !== false ) {
+ } elseif ( $value !== false && $this->isValid( $value,
$versioned, $asOf, $minTime ) ) {
// If it cannot be acquired; then the stale
value can be used
return $value;
} else {
- // Use the stash value for tombstoned keys to
reduce regeneration load.
+ // Use the INTERIM value for tombstoned keys to
reduce regeneration load.
// For hot keys, either another thread has the
lock or the lock failed;
- // use the stash value from the last thread
that regenerated it.
- $value = $this->cache->get(
self::STASH_KEY_PREFIX . $key );
- if ( $value !== false ) {
+ // use the INTERIM value from the last thread
that regenerated it.
+ $wrapped = $this->cache->get(
self::INTERIM_KEY_PREFIX . $key );
+ $value = $this->unwrap( $wrapped, microtime(
true ) );
+ if ( $value !== false && $this->isValid(
$value, $versioned, $asOf, $minTime ) ) {
+ $asOf = $wrapped[self::FLD_TIME];
+
return $value;
}
}
@@ -850,11 +910,13 @@
// Generate the new value from the callback...
$setOpts = [];
$value = call_user_func_array( $callback, [ $cValue, &$ttl,
&$setOpts ] );
+ $asOf = microtime( true );
// When delete() is called, writes are write-holed by the
tombstone,
- // so use a special stash key to pass the new value around
threads.
+ // so use a special INTERIM key to pass the new value around
threads.
if ( $useMutex && $value !== false && $ttl >= 0 ) {
$tempTTL = max( 1, (int)$lockTSE ); // set() expects
seconds
- $this->cache->set( self::STASH_KEY_PREFIX . $key,
$value, $tempTTL );
+ $wrapped = $this->wrap( $value, $tempTTL, $asOf );
+ $this->cache->set( self::INTERIM_KEY_PREFIX . $key,
$wrapped, $tempTTL );
}
if ( $lockAcquired ) {
@@ -1006,6 +1068,25 @@
}
/**
+ * Check whether $value is appropriately versioned and not older than
$minTime (if set)
+ *
+ * @param array $value
+ * @param bool $versioned
+ * @param float $asOf The time $value was generated
+ * @param float $minTime The last time the main value was generated
(0.0 if unknown)
+ * @return bool
+ */
+ protected function isValid( $value, $versioned, $asOf, $minTime ) {
+ if ( $versioned && !isset( $value[self::VFLD_VERSION] ) ) {
+ return false;
+ } elseif ( $minTime > 0 && $asOf < $minTime ) {
+ return false;
+ }
+
+ return true;
+ }
+
+ /**
* Do not use this method outside WANObjectCache
*
* @param mixed $value
diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
index 266e025..3a4aab4 100644
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -33,17 +33,26 @@
* @param integer $ttl
*/
public function testSetAndGet( $value, $ttl ) {
+ $curTTL = null;
+ $asOf = null;
$key = wfRandomString();
+
+ $this->cache->get( $key, $curTTL, [], $asOf );
+ $this->assertNull( $curTTL, "Current TTL is null" );
+ $this->assertNull( $asOf, "Current as-of-time is infinite" );
+
+ $t = microtime( true );
$this->cache->set( $key, $value, $ttl );
- $curTTL = null;
- $this->assertEquals( $value, $this->cache->get( $key, $curTTL )
);
+ $this->assertEquals( $value, $this->cache->get( $key, $curTTL,
[], $asOf ) );
if ( is_infinite( $ttl ) || $ttl == 0 ) {
$this->assertTrue( is_infinite( $curTTL ), "Current TTL
is infinite" );
} else {
$this->assertGreaterThan( 0, $curTTL, "Current TTL > 0"
);
$this->assertLessThanOrEqual( $ttl, $curTTL, "Current
TTL < nominal TTL" );
}
+ $this->assertGreaterThanOrEqual( $t - 1, $asOf, "As-of-time in
range of set() time" );
+ $this->assertLessThanOrEqual( $t + 1, $asOf, "As-of-time in
range of set() time" );
}
public static function provideSetAndGet() {
@@ -97,10 +106,13 @@
}
/**
+ * @dataProvider getWithSetCallback_provider
* @covers WANObjectCache::getWithSetCallback()
* @covers WANObjectCache::doGetWithSetCallback()
+ * @param array $extOpts
+ * @param bool $versioned
*/
- public function testGetWithSetCallback() {
+ public function testGetWithSetCallback( array $extOpts, $versioned ) {
$cache = $this->cache;
$key = wfRandomString();
@@ -116,7 +128,7 @@
};
$wasSet = 0;
- $v = $cache->getWithSetCallback( $key, 30, $func, [ 'lockTSE'
=> 5 ] );
+ $v = $cache->getWithSetCallback( $key, 30, $func, [ 'lockTSE'
=> 5 ] + $extOpts );
$this->assertEquals( $value, $v, "Value returned" );
$this->assertEquals( 1, $wasSet, "Value regenerated" );
@@ -129,15 +141,16 @@
$v = $cache->getWithSetCallback( $key, 30, $func, [
'lowTTL' => 0,
'lockTSE' => 5,
- ] );
+ ] + $extOpts );
$this->assertEquals( $value, $v, "Value returned" );
$this->assertEquals( 0, $wasSet, "Value not regenerated" );
$priorTime = microtime( true );
usleep( 1 );
$wasSet = 0;
- $v = $cache->getWithSetCallback( $key, 30, $func,
- [ 'checkKeys' => [ $cKey1, $cKey2 ] ] );
+ $v = $cache->getWithSetCallback(
+ $key, 30, $func, [ 'checkKeys' => [ $cKey1, $cKey2 ] ]
+ $extOpts
+ );
$this->assertEquals( $value, $v, "Value returned" );
$this->assertEquals( 1, $wasSet, "Value regenerated due to
check keys" );
$t1 = $cache->getCheckKeyTime( $cKey1 );
@@ -147,8 +160,9 @@
$priorTime = microtime( true );
$wasSet = 0;
- $v = $cache->getWithSetCallback( $key, 30, $func,
- [ 'checkKeys' => [ $cKey1, $cKey2 ] ] );
+ $v = $cache->getWithSetCallback(
+ $key, 30, $func, [ 'checkKeys' => [ $cKey1, $cKey2 ] ]
+ $extOpts
+ );
$this->assertEquals( $value, $v, "Value returned" );
$this->assertEquals( 1, $wasSet, "Value regenerated due to
still-recent check keys" );
$t1 = $cache->getCheckKeyTime( $cKey1 );
@@ -158,17 +172,28 @@
$curTTL = null;
$v = $cache->get( $key, $curTTL, [ $cKey1, $cKey2 ] );
- $this->assertEquals( $value, $v, "Value returned" );
+ if ( $versioned ) {
+ $this->assertEquals( $value, $v[$cache::VFLD_DATA],
"Value returned" );
+ } else {
+ $this->assertEquals( $value, $v, "Value returned" );
+ }
$this->assertLessThanOrEqual( 0, $curTTL, "Value has current
TTL < 0 due to check keys" );
$wasSet = 0;
$key = wfRandomString();
- $v = $cache->getWithSetCallback( $key, 30, $func, [ 'pcTTL' =>
5 ] );
+ $v = $cache->getWithSetCallback( $key, 30, $func, [ 'pcTTL' =>
5 ] + $extOpts );
$this->assertEquals( $value, $v, "Value returned" );
$cache->delete( $key );
- $v = $cache->getWithSetCallback( $key, 30, $func, [ 'pcTTL' =>
5 ] );
+ $v = $cache->getWithSetCallback( $key, 30, $func, [ 'pcTTL' =>
5 ] + $extOpts );
$this->assertEquals( $value, $v, "Value still returned after
deleted" );
$this->assertEquals( 1, $wasSet, "Value process cached while
deleted" );
+ }
+
+ public static function getWithSetCallback_provider() {
+ return [
+ [ [], false ],
+ [ [ 'version' => 1 ], true ]
+ ];
}
/**
@@ -435,6 +460,71 @@
}
/**
+ * @dataProvider getWithSetCallback_versions_provider
+ * @param array $extOpts
+ * @param $versioned
+ */
+ public function testGetWithSetCallback_versions( array $extOpts,
$versioned ) {
+ $cache = $this->cache;
+
+ $key = wfRandomString();
+ $value = wfRandomString();
+
+ $wasSet = 0;
+ $func = function( $old, &$ttl ) use ( &$wasSet, $value ) {
+ ++$wasSet;
+ return $value;
+ };
+
+ // Set the main key (version N if versioned)
+ $wasSet = 0;
+ $v = $cache->getWithSetCallback( $key, 30, $func, $extOpts );
+ $this->assertEquals( $value, $v, "Value returned" );
+ $this->assertEquals( 1, $wasSet, "Value regenerated" );
+ $cache->getWithSetCallback( $key, 30, $func, $extOpts );
+ $this->assertEquals( 1, $wasSet, "Value not regenerated" );
+ // Set the key for version N+1 (if versioned)
+ if ( $versioned ) {
+ $verOpts = [ 'version' => $extOpts['version'] + 1 ];
+
+ $wasSet = 0;
+ $v = $cache->getWithSetCallback( $key, 30, $func,
$verOpts + $extOpts );
+ $this->assertEquals( $value, $v, "Value returned" );
+ $this->assertEquals( 1, $wasSet, "Value regenerated" );
+
+ $wasSet = 0;
+ $v = $cache->getWithSetCallback( $key, 30, $func,
$verOpts + $extOpts );
+ $this->assertEquals( $value, $v, "Value returned" );
+ $this->assertEquals( 0, $wasSet, "Value not
regenerated" );
+ }
+
+ $wasSet = 0;
+ $cache->getWithSetCallback( $key, 30, $func, $extOpts );
+ $this->assertEquals( 0, $wasSet, "Value not regenerated" );
+
+ $wasSet = 0;
+ $cache->delete( $key );
+ $v = $cache->getWithSetCallback( $key, 30, $func, $extOpts );
+ $this->assertEquals( $value, $v, "Value returned" );
+ $this->assertEquals( 1, $wasSet, "Value regenerated" );
+
+ if ( $versioned ) {
+ $wasSet = 0;
+ $verOpts = [ 'version' => $extOpts['version'] + 1 ];
+ $v = $cache->getWithSetCallback( $key, 30, $func,
$verOpts + $extOpts );
+ $this->assertEquals( $value, $v, "Value returned" );
+ $this->assertEquals( 1, $wasSet, "Value regenerated" );
+ }
+ }
+
+ public static function getWithSetCallback_versions_provider() {
+ return [
+ [ [], false ],
+ [ [ 'version' => 1 ], true ]
+ ];
+ }
+
+ /**
* @covers WANObjectCache::touchCheckKey()
* @covers WANObjectCache::resetCheckKey()
* @covers WANObjectCache::getCheckKeyTime()
--
To view, visit https://gerrit.wikimedia.org/r/288347
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I26ae62f116e32b48bcf06bc13f8b9e79ae976745
Gerrit-PatchSet: 8
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: 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
