[FileBackend] Stat caching improvements.
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 16 Nov 2012 20:02:42 +0000 (12:02 -0800)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 16 Nov 2012 20:14:20 +0000 (12:14 -0800)
Callers often tend to end up calling getFileStat(), at least indirectly,
or in various successive function on the same path. This created RTTs
when the file didn't exist since negatives were not cached. This change
does the following:

* Cache definitive negatives (404s) in the process cache.
  Nothing is cached on failure (like network problems).
* Ignore process cache entries after a brief time period
  so long running scripts do not have overly stale entries.

Change-Id: I356bd9f48281e3c7e7a273778b2aca59c521a0c7

includes/cache/ProcessCacheLRU.php
includes/filebackend/FileBackendStore.php

index f215ebd..76c76f3 100644 (file)
@@ -28,6 +28,8 @@
 class ProcessCacheLRU {
        /** @var Array */
        protected $cache = array(); // (key => prop => value)
+       /** @var Array */
+       protected $cacheTimes = array(); // (key => prop => UNIX timestamp)
 
        protected $maxCacheKeys; // integer; max entries
 
@@ -44,7 +46,7 @@ class ProcessCacheLRU {
 
        /**
         * Set a property field for a cache entry.
-        * This will prune the cache if it gets too large.
+        * This will prune the cache if it gets too large based on LRU.
         * If the item is already set, it will be pushed to the top of the cache.
         *
         * @param $key string
@@ -57,9 +59,12 @@ class ProcessCacheLRU {
                        $this->ping( $key ); // push to top
                } elseif ( count( $this->cache ) >= $this->maxCacheKeys ) {
                        reset( $this->cache );
-                       unset( $this->cache[key( $this->cache )] );
+                       $evictKey = key( $this->cache );
+                       unset( $this->cache[$evictKey] );
+                       unset( $this->cacheTimes[$evictKey] );
                }
                $this->cache[$key][$prop] = $value;
+               $this->cacheTimes[$key][$prop] = time();
        }
 
        /**
@@ -67,10 +72,14 @@ class ProcessCacheLRU {
         *
         * @param $key string
         * @param $prop string
+        * @param $maxAge integer Ignore items older than this many seconds (since 1.21)
         * @return bool
         */
-       public function has( $key, $prop ) {
-               return isset( $this->cache[$key][$prop] );
+       public function has( $key, $prop, $maxAge = 0 ) {
+               if ( isset( $this->cache[$key][$prop] ) ) {
+                       return ( $maxAge <= 0 || ( time() - $this->cacheTimes[$key][$prop] ) <= $maxAge );
+               }
+               return false;
        }
 
        /**
@@ -100,9 +109,11 @@ class ProcessCacheLRU {
        public function clear( $keys = null ) {
                if ( $keys === null ) {
                        $this->cache = array();
+                       $this->cacheTimes = array();
                } else {
                        foreach ( (array)$keys as $key ) {
                                unset( $this->cache[$key] );
+                               unset( $this->cacheTimes[$key] );
                        }
                }
        }
index 97e49a5..43e6cb3 100644 (file)
@@ -48,6 +48,8 @@ abstract class FileBackendStore extends FileBackend {
 
        protected $maxFileSize = 4294967296; // integer bytes (4GiB)
 
+       const CACHE_TTL = 10; // integer; TTL in seconds for process cache entries
+
        /**
         * @see FileBackend::__construct()
         *
@@ -601,14 +603,15 @@ abstract class FileBackendStore extends FileBackend {
                wfProfileIn( __METHOD__ );
                wfProfileIn( __METHOD__ . '-' . $this->name );
                $latest = !empty( $params['latest'] ); // use latest data?
-               if ( !$this->cheapCache->has( $path, 'stat' ) ) {
+               if ( !$this->cheapCache->has( $path, 'stat', self::CACHE_TTL ) ) {
                        $this->primeFileCache( array( $path ) ); // check persistent cache
                }
-               if ( $this->cheapCache->has( $path, 'stat' ) ) {
+               if ( $this->cheapCache->has( $path, 'stat', self::CACHE_TTL ) ) {
                        $stat = $this->cheapCache->get( $path, 'stat' );
                        // If we want the latest data, check that this cached
-                       // value was in fact fetched with the latest available data.
-                       if ( !$latest || $stat['latest'] ) {
+                       // value was in fact fetched with the latest available data
+                       // (the process cache is ignored if it contains a negative).
+                       if ( !$latest || ( is_array( $stat ) && $stat['latest'] ) ) {
                                wfProfileOut( __METHOD__ . '-' . $this->name );
                                wfProfileOut( __METHOD__ );
                                return $stat;
@@ -619,7 +622,7 @@ abstract class FileBackendStore extends FileBackend {
                $stat = $this->doGetFileStat( $params );
                wfProfileOut( __METHOD__ . '-miss-' . $this->name );
                wfProfileOut( __METHOD__ . '-miss' );
-               if ( is_array( $stat ) ) { // don't cache negatives
+               if ( is_array( $stat ) ) { // file exists
                        $stat['latest'] = $latest;
                        $this->cheapCache->set( $path, 'stat', $stat );
                        $this->setFileCache( $path, $stat ); // update persistent cache
@@ -627,8 +630,11 @@ abstract class FileBackendStore extends FileBackend {
                                $this->cheapCache->set( $path, 'sha1',
                                        array( 'hash' => $stat['sha1'], 'latest' => $latest ) );
                        }
-               } else {
+               } elseif ( $stat === false ) { // file does not exist
+                       $this->cheapCache->set( $path, 'stat', false );
                        wfDebug( __METHOD__ . ": File $path does not exist.\n" );
+               } else { // an error occurred
+                       wfDebug( __METHOD__ . ": Could not stat file $path.\n" );
                }
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
@@ -682,7 +688,7 @@ abstract class FileBackendStore extends FileBackend {
                wfProfileIn( __METHOD__ );
                wfProfileIn( __METHOD__ . '-' . $this->name );
                $latest = !empty( $params['latest'] ); // use latest data?
-               if ( $this->cheapCache->has( $path, 'sha1' ) ) {
+               if ( $this->cheapCache->has( $path, 'sha1', self::CACHE_TTL ) ) {
                        $stat = $this->cheapCache->get( $path, 'sha1' );
                        // If we want the latest data, check that this cached
                        // value was in fact fetched with the latest available data.