From 957f09b8a4df2619801be847b7f6739ec494e856 Mon Sep 17 00:00:00 2001 From: Aaron Date: Tue, 15 May 2012 17:42:20 -0700 Subject: [PATCH] [FileBackend] Added doQuickOperations() function for things like purging thumbnails. * doQuickOperations() lets us do things like purge thumbnails as fast as possible. Stat calls, SHA1 checks, and RTTs in general are avoided. It also avoids the slowness of lazy population of thumbnail SHA1s in Swift (for those made by rewrite.py). * Removed supportedOperations() to avoid the extra complexity. * Made a few variable type and exception documentation cleanups. * Cleaned up unit test file removal a bit and made some functions private. Change-Id: I6922368c6af7752a6927d96402519132203108a1 --- includes/filerepo/backend/FileBackend.php | 71 ++++++++++++ .../backend/FileBackendMultiWrite.php | 18 +++ .../filerepo/backend/FileBackendStore.php | 103 ++++++++++++++---- includes/filerepo/backend/FileOpBatch.php | 2 +- .../includes/filerepo/FileBackendTest.php | 66 ++++++++--- 5 files changed, 223 insertions(+), 37 deletions(-) diff --git a/includes/filerepo/backend/FileBackend.php b/includes/filerepo/backend/FileBackend.php index 24c68495ba..77e78b0269 100644 --- a/includes/filerepo/backend/FileBackend.php +++ b/includes/filerepo/backend/FileBackend.php @@ -350,6 +350,77 @@ abstract class FileBackend { return $this->doOperation( $params, $opts ); } + /** + * Perform a set of independent file operations on some files. + * + * This does no locking, nor journaling, and possibly no stat calls. + * Any destination files that already exist will be overwritten. + * This should *only* be used on non-original files, like cache files. + * + * Supported operations and their parameters: + * a) Create a new file in storage with the contents of a string + * array( + * 'op' => 'create', + * 'dst' => , + * 'content' => + * ) + * b) Copy a file system file into storage + * array( + * 'op' => 'store', + * 'src' => , + * 'dst' => + * ) + * c) Copy a file within storage + * array( + * 'op' => 'copy', + * 'src' => , + * 'dst' => + * ) + * d) Move a file within storage + * array( + * 'op' => 'move', + * 'src' => , + * 'dst' => + * ) + * e) Delete a file within storage + * array( + * 'op' => 'delete', + * 'src' => , + * 'ignoreMissingSource' => + * ) + * f) Do nothing (no-op) + * array( + * 'op' => 'null', + * ) + * + * Boolean flags for operations (operation-specific): + * 'ignoreMissingSource' : The operation will simply succeed and do + * nothing if the source file does not exist. + * + * Return value: + * This returns a Status, which contains all warnings and fatals that occured + * during the operation. The 'failCount', 'successCount', and 'success' members + * will reflect each operation attempted for the given files. The status will be + * considered "OK" as long as no fatal errors occured. + * + * @param $ops Array Set of operations to execute + * @return Status + */ + final public function doQuickOperations( array $ops ) { + if ( $this->isReadOnly() ) { + return Status::newFatal( 'backend-fail-readonly', $this->name, $this->readOnly ); + } + foreach ( $ops as &$op ) { + $op['overwrite'] = true; // avoids RTTs in key/value stores + } + return $this->doQuickOperationsInternal( $ops ); + } + + /** + * @see FileBackend::doQuickOperations() + */ + abstract protected function doQuickOperationsInternal( array $ops ); + /** * Concatenate a list of storage files into a single file system file. * The target path should refer to a file that is already locked or diff --git a/includes/filerepo/backend/FileBackendMultiWrite.php b/includes/filerepo/backend/FileBackendMultiWrite.php index 4f9f0d9eee..b8b0ea6a6a 100644 --- a/includes/filerepo/backend/FileBackendMultiWrite.php +++ b/includes/filerepo/backend/FileBackendMultiWrite.php @@ -290,6 +290,24 @@ class FileBackendMultiWrite extends FileBackend { ); } + /** + * @see FileBackend::doQuickOperationsInternal() + * @return Status + */ + public function doQuickOperationsInternal( array $ops ) { + // Do the operations on the master backend; setting Status fields... + $realOps = $this->substOpBatchPaths( $ops, $this->backends[$this->masterIndex] ); + $status = $this->backends[$this->masterIndex]->doQuickOperations( $realOps ); + // Propagate the operations to the clone backends... + foreach ( $this->backends as $index => $backend ) { + if ( $index !== $this->masterIndex ) { // not done already + $realOps = $this->substOpBatchPaths( $ops, $backend ); + $status->merge( $backend->doQuickOperations( $realOps ) ); + } + } + return $status; + } + /** * @see FileBackend::doPrepare() * @return Status diff --git a/includes/filerepo/backend/FileBackendStore.php b/includes/filerepo/backend/FileBackendStore.php index 1a7bd06db3..1cb077bf03 100644 --- a/includes/filerepo/backend/FileBackendStore.php +++ b/includes/filerepo/backend/FileBackendStore.php @@ -257,6 +257,17 @@ abstract class FileBackendStore extends FileBackend { return $status; } + /** + * No-op file operation that does nothing. + * Do not call this function from places outside FileBackend and FileOp. + * + * @param $params Array + * @return Status + */ + final public function nullInternal( array $params ) { + return Status::newGood(); + } + /** * @see FileBackend::concatenate() * @return Status @@ -818,22 +829,6 @@ abstract class FileBackendStore extends FileBackend { */ abstract public function getFileListInternal( $container, $dir, array $params ); - /** - * Get the list of supported operations and their corresponding FileOp classes. - * - * @return Array - */ - protected function supportedOperations() { - return array( - 'store' => 'StoreFileOp', - 'copy' => 'CopyFileOp', - 'move' => 'MoveFileOp', - 'delete' => 'DeleteFileOp', - 'create' => 'CreateFileOp', - 'null' => 'NullFileOp' - ); - } - /** * Return a list of FileOp objects from a list of operations. * Do not call this function from places outside FileBackend. @@ -846,7 +841,14 @@ abstract class FileBackendStore extends FileBackend { * @throws MWException */ final public function getOperationsInternal( array $ops ) { - $supportedOps = $this->supportedOperations(); + $supportedOps = array( + 'store' => 'StoreFileOp', + 'copy' => 'CopyFileOp', + 'move' => 'MoveFileOp', + 'delete' => 'DeleteFileOp', + 'create' => 'CreateFileOp', + 'null' => 'NullFileOp' + ); $performOps = array(); // array of FileOp objects // Build up ordered array of FileOps... @@ -934,6 +936,59 @@ abstract class FileBackendStore extends FileBackend { return $status; } + /** + * @see FileBackend::doQuickOperationsInternal() + * @return Status + * @throws MWException + */ + final protected function doQuickOperationsInternal( array $ops ) { + $status = Status::newGood(); + + $async = $this->parallelize; + $maxConcurrency = $this->concurrency; // throttle + + $statuses = array(); // array of (index => Status) + $fileOpHandles = array(); // list of (index => handle) arrays + $curFileOpHandles = array(); // current handle batch + // Perform the sync-only ops and build up op handles for the async ops... + foreach ( $ops as $index => $params ) { + $method = $params['op'] . 'Internal'; // e.g. "storeInternal" + if ( !MWInit::methodExists( __CLASS__, $method ) ) { + throw new MWException( "Operation '{$params['op']}' is not supported." ); + } + $subStatus = $this->$method( array( 'async' => $async ) + $params ); + if ( $subStatus->value instanceof FileBackendStoreOpHandle ) { // async + if ( count( $curFileOpHandles ) >= $maxConcurrency ) { + $fileOpHandles[] = $curFileOpHandles; // push this batch + $curFileOpHandles = array(); + } + $curFileOpHandles[$index] = $subStatus->value; // keep index + } else { // error or completed + $statuses[$index] = $subStatus; // keep index + } + } + if ( count( $curFileOpHandles ) ) { + $fileOpHandles[] = $curFileOpHandles; // last batch + } + // Do all the async ops that can be done concurrently... + foreach ( $fileOpHandles as $fileHandleBatch ) { + $statuses = $statuses + $this->executeOpHandlesInternal( $fileHandleBatch ); + } + // Marshall and merge all the responses... + foreach ( $statuses as $index => $subStatus ) { + $status->merge( $subStatus ); + if ( $subStatus->isOK() ) { + $status->success[$index] = true; + ++$status->successCount; + } else { + $status->success[$index] = false; + ++$status->failCount; + } + } + + return $status; + } + /** * Execute a list of FileBackendStoreOpHandle handles in parallel. * The resulting Status object fields will correspond @@ -941,6 +996,7 @@ abstract class FileBackendStore extends FileBackend { * * @param $handles Array List of FileBackendStoreOpHandle objects * @return Array Map of Status objects + * @throws MWException */ final public function executeOpHandlesInternal( array $fileOpHandles ) { wfProfileIn( __METHOD__ ); @@ -961,6 +1017,7 @@ abstract class FileBackendStore extends FileBackend { /** * @see FileBackendStore::executeOpHandlesInternal() * @return Array List of corresponding Status objects + * @throws MWException */ protected function doExecuteOpHandlesInternal( array $fileOpHandles ) { foreach ( $fileOpHandles as $fileOpHandle ) { // OK if empty @@ -1263,7 +1320,7 @@ abstract class FileBackendStore extends FileBackend { /** * Get the cache key for a container * - * @param $container Resolved container name + * @param $container string Resolved container name * @return string */ private function containerCacheKey( $container ) { @@ -1273,7 +1330,7 @@ abstract class FileBackendStore extends FileBackend { /** * Set the cached info for a container * - * @param $container Resolved container name + * @param $container string Resolved container name * @param $val mixed Information to cache * @return void */ @@ -1284,7 +1341,7 @@ abstract class FileBackendStore extends FileBackend { /** * Delete the cached info for a container * - * @param $container Resolved container name + * @param $containers string Resolved container name * @return void */ final protected function deleteContainerCache( $container ) { @@ -1352,7 +1409,7 @@ abstract class FileBackendStore extends FileBackend { /** * Get the cache key for a file path * - * @param $path Storage path + * @param $path string Storage path * @return string */ private function fileCacheKey( $path ) { @@ -1362,7 +1419,7 @@ abstract class FileBackendStore extends FileBackend { /** * Set the cached stat info for a file path * - * @param $path Storage path + * @param $path string Storage path * @param $val mixed Information to cache * @return void */ @@ -1373,7 +1430,7 @@ abstract class FileBackendStore extends FileBackend { /** * Delete the cached stat info for a file path * - * @param $path Storage path + * @param $path string Storage path * @return void */ final protected function deleteFileCache( $path ) { diff --git a/includes/filerepo/backend/FileOpBatch.php b/includes/filerepo/backend/FileOpBatch.php index 049f2c54d9..9ffe0f23ec 100644 --- a/includes/filerepo/backend/FileOpBatch.php +++ b/includes/filerepo/backend/FileOpBatch.php @@ -202,7 +202,7 @@ class FileOpBatch { } // Try to do all the operations concurrently... $statuses = $statuses + $backend->executeOpHandlesInternal( $opHandles ); - // Marshall and merge all the responses (blocking)... + // Marshall and merge all the responses... foreach ( $performOpsBatch as $i => $fileOp ) { if ( !$fileOp->failed() ) { // failed => already has Status $subStatus = $statuses[$i]; diff --git a/tests/phpunit/includes/filerepo/FileBackendTest.php b/tests/phpunit/includes/filerepo/FileBackendTest.php index 8da54e2f23..2a9608e0c9 100644 --- a/tests/phpunit/includes/filerepo/FileBackendTest.php +++ b/tests/phpunit/includes/filerepo/FileBackendTest.php @@ -7,7 +7,6 @@ class FileBackendTest extends MediaWikiTestCase { private $backend, $multiBackend; private $filesToPrune = array(); - private $dirsToPrune = array(); private static $backendToUse; function setUp() { @@ -652,6 +651,54 @@ class FileBackendTest extends MediaWikiTestCase { return $cases; } + public function testDoQuickOperations() { + $this->backend = $this->singleBackend; + $this->doTestDoQuickOperations(); + $this->tearDownFiles(); + + $this->backend = $this->multiBackend; + $this->doTestDoQuickOperations(); + $this->tearDownFiles(); + } + + private function doTestDoQuickOperations() { + $backendName = $this->backendClass(); + + $base = $this->baseStorePath(); + $files = array( + "$base/unittest-cont1/fileA.a", + "$base/unittest-cont1/fileB.a", + "$base/unittest-cont1/fileC.a" + ); + $ops = array(); + $purgeOps = array(); + foreach ( $files as $path ) { + $status = $this->prepare( array( 'dir' => dirname( $path ) ) ); + $this->assertGoodStatus( $status, + "Preparing $path succeeded without warnings ($backendName)." ); + $ops[] = array( 'op' => 'create', 'dst' => $path, 'content' => mt_rand(0,50000) ); + $purgeOps[] = array( 'op' => 'delete', 'src' => $path ); + } + $purgeOps[] = array( 'op' => 'null' ); + $status = $this->backend->doQuickOperations( $ops ); + $this->assertGoodStatus( $status, + "Creation of source files succeeded ($backendName)." ); + + foreach ( $files as $file ) { + $this->assertTrue( $this->backend->fileExists( array( 'src' => $file ) ), + "File $file exists." ); + } + + $status = $this->backend->doQuickOperations( $purgeOps ); + $this->assertGoodStatus( $status, + "Quick deletion of source files succeeded ($backendName)." ); + + foreach ( $files as $file ) { + $this->assertFalse( $this->backend->fileExists( array( 'src' => $file ) ), + "File $file purged." ); + } + } + /** * @dataProvider provider_testConcatenate */ @@ -1034,7 +1081,7 @@ class FileBackendTest extends MediaWikiTestCase { $this->tearDownFiles(); } - function doTestRecursiveClean() { + private function doTestRecursiveClean() { $backendName = $this->backendClass(); $base = $this->baseStorePath(); @@ -1184,7 +1231,7 @@ class FileBackendTest extends MediaWikiTestCase { } // concurrency orientated - function doTestDoOperations2() { + private function doTestDoOperations2() { $base = $this->baseStorePath(); $fileAContents = '3tqtmoeatmn4wg4qe-mg3qt3 tq'; @@ -1270,7 +1317,7 @@ class FileBackendTest extends MediaWikiTestCase { "Correct file SHA-1 of $fileC" ); } - function doTestDoOperationsFailing() { + private function doTestDoOperationsFailing() { $base = $this->baseStorePath(); $fileA = "$base/unittest-cont2/a/b/fileA.txt"; @@ -1721,7 +1768,6 @@ class FileBackendTest extends MediaWikiTestCase { // test helper wrapper for backend prepare() function private function prepare( array $params ) { - $this->dirsToPrune[] = $params['dir']; return $this->backend->prepare( $params ); } @@ -1733,10 +1779,7 @@ class FileBackendTest extends MediaWikiTestCase { foreach ( $containers as $container ) { $this->deleteFiles( $container ); } - foreach ( $this->dirsToPrune as $dir ) { - $this->recursiveClean( $dir ); - } - $this->filesToPrune = $this->dirsToPrune = array(); + $this->filesToPrune = array(); } private function deleteFiles( $container ) { @@ -1748,10 +1791,7 @@ class FileBackendTest extends MediaWikiTestCase { array( 'force' => 1, 'nonLocking' => 1 ) ); } } - } - - private function recursiveClean( $dir ) { - $this->backend->clean( array( 'dir' => $dir, 'recursive' => 1 ) ); + $this->backend->clean( array( 'dir' => "$base/$container", 'recursive' => 1 ) ); } function assertGoodStatus( $status, $msg ) { -- 2.20.1