objectcache: add and use adaptiveTTL() method
authorAaron Schulz <aschulz@wikimedia.org>
Sun, 21 Aug 2016 21:53:55 +0000 (14:53 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Wed, 31 Aug 2016 03:40:56 +0000 (03:40 +0000)
* This better handles delayed/lost cache purges by
  having lower TTLs for entries that often changes.
* Use this for foreign upload description page caches,
  we purges are never received from the source wiki.
* Also use this for User and LocalFile cache TTLs.
* Also move the Database::getCacheSetOptions() call in
  User *before* doing the queries, which is preferred.
* Fixed some IDEA errors too, like the undeclared
  mApiBase field.

Change-Id: I70f8ebb29ac853c2a530d9eedb9e7facc1b7b710

includes/HttpFunctions.php
includes/filerepo/ForeignAPIRepo.php
includes/filerepo/file/LocalFile.php
includes/libs/objectcache/WANObjectCache.php
includes/user/User.php
tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php

index 54b057a..2ca5e1b 100644 (file)
@@ -599,7 +599,7 @@ class MWHttpRequest {
         * Returns the value of the given response header.
         *
         * @param string $header
-        * @return string
+        * @return string|null
         */
        public function getResponseHeader( $header ) {
                if ( !$this->respHeaders ) {
index b48191f..8619ba6 100644 (file)
@@ -63,8 +63,8 @@ class ForeignAPIRepo extends FileRepo {
        /** @var array */
        protected $mFileExists = [];
 
-       /** @var array */
-       private $mQueryCache = [];
+       /** @var string */
+       private $mApiBase;
 
        /**
         * @param array|null $info
@@ -397,7 +397,8 @@ class ForeignAPIRepo extends FileRepo {
                        }
                        /* There is a new Commons file, or existing thumbnail older than a month */
                }
-               $thumb = self::httpGet( $foreignUrl );
+
+               $thumb = self::httpGet( $foreignUrl, 'default', [], $mtime );
                if ( !$thumb ) {
                        wfDebug( __METHOD__ . " Could not download thumb\n" );
 
@@ -413,7 +414,11 @@ class ForeignAPIRepo extends FileRepo {
                        return $foreignUrl;
                }
                $knownThumbUrls[$sizekey] = $localUrl;
-               $cache->set( $key, $knownThumbUrls, $this->apiThumbCacheExpiry );
+
+               $ttl = $mtime
+                       ? $cache->adaptiveTTL( $mtime, $this->apiThumbCacheExpiry )
+                       : $this->apiThumbCacheExpiry;
+               $cache->set( $key, $knownThumbUrls, $ttl );
                wfDebug( __METHOD__ . " got local thumb $localUrl, saving to cache \n" );
 
                return $localUrl;
@@ -506,9 +511,12 @@ class ForeignAPIRepo extends FileRepo {
         * @param string $url
         * @param string $timeout
         * @param array $options
+        * @param integer|bool &$mtime Resulting Last-Modified UNIX timestamp if received
         * @return bool|string
         */
-       public static function httpGet( $url, $timeout = 'default', $options = [] ) {
+       public static function httpGet(
+               $url, $timeout = 'default', $options = [], &$mtime = false
+       ) {
                $options['timeout'] = $timeout;
                /* Http::get */
                $url = wfExpandUrl( $url, PROTO_HTTP );
@@ -524,6 +532,9 @@ class ForeignAPIRepo extends FileRepo {
                $status = $req->execute();
 
                if ( $status->isOK() ) {
+                       $mtime = wfTimestampOrNull( TS_UNIX, $req->getResponseHeader( 'Last-Modified' ) );
+                       $mtime = $mtime ?: false;
+
                        return $req->getContent();
                } else {
                        $logger = LoggerFactory::getInstance( 'http' );
@@ -531,6 +542,7 @@ class ForeignAPIRepo extends FileRepo {
                                $status->getWikiText( false, false, 'en' ),
                                [ 'caller' => 'ForeignAPIRepo::httpGet' ]
                        );
+
                        return false;
                }
        }
@@ -548,7 +560,7 @@ class ForeignAPIRepo extends FileRepo {
         * @param string $target Used in cache key creation, mostly
         * @param array $query The query parameters for the API request
         * @param int $cacheTTL Time to live for the memcached caching
-        * @return null
+        * @return string|null
         */
        public function httpGetCached( $target, $query, $cacheTTL = 3600 ) {
                if ( $this->mApiBase ) {
@@ -557,28 +569,23 @@ class ForeignAPIRepo extends FileRepo {
                        $url = $this->makeUrl( $query, 'api' );
                }
 
-               if ( !isset( $this->mQueryCache[$url] ) ) {
-                       $data = ObjectCache::getMainWANInstance()->getWithSetCallback(
-                               $this->getLocalCacheKey( get_class( $this ), $target, md5( $url ) ),
-                               $cacheTTL,
-                               function () use ( $url ) {
-                                       return ForeignAPIRepo::httpGet( $url );
+               $cache = ObjectCache::getMainWANInstance();
+               return $cache->getWithSetCallback(
+                       $this->getLocalCacheKey( get_class( $this ), $target, md5( $url ) ),
+                       $cacheTTL,
+                       function ( $curValue, &$ttl ) use ( $url, $cache ) {
+                               $html = self::httpGet( $url, 'default', [], $mtime );
+                               if ( $html !== false ) {
+                                       $ttl = $mtime ? $cache->adaptiveTTL( $mtime, $ttl ) : $ttl;
+                               } else {
+                                       $ttl = $cache->adaptiveTTL( $mtime, $ttl );
+                                       $html = null; // caches negatives
                                }
-                       );
 
-                       if ( !$data ) {
-                               return null;
-                       }
-
-                       if ( count( $this->mQueryCache ) > 100 ) {
-                               // Keep the cache from growing infinitely
-                               $this->mQueryCache = [];
-                       }
-
-                       $this->mQueryCache[$url] = $data;
-               }
-
-               return $this->mQueryCache[$url];
+                               return $html;
+                       },
+                       [ 'pcTTL' => $cache::TTL_PROC_LONG ]
+               );
        }
 
        /**
index 40141c9..8d25726 100644 (file)
@@ -279,8 +279,9 @@ class LocalFile extends File {
 
        /**
         * Save the file metadata to memcached
+        * @param array $cacheSetOpts Result of Database::getCacheSetOptions()
         */
-       private function saveToCache() {
+       private function saveToCache( array $cacheSetOpts ) {
                $this->load();
 
                $key = $this->getCacheKey();
@@ -308,9 +309,14 @@ class LocalFile extends File {
                }
 
                // Cache presence for 1 week and negatives for 1 day
-               $ttl = $this->fileExists ? 86400 * 7 : 86400;
-               $opts = Database::getCacheSetOptions( $this->repo->getSlaveDB() );
-               ObjectCache::getMainWANInstance()->set( $key, $cacheVal, $ttl, $opts );
+               $wanCache = ObjectCache::getMainWANInstance();
+               if ( $this->fileExists ) {
+                       $ttl = $wanCache::TTL_WEEK;
+                       $ttl = $wanCache->adaptiveTTL( wfTimestamp( TS_UNIX, $this->timestamp ), $ttl );
+               } else {
+                       $ttl = $wanCache::TTL_DAY;
+               }
+               $wanCache->set( $key, $cacheVal, $ttl, $cacheSetOpts );
        }
 
        /**
@@ -546,8 +552,9 @@ class LocalFile extends File {
        function load( $flags = 0 ) {
                if ( !$this->dataLoaded ) {
                        if ( ( $flags & self::READ_LATEST ) || !$this->loadFromCache() ) {
+                               $opts = Database::getCacheSetOptions( $this->repo->getSlaveDB() );
                                $this->loadFromDB( $flags );
-                               $this->saveToCache();
+                               $this->saveToCache( $opts );
                        }
                        $this->dataLoaded = true;
                }
index c40c819..0d7da91 100644 (file)
@@ -1042,6 +1042,43 @@ class WANObjectCache implements IExpiringStore, LoggerAwareInterface {
                return $this->cache->getQoS( $flag );
        }
 
+       /**
+        * Get a TTL that is higher for objects that have not changed recently
+        *
+        * This is useful for keys that get explicit purges and DB or purge relay
+        * lag is a potential concern (especially how it interacts with CDN cache)
+        *
+        * Example usage:
+        * @code
+        *     // Last-modified time of page
+        *     $mtime = wfTimestamp( TS_UNIX, $page->getTimestamp() );
+        *     // Get adjusted TTL. If $mtime is 3600 seconds ago and $minTTL/$factor left at
+        *     // defaults, then $ttl is 3600 * .2 = 720. If $minTTL was greater than 720, then
+        *     // $ttl would be $minTTL. If $maxTTL was smaller than 720, $ttl would be $maxTTL.
+        *     $ttl = $cache->adaptiveTTL( $mtime, $cache::TTL_DAY );
+        * @endcode
+        *
+        * @param integer|float $mtime UNIX timestamp
+        * @param integer $maxTTL Maximum TTL (seconds)
+        * @param integer $minTTL Minimum TTL (seconds); Default: 30
+        * @param float $factor Value in the range (0,1); Default: .2
+        * @return integer Adaptive TTL
+        * @since 1.28
+        */
+       public function adaptiveTTL( $mtime, $maxTTL, $minTTL = 30, $factor = .2 ) {
+               if ( is_float( $mtime ) ) {
+                       $mtime = (int)$mtime; // ignore fractional seconds
+               }
+
+               if ( !is_int( $mtime ) || $mtime <= 0 ) {
+                       return $minTTL; // no last-modified time provided
+               }
+
+               $age = time() - $mtime;
+
+               return (int)min( $maxTTL, max( $minTTL, $factor * $age ) );
+       }
+
        /**
         * Do the actual async bus purge of a key
         *
index a9ccc4e..4ec8d54 100644 (file)
@@ -473,7 +473,7 @@ class User implements IDBAccessObject {
                $data = $cache->getWithSetCallback(
                        $this->getCacheKey( $cache ),
                        $cache::TTL_HOUR,
-                       function ( $oldValue, &$ttl, array &$setOpts ) {
+                       function ( $oldValue, &$ttl, array &$setOpts ) use ( $cache ) {
                                $setOpts += Database::getCacheSetOptions( wfGetDB( DB_SLAVE ) );
                                wfDebug( "User: cache miss for user {$this->mId}\n" );
 
@@ -486,6 +486,8 @@ class User implements IDBAccessObject {
                                        $data[$name] = $this->$name;
                                }
 
+                               $ttl = $cache->adaptiveTTL( wfTimestamp( TS_UNIX, $this->mTouched ), $ttl );
+
                                return $data;
 
                        },
index 35005f5..aeb4666 100644 (file)
@@ -722,4 +722,27 @@ class WANObjectCacheTest extends MediaWikiTestCase {
                $wanCache->getWithSetCallback( 'p', 30, $valFunc );
                $wanCache->getCheckKeyTime( 'zzz' );
        }
+
+       /**
+        * @dataProvider provideAdaptiveTTL
+        * @covers WANObjectCache::adaptiveTTL()
+        */
+       public function testAdaptiveTTL( $ago, $maxTTL, $minTTL, $factor, $adaptiveTTL ) {
+               $mtime = is_int( $ago ) ? time() - $ago : $ago;
+               $margin = 5;
+               $ttl = $this->cache->adaptiveTTL( $mtime, $maxTTL, $minTTL, $factor );
+
+               $this->assertGreaterThanOrEqual( $adaptiveTTL - $margin, $ttl );
+               $this->assertLessThanOrEqual( $adaptiveTTL + $margin, $ttl );
+       }
+
+       public static function provideAdaptiveTTL() {
+               return [
+                       [ 3600, 900, 30, .2, 720 ],
+                       [ 3600, 500, 30, .2, 500 ],
+                       [ 3600, 86400, 800, .2, 800 ],
+                       [ false, 86400, 800, .2, 800 ],
+                       [ null, 86400, 800, .2, 800 ]
+               ];
+       }
 }