Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/248281
Change subject: Fix slow callbacks in getWithSetCallback() using lockTSE
......................................................................
Fix slow callbacks in getWithSetCallback() using lockTSE
* Keys that take a long time to generate would run into
the MAX_SNAPSHOT_LAG check and have set() no-op. This
would be fine except that leaves no key there to figure
out the time since expiry and therefore whether to use
the mutex, so it didn't. This now saves the keys but with
a FLG_STALE bit set, making the next caller that sees it
perform a regeneration (unless it can't get the mutex).
* Bumped LOCK_TTL so that keys can stay locked much longer.
* This is easy to test via sleep(5) in tagUsageStatistics()
and two Special:Tags browser tabs.
Bug: T91535
Change-Id: I549e70ace3d2e7da40d3c4346ebacc36024cd522
---
M includes/libs/objectcache/WANObjectCache.php
1 file changed, 28 insertions(+), 13 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/81/248281/1
diff --git a/includes/libs/objectcache/WANObjectCache.php
b/includes/libs/objectcache/WANObjectCache.php
index 39eb792..2fde0c0 100644
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -85,7 +85,7 @@
/** Seconds to keep dependency purge keys around */
const CHECK_KEY_TTL = 31536000; // 1 year
/** Seconds to keep lock keys around */
- const LOCK_TTL = 5;
+ const LOCK_TTL = 10;
/** Default remaining TTL at which to consider pre-emptive regeneration
*/
const LOW_TTL = 30;
/** Default time-since-expiry on a miss that makes a key "hot" */
@@ -100,6 +100,9 @@
/** Max TTL to store keys when a data sourced is lagged */
const TTL_LAGGED = 30;
+ /** Tiny negative float to use when CTL comes up >= 0 due to clock skew
*/
+ const TINY_NEGATIVE = -0.000001;
+
/** Cache format version number */
const VERSION = 1;
@@ -107,6 +110,10 @@
const FLD_VALUE = 1;
const FLD_TTL = 2;
const FLD_TIME = 3;
+ const FLD_FLAGS = 4;
+
+ /** @var integer Treat this value as expired-on-arrival */
+ const FLG_STALE = 1;
const ERR_NONE = 0; // no error
const ERR_NO_RESPONSE = 1; // no response
@@ -149,11 +156,11 @@
/**
* Fetch the value of a key from cache
*
- * If passed in, $curTTL is set to the remaining TTL (current time
left):
- * - a) INF; if the key exists, has no TTL, and is not expired by
$checkKeys
- * - b) float (>=0); if the key exists, has a TTL, and is not expired
by $checkKeys
- * - c) float (<0); if the key is tombstoned or existing but expired
by $checkKeys
- * - d) null; if the key does not exist and is not tombstoned
+ * If supplied, $curTTL is set to the remaining TTL (current time left):
+ * - a) INF; if $key exists, has no TTL, and is not expired by
$checkKeys
+ * - b) float (>=0); if $key exists, has a TTL, and is not expired by
$checkKeys
+ * - c) float (<0); if $key is tombstoned, stale, or existing but
expired by $checkKeys
+ * - d) null; if $key does not exist and is not tombstoned
*
* If a key is tombstoned, $curTTL will reflect the time since delete().
*
@@ -319,14 +326,17 @@
if ( $age > self::MAX_SNAPSHOT_LAG ) {
if ( $lockTSE >= 0 ) {
+ // Store the value for "lockTSE" seconds, but
mark as stale
$tempTTL = max( 1, (int)$lockTSE ); // set()
expects seconds
- $this->cache->set( self::STASH_KEY_PREFIX .
$key, $value, $tempTTL );
+ $wrapped = $this->wrap( $value, $tempTTL );
+ $wrapped[self::FLD_FLAGS] = self::FLG_STALE;
+ } else {
+ // Just no-op the write for being unsafe
+ return true;
}
-
- return true; // no-op the write for being unsafe
+ } else {
+ $wrapped = $this->wrap( $value, $ttl );
}
-
- $wrapped = $this->wrap( $value, $ttl );
$func = function ( $cache, $key, $cWrapped ) use ( $wrapped ) {
return ( is_string( $cWrapped ) )
@@ -913,7 +923,7 @@
$purgeTimestamp = self::parsePurgeValue( $wrapped );
if ( is_float( $purgeTimestamp ) ) {
// Purged values should always have a negative current
$ttl
- $curTTL = min( -0.000001, $purgeTimestamp - $now );
+ $curTTL = min( $purgeTimestamp - $now,
self::TINY_NEGATIVE );
return array( false, $curTTL );
}
@@ -924,7 +934,12 @@
return array( false, null );
}
- if ( $wrapped[self::FLD_TTL] > 0 ) {
+ $flags = isset( $wrapped[self::FLD_FLAGS] ) ?
$wrapped[self::FLD_FLAGS] : 0;
+ if ( ( $flags & self::FLG_STALE ) == self::FLG_STALE ) {
+ // Treat as expired, with the cache time as the
expiration
+ $age = $now - $wrapped[self::FLD_TIME];
+ $curTTL = min( -$age, self::TINY_NEGATIVE );
+ } elseif ( $wrapped[self::FLD_TTL] > 0 ) {
// Get the approximate time left on the key
$age = $now - $wrapped[self::FLD_TIME];
$curTTL = max( $wrapped[self::FLD_TTL] - $age, 0.0 );
--
To view, visit https://gerrit.wikimedia.org/r/248281
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I549e70ace3d2e7da40d3c4346ebacc36024cd522
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
