From ab0a45cc0642169e7a58e67bf7c6b58b2333fe22 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Fri, 16 Nov 2012 15:25:50 -0800 Subject: [PATCH] [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 --- includes/filebackend/FileBackendStore.php | 47 +++++++++++++++++------ includes/filebackend/FileOp.php | 10 ++++- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 4172819e43..a7df19de14 100644 --- 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 bfa2757f0c..0d64a44b5e 100644 --- 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; -- 2.20.1