objectcache: Add "busyValue" option to WANObjectCache::getWithSetCallback

This is useful for avoiding stampedes in the one case that lockTSE
does not alone cover, which is when the key does not exist or is
tombstoned.

Also avoid saving interim values unless the key is tombstoned
since there is no point in doing that otherwise.

Change-Id: I70997e90217a0979e0589afa7a5107b0e623c7cf

diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php
index ab702d5..2dc17bf 100644 (file)
--- a/includes/libs/objectcache/WANObjectCache.php
+++ b/includes/libs/objectcache/WANObjectCache.php
@@ -759,6 +759,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
         * @param array $opts Options map:
         *   - checkKeys: List of "check" keys. The key at $key will be seen as invalid when either
         *      touchCheckKey() or resetCheckKey() is called on any of these keys.
+        *      Default: [];
         *   - lowTTL: Consider pre-emptive updates when the current TTL (sec) of the key is less than
         *      this. It becomes more likely over time, becoming a certainty once the key is expired.
         *      Default: WANObjectCache::LOW_TTL seconds.
@@ -770,6 +771,11 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
         *      higher this is set, the higher the worst-case staleness can be.
         *      Use WANObjectCache::TSE_NONE to disable this logic.
         *      Default: WANObjectCache::TSE_NONE.
+        *   - busyValue: If no value exists and another thread is currently regenerating it, use this
+        *      as a fallback value (or a callback to generate such a value). This assures that cache
+        *      stampedes cannot happen if the value falls out of cache. This can be used as insurance
+        *      against cache regeneration becoming very slow for some reason (greater than the TTL).
+        *      Default: null.
         *   - 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;
@@ -861,6 +867,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                $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'] : [];
+               $busyValue = isset( $opts['busyValue'] ) ? $opts['busyValue'] : null;
                $minTime = isset( $opts['minTime'] ) ? $opts['minTime'] : 0.0;
                $versioned = isset( $opts['version'] );
 
@@ -882,11 +889,13 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                $isTombstone = ( $curTTL !== null && $value === false );
                // Assume a key is hot if requested soon after invalidation
                $isHot = ( $curTTL !== null && $curTTL <= 0 && abs( $curTTL ) <= $lockTSE );
+               // Use the mutex if there is no value and a busy fallback is given
+               $checkBusy = ( $busyValue !== null && $value === false );
                // Decide whether a single thread should handle regenerations.
                // This avoids stampedes when $checkKeys are bumped and when preemptive
                // renegerations take too long. It also reduces regenerations while $key
                // is tombstoned. This balances cache freshness with avoiding DB load.
-               $useMutex = ( $isHot || ( $isTombstone && $lockTSE > 0 ) );
+               $useMutex = ( $isHot || ( $isTombstone && $lockTSE > 0 ) || $checkBusy );
 
                $lockAcquired = false;
                if ( $useMutex ) {
@@ -908,6 +917,10 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
 
                                        return $value;
                                }
+                               // Use the busy fallback value if nothing else
+                               if ( $busyValue !== null ) {
+                                       return is_callable( $busyValue ) ? $busyValue() : $busyValue;
+                               }
                        }
                }
 
@@ -921,7 +934,7 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                $asOf = microtime( true );
                // When delete() is called, writes are write-holed by the tombstone,
                // so use a special INTERIM key to pass the new value around threads.
-               if ( $useMutex && $value !== false && $ttl >= 0 ) {
+               if ( ( $isTombstone && $lockTSE > 0 ) && $value !== false && $ttl >= 0 ) {
                        $tempTTL = max( 1, (int)$lockTSE ); // set() expects seconds
                        $wrapped = $this->wrap( $value, $tempTTL, $asOf );
                        $this->cache->set( self::INTERIM_KEY_PREFIX . $key, $wrapped, $tempTTL );

diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
index 5bc1c8d..6a3cd15 100644 (file)
--- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
+++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php
@@ -221,18 +221,18 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                $checkKeys = [ wfRandomString() ]; // new check keys => force misses
                $ret = $cache->getWithSetCallback( $key, 30, $func,
                        [ 'lockTSE' => 5, 'checkKeys' => $checkKeys ] );
-               $this->assertEquals( $value, $ret );
+               $this->assertEquals( $value, $ret, 'Old value used' );
                $this->assertEquals( 1, $calls, 'Callback was not used' );
 
                $cache->delete( $key );
                $ret = $cache->getWithSetCallback( $key, 30, $func,
-                       [ 'lockTSE' => 5, 'checkKeys' => $checkKeys ] ); // should use interim value
-               $this->assertEquals( $value, $ret );
-               $this->assertEquals( 2, $calls, 'Callback was used' );
+                       [ 'lockTSE' => 5, 'checkKeys' => $checkKeys ] );
+               $this->assertEquals( $value, $ret, 'Callback was used; interim saved' );
+               $this->assertEquals( 2, $calls, 'Callback was used; interim saved' );
 
                $ret = $cache->getWithSetCallback( $key, 30, $func,
                        [ 'lockTSE' => 5, 'checkKeys' => $checkKeys ] );
-               $this->assertEquals( $value, $ret );
+               $this->assertEquals( $value, $ret, 'Callback was not used; used interim' );
                $this->assertEquals( 2, $calls, 'Callback was not used; used interim' );
        }
 
@@ -267,6 +267,59 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                $this->assertEquals( 1, $calls, 'Callback was not used' );
        }
 
+       /**
+        * @covers WANObjectCache::getWithSetCallback()
+        * @covers WANObjectCache::doGetWithSetCallback()
+        */
+       public function testBusyValue() {
+               $cache = $this->cache;
+               $key = wfRandomString();
+               $value = wfRandomString();
+               $busyValue = wfRandomString();
+
+               $calls = 0;
+               $func = function() use ( &$calls, $value ) {
+                       ++$calls;
+                       return $value;
+               };
+
+               $ret = $cache->getWithSetCallback( $key, 30, $func, [ 'busyValue' => $busyValue ] );
+               $this->assertEquals( $value, $ret );
+               $this->assertEquals( 1, $calls, 'Value was populated' );
+
+               // Acquire a lock to verify that getWithSetCallback uses busyValue properly
+               $this->internalCache->lock( $key, 0 );
+
+               $checkKeys = [ wfRandomString() ]; // new check keys => force misses
+               $ret = $cache->getWithSetCallback( $key, 30, $func,
+                       [ 'busyValue' => $busyValue, 'checkKeys' => $checkKeys ] );
+               $this->assertEquals( $value, $ret, 'Callback used' );
+               $this->assertEquals( 2, $calls, 'Callback used' );
+
+               $ret = $cache->getWithSetCallback( $key, 30, $func,
+                       [ 'lockTSE' => 30, 'busyValue' => $busyValue, 'checkKeys' => $checkKeys ] );
+               $this->assertEquals( $value, $ret, 'Old value used' );
+               $this->assertEquals( 2, $calls, 'Callback was not used' );
+
+               $cache->delete( $key ); // no value at all anymore and still locked
+               $ret = $cache->getWithSetCallback( $key, 30, $func,
+                       [ 'busyValue' => $busyValue, 'checkKeys' => $checkKeys ] );
+               $this->assertEquals( $busyValue, $ret, 'Callback was not used; used busy value' );
+               $this->assertEquals( 2, $calls, 'Callback was not used; used busy value' );
+
+               $this->internalCache->unlock( $key );
+               $ret = $cache->getWithSetCallback( $key, 30, $func,
+                       [ 'lockTSE' => 30, 'busyValue' => $busyValue, 'checkKeys' => $checkKeys ] );
+               $this->assertEquals( $value, $ret, 'Callback was used; saved interim' );
+               $this->assertEquals( 3, $calls, 'Callback was used; saved interim' );
+
+               $this->internalCache->lock( $key, 0 );
+               $ret = $cache->getWithSetCallback( $key, 30, $func,
+                       [ 'busyValue' => $busyValue, 'checkKeys' => $checkKeys ] );
+               $this->assertEquals( $value, $ret, 'Callback was not used; used interim' );
+               $this->assertEquals( 3, $calls, 'Callback was not used; used interim' );
+       }
+
        /**
         * @covers WANObjectCache::getMulti()
         */