[FileBackend] More stat caching improvements.

* Extended negative caching to handle the "latest" parameter.
* Added a new "dstExists" parameter to some write functions to avoid
  salting the cache when a file is created at an unused path. The ability
  to do this was already mentioned in the setFileCache() doc comments.

Change-Id: Ib64e4c128e16f4d284033fff70b88686fa0593ab

diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php
index 4172819..a7df19d 100644 (file)
--- a/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@ -97,6 +97,8 @@ abstract class FileBackendStore extends FileBackend {
         *   - async       : Status will be returned immediately if supported.
         *                   If the status is OK, then its value field will be
         *                   set to a FileBackendStoreOpHandle object.
+        *   - dstExists   : Whether a file exists at the destination (optimization).
+        *                   Callers can use "false" if no existing file is being changed.
         *
         * @param $params Array
         * @return Status
@@ -110,7 +112,9 @@ abstract class FileBackendStore extends FileBackend {
                } else {
                        $status = $this->doCreateInternal( $params );
                        $this->clearCache( array( $params['dst'] ) );
-                       $this->deleteFileCache( $params['dst'] ); // persistent cache
+                       if ( !isset( $params['dstExists'] ) || $params['dstExists'] ) {
+                               $this->deleteFileCache( $params['dst'] ); // persistent cache
+                       }
                }
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
@@ -136,6 +140,8 @@ abstract class FileBackendStore extends FileBackend {
         *   - async       : Status will be returned immediately if supported.
         *                   If the status is OK, then its value field will be
         *                   set to a FileBackendStoreOpHandle object.
+        *   - dstExists   : Whether a file exists at the destination (optimization).
+        *                   Callers can use "false" if no existing file is being changed.
         *
         * @param $params Array
         * @return Status
@@ -149,7 +155,9 @@ abstract class FileBackendStore extends FileBackend {
                } else {
                        $status = $this->doStoreInternal( $params );
                        $this->clearCache( array( $params['dst'] ) );
-                       $this->deleteFileCache( $params['dst'] ); // persistent cache
+                       if ( !isset( $params['dstExists'] ) || $params['dstExists'] ) {
+                               $this->deleteFileCache( $params['dst'] ); // persistent cache
+                       }
                }
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
@@ -175,6 +183,8 @@ abstract class FileBackendStore extends FileBackend {
         *   - async               : Status will be returned immediately if supported.
         *                           If the status is OK, then its value field will be
         *                           set to a FileBackendStoreOpHandle object.
+        *   - dstExists           : Whether a file exists at the destination (optimization).
+        *                           Callers can use "false" if no existing file is being changed.
         *
         * @param $params Array
         * @return Status
@@ -184,7 +194,9 @@ abstract class FileBackendStore extends FileBackend {
                wfProfileIn( __METHOD__ . '-' . $this->name );
                $status = $this->doCopyInternal( $params );
                $this->clearCache( array( $params['dst'] ) );
-               $this->deleteFileCache( $params['dst'] ); // persistent cache
+               if ( !isset( $params['dstExists'] ) || $params['dstExists'] ) {
+                       $this->deleteFileCache( $params['dst'] ); // persistent cache
+               }
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
@@ -240,6 +252,8 @@ abstract class FileBackendStore extends FileBackend {
         *   - async               : Status will be returned immediately if supported.
         *                           If the status is OK, then its value field will be
         *                           set to a FileBackendStoreOpHandle object.
+        *   - dstExists           : Whether a file exists at the destination (optimization).
+        *                           Callers can use "false" if no existing file is being changed.
         *
         * @param $params Array
         * @return Status
@@ -250,7 +264,9 @@ abstract class FileBackendStore extends FileBackend {
                $status = $this->doMoveInternal( $params );
                $this->clearCache( array( $params['src'], $params['dst'] ) );
                $this->deleteFileCache( $params['src'] ); // persistent cache
-               $this->deleteFileCache( $params['dst'] ); // persistent cache
+               if ( !isset( $params['dstExists'] ) || $params['dstExists'] ) {
+                       $this->deleteFileCache( $params['dst'] ); // persistent cache
+               }
                wfProfileOut( __METHOD__ . '-' . $this->name );
                wfProfileOut( __METHOD__ );
                return $status;
@@ -664,12 +680,19 @@ abstract class FileBackendStore extends FileBackend {
                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
-                       // (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;
+                       // value was in fact fetched with the latest available data.
+                       if ( is_array( $stat ) ) {
+                               if ( !$latest || $stat['latest'] ) {
+                                       wfProfileOut( __METHOD__ . '-' . $this->name );
+                                       wfProfileOut( __METHOD__ );
+                                       return $stat;
+                               }
+                       } elseif ( in_array( $stat, array( 'NOT_EXIST', 'NOT_EXIST_LATEST' ) ) ) {
+                               if ( !$latest || $stat === 'NOT_EXIST_LATEST' ) {
+                                       wfProfileOut( __METHOD__ . '-' . $this->name );
+                                       wfProfileOut( __METHOD__ );
+                                       return false;
+                               }
                        }
                }
                wfProfileIn( __METHOD__ . '-miss' );
@@ -686,7 +709,7 @@ abstract class FileBackendStore extends FileBackend {
                                        array( 'hash' => $stat['sha1'], 'latest' => $latest ) );
                        }
                } elseif ( $stat === false ) { // file does not exist
-                       $this->cheapCache->set( $path, 'stat', false );
+                       $this->cheapCache->set( $path, 'stat', $latest ? 'NOT_EXIST_LATEST' : 'NOT_EXIST' );
                        wfDebug( __METHOD__ . ": File $path does not exist.\n" );
                } else { // an error occurred
                        wfDebug( __METHOD__ . ": Could not stat file $path.\n" );
@@ -1661,6 +1684,8 @@ abstract class FileBackendStore extends FileBackend {
        /**
         * Delete the cached stat info for a file path.
         * The cache key is salted for a while to prevent race conditions.
+        * Since negatives (404s) are not cached, this does not need to be called when
+        * a file is created at a path were there was none before.
         *
         * @param $path string Storage path
         */

diff --git a/includes/filebackend/FileOp.php b/includes/filebackend/FileOp.php
index bfa2757..0d64a44 100644 (file)
--- a/includes/filebackend/FileOp.php
+++ b/includes/filebackend/FileOp.php
@@ -48,6 +48,7 @@ abstract class FileOp {
        protected $doOperation = true; // boolean; operation is not a no-op
        protected $sourceSha1; // string
        protected $destSameAsSource; // boolean
+       protected $destExists; // boolean
 
        /* Object life-cycle */
        const STATE_NEW = 1;
@@ -351,7 +352,7 @@ abstract class FileOp {
 
        /**
         * Check for errors with regards to the destination file already existing.
-        * This also updates the destSameAsSource and sourceSha1 member variables.
+        * Also set the destExists, destSameAsSource and sourceSha1 member variables.
         * A bad status will be returned if there is no chance it can be overwritten.
         *
         * @param $predicates Array
@@ -365,7 +366,8 @@ abstract class FileOp {
                        $this->sourceSha1 = $this->fileSha1( $this->params['src'], $predicates );
                }
                $this->destSameAsSource = false;
-               if ( $this->fileExists( $this->params['dst'], $predicates ) ) {
+               $this->destExists = $this->fileExists( $this->params['dst'], $predicates );
+               if ( $this->destExists ) {
                        if ( $this->getParam( 'overwrite' ) ) {
                                return $status; // OK
                        } elseif ( $this->getParam( 'overwriteSame' ) ) {
@@ -485,6 +487,7 @@ class CreateFileOp extends FileOp {
                }
                // Check if destination file exists
                $status->merge( $this->precheckDestExistence( $predicates ) );
+               $this->params['dstExists'] = $this->destExists; // see FileBackendStore::setFileCache()
                if ( $status->isOK() ) {
                        // Update file existence predicates
                        $predicates['exists'][$this->params['dst']] = true;
@@ -556,6 +559,7 @@ class StoreFileOp extends FileOp {
                }
                // Check if destination file exists
                $status->merge( $this->precheckDestExistence( $predicates ) );
+               $this->params['dstExists'] = $this->destExists; // see FileBackendStore::setFileCache()
                if ( $status->isOK() ) {
                        // Update file existence predicates
                        $predicates['exists'][$this->params['dst']] = true;
@@ -632,6 +636,7 @@ class CopyFileOp extends FileOp {
                }
                // Check if destination file exists
                $status->merge( $this->precheckDestExistence( $predicates ) );
+               $this->params['dstExists'] = $this->destExists; // see FileBackendStore::setFileCache()
                if ( $status->isOK() ) {
                        // Update file existence predicates
                        $predicates['exists'][$this->params['dst']] = true;
@@ -708,6 +713,7 @@ class MoveFileOp extends FileOp {
                }
                // Check if destination file exists
                $status->merge( $this->precheckDestExistence( $predicates ) );
+               $this->params['dstExists'] = $this->destExists; // see FileBackendStore::setFileCache()
                if ( $status->isOK() ) {
                        // Update file existence predicates
                        $predicates['exists'][$this->params['src']] = false;