From: Aaron Schulz Date: Sun, 21 Aug 2016 21:53:55 +0000 (-0700) Subject: objectcache: add and use adaptiveTTL() method X-Git-Tag: 1.31.0-rc.0~5821 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/?a=commitdiff_plain;h=52511952de7b716e3e7771366852170049afb072;p=lhc%2Fweb%2Fwiklou.git objectcache: add and use adaptiveTTL() method * 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 --- diff --git a/includes/HttpFunctions.php b/includes/HttpFunctions.php index 54b057aec6..2ca5e1b71f 100644 --- a/includes/HttpFunctions.php +++ b/includes/HttpFunctions.php @@ -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 ) { diff --git a/includes/filerepo/ForeignAPIRepo.php b/includes/filerepo/ForeignAPIRepo.php index b48191fd1c..8619ba6ec5 100644 --- a/includes/filerepo/ForeignAPIRepo.php +++ b/includes/filerepo/ForeignAPIRepo.php @@ -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 ] + ); } /** diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php index 40141c9cd0..8d25726cd3 100644 --- a/includes/filerepo/file/LocalFile.php +++ b/includes/filerepo/file/LocalFile.php @@ -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; } diff --git a/includes/libs/objectcache/WANObjectCache.php b/includes/libs/objectcache/WANObjectCache.php index c40c819861..0d7da91683 100644 --- a/includes/libs/objectcache/WANObjectCache.php +++ b/includes/libs/objectcache/WANObjectCache.php @@ -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 * diff --git a/includes/user/User.php b/includes/user/User.php index a9ccc4ed52..4ec8d54784 100644 --- a/includes/user/User.php +++ b/includes/user/User.php @@ -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; }, diff --git a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php index 35005f5c6a..aeb466629d 100644 --- a/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php +++ b/tests/phpunit/includes/libs/objectcache/WANObjectCacheTest.php @@ -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 ] + ]; + } }