Add process cache support to WANObjectCache
authorAaron Schulz <aschulz@wikimedia.org>
Wed, 7 Oct 2015 08:55:39 +0000 (01:55 -0700)
committerOri.livneh <ori@wikimedia.org>
Sat, 10 Oct 2015 06:14:50 +0000 (06:14 +0000)
* Make getWithSetCallback() accept a TTL field for this
* Make ChangeTag callers use this flag to avoid hundreds of
  duplicate queries at Special:Tags

Change-Id: Ic1ed28294f5d557e02f39a7f20d36766244b9ded

includes/changetags/ChangeTags.php
includes/libs/objectcache/WANObjectCache.php
tests/phpunit/includes/objectcache/WANObjectCacheTest.php

index 55fc104..5c70c99 100644 (file)
@@ -1101,7 +1101,8 @@ class ChangeTags {
                        },
                        array(
                                'checkKeys' => array( wfMemcKey( 'active-tags' ) ),
-                               'lockTSE' => INF
+                               'lockTSE' => INF,
+                               'pcTTL' => 30
                        )
                );
        }
@@ -1146,7 +1147,8 @@ class ChangeTags {
                        },
                        array(
                                'checkKeys' => array( wfMemcKey( 'valid-tags-db' ) ),
-                               'lockTSE' => INF
+                               'lockTSE' => INF,
+                               'pcTTL' => 30
                        )
                );
        }
@@ -1173,7 +1175,8 @@ class ChangeTags {
                        },
                        array(
                                'checkKeys' => array( wfMemcKey( 'valid-tags-hook' ) ),
-                               'lockTSE' => INF
+                               'lockTSE' => INF,
+                               'pcTTL' => 30
                        )
                );
        }
@@ -1214,15 +1217,8 @@ class ChangeTags {
         * @return array Array of string => int
         */
        public static function tagUsageStatistics() {
-               static $cachedStats = null;
-
-               // Process cache to avoid I/O and repeated regens during holdoff
-               if ( $cachedStats !== null ) {
-                       return $cachedStats;
-               }
-
                $fname = __METHOD__;
-               $cachedStats = ObjectCache::getMainWANInstance()->getWithSetCallback(
+               return ObjectCache::getMainWANInstance()->getWithSetCallback(
                        wfMemcKey( 'change-tag-statistics' ),
                        300,
                        function ( $oldValue, &$ttl, array &$setOpts ) use ( $fname ) {
@@ -1247,11 +1243,10 @@ class ChangeTags {
                        },
                        array(
                                'checkKeys' => array( wfMemcKey( 'change-tag-statistics' ) ),
-                               'lockTSE' => INF
+                               'lockTSE' => INF,
+                               'pcTTL' => 30
                        )
                );
-
-               return $cachedStats;
        }
 
        /**
index 621ad42..b0cb775 100644 (file)
@@ -60,6 +60,8 @@
 class WANObjectCache {
        /** @var BagOStuff The local datacenter cache */
        protected $cache;
+       /** @var HashBagOStuff Script instance PHP cache */
+       protected $procCache;
        /** @var string Cache pool name */
        protected $pool;
        /** @var EventRelayer */
@@ -127,6 +129,7 @@ class WANObjectCache {
                $this->cache = $params['cache'];
                $this->pool = $params['pool'];
                $this->relayer = $params['relayer'];
+               $this->procCache = new HashBagOStuff();
        }
 
        /**
@@ -638,7 +641,15 @@ class WANObjectCache {
         *      expiration is low, the assumption is that the key is hot and that a stampede is worth
         *      avoiding. Setting this above WANObjectCache::HOLDOFF_TTL makes no difference. The
         *      higher this is set, the higher the worst-case staleness can be.
-        *      Use WANObjectCache::TSE_NONE to disable this logic. Default: WANObjectCache::TSE_NONE.
+        *      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
+        *      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 matter much
+        *      Default: WANObjectCache::TTL_UNCACHEABLE.
+        * @param array $oldOpts Unused (mentioned only to avoid PHPDoc warnings)
         * @return mixed Value to use for the key
         */
        final public function getWithSetCallback(
@@ -658,6 +669,36 @@ class WANObjectCache {
                        $checkKeys = isset( $opts['checkKeys'] ) ? $opts['checkKeys'] : array();
                }
 
+               $pcTTL = isset( $opts['pcTTL'] ) ? $opts['pcTTL'] : self::TTL_UNCACHEABLE;
+
+               // Try the process cache if enabled
+               $value = ( $pcTTL >= 0 ) ? $this->procCache->get( $key ) : false;
+
+               if ( $value === false ) {
+                       // Fetch the value over the network
+                       $value = $this->doGetWithSetCallback( $key, $ttl, $callback, $checkKeys, $opts );
+                       // Update the process cache if enabled
+                       if ( $pcTTL >= 0 && $value !== false ) {
+                               $this->procCache->set( $key, $value, $pcTTL );
+                       }
+               }
+
+               return $value;
+       }
+
+       /**
+        * @see WANObjectCache::getWithSetCallback()
+        *
+        * @param string $key
+        * @param integer $ttl
+        * @param callback $callback
+        * @param array $checkKeys
+        * @param array $opts
+        * @return mixed
+        */
+       protected function doGetWithSetCallback(
+               $key, $ttl, $callback, array $checkKeys, array $opts
+       ) {
                $lowTTL = isset( $opts['lowTTL'] ) ? $opts['lowTTL'] : min( self::LOW_TTL, $ttl );
                $lockTSE = isset( $opts['lockTSE'] ) ? $opts['lockTSE'] : self::TSE_NONE;
 
index 4195216..8981f2f 100644 (file)
@@ -105,11 +105,11 @@ class WANObjectCacheTest extends MediaWikiTestCase {
 
                $wasSet = 0;
                $v = $cache->getWithSetCallback( $key, $func, 30, array(), array( 'lockTSE' => 5 ) );
-               $this->assertEquals( $v, $value );
+               $this->assertEquals( $value, $v, "Value returned" );
                $this->assertEquals( 1, $wasSet, "Value regenerated" );
 
                $curTTL = null;
-               $v = $cache->get( $key, $curTTL );
+               $cache->get( $key, $curTTL );
                $this->assertLessThanOrEqual( 20, $curTTL, 'Current TTL between 19-20 (overriden)' );
                $this->assertGreaterThanOrEqual( 19, $curTTL, 'Current TTL between 19-20 (overriden)' );
 
@@ -118,14 +118,14 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                        'lowTTL' => 0,
                        'lockTSE' => 5,
                ) );
-               $this->assertEquals( $v, $value );
+               $this->assertEquals( $value, $v, "Value returned" );
                $this->assertEquals( 0, $wasSet, "Value not regenerated" );
 
                $priorTime = microtime( true );
                usleep( 1 );
                $wasSet = 0;
                $v = $cache->getWithSetCallback( $key, $func, 30, array( $cKey1, $cKey2 ) );
-               $this->assertEquals( $v, $value );
+               $this->assertEquals( $value, $v, "Value returned" );
                $this->assertEquals( 1, $wasSet, "Value regenerated due to check keys" );
                $t1 = $cache->getCheckKeyTime( $cKey1 );
                $this->assertGreaterThanOrEqual( $priorTime, $t1, 'Check keys generated on miss' );
@@ -135,7 +135,7 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                $priorTime = microtime( true );
                $wasSet = 0;
                $v = $cache->getWithSetCallback( $key, $func, 30, array( $cKey1, $cKey2 ) );
-               $this->assertEquals( $v, $value );
+               $this->assertEquals( $value, $v, "Value returned" );
                $this->assertEquals( 1, $wasSet, "Value regenerated due to still-recent check keys" );
                $t1 = $cache->getCheckKeyTime( $cKey1 );
                $this->assertLessThanOrEqual( $priorTime, $t1, 'Check keys did not change again' );
@@ -144,8 +144,17 @@ class WANObjectCacheTest extends MediaWikiTestCase {
 
                $curTTL = null;
                $v = $cache->get( $key, $curTTL, array( $cKey1, $cKey2 ) );
-               $this->assertEquals( $v, $value );
+               $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, $func, 30, array(), array( 'pcTTL' => 5 ) );
+               $this->assertEquals( $value, $v, "Value returned" );
+               $cache->delete( $key );
+               $v = $cache->getWithSetCallback( $key, $func, 30, array(), array( 'pcTTL' => 5 ) );
+               $this->assertEquals( $value, $v, "Value still returned after deleted" );
+               $this->assertEquals( 1, $wasSet, "Value process cached while deleted" );
        }
 
        /**