From 65612b4290c218731e9ac4c89ff9a87ec187214a Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sun, 25 Nov 2012 00:59:16 +0000 Subject: [PATCH] Revert "[FileBackend] Added optional callback parameter to concatenate()." This gives updates for the faster part (local FS file changes) and not the slow GETs that really need this. This should be re-done to use a byte based read progress callback (that handles the parallel GETs). This reverts commit 6c9f13b5477f54fbe9cff893f665814ad8cdff87 Change-Id: Id8d739e1d5048e7f38c68eda4f9a008682707cba --- includes/filebackend/FileBackend.php | 10 ---------- includes/filebackend/FileBackendStore.php | 14 -------------- includes/filerepo/FileRepo.php | 5 ++--- 3 files changed, 2 insertions(+), 27 deletions(-) diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php index f1a67c8f08..b5e231540b 100644 --- a/includes/filebackend/FileBackend.php +++ b/includes/filebackend/FileBackend.php @@ -586,21 +586,11 @@ abstract class FileBackend { * otherwise safe from modification from other processes. Normally, * the file will be a new temp file, which should be adequate. * - * If a callback function is given, it will be called each time a segment is - * appended and when the overall concatenate operation completes or fails. - * The arguments passed in are: - * - 1) A Status object containing errors if any problems occurred. - * - 2) The index of the relevant segment (starting at 1) if a segment was appended - * (including the last one) or null in the case of overall success or failure. - * When a good Status is returned with a null segment, then the operation completed. - * Callbacks should generally avoid throwing exceptions. - * * @param $params Array Operation parameters * $params include: * - srcs : ordered source storage paths (e.g. chunk1, chunk2, ...) * - dst : file system path to 0-byte temp file * - parallelize : try to do operations in parallel when possible - * - callback : closure called when chunks are appended and on success/failure * @return Status */ abstract public function concatenate( array $params ); diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php index 38d58ac70e..0f435a399d 100644 --- a/includes/filebackend/FileBackendStore.php +++ b/includes/filebackend/FileBackendStore.php @@ -310,12 +310,8 @@ abstract class FileBackendStore extends FileBackend { */ protected function doConcatenate( array $params ) { $status = Status::newGood(); - $tmpPath = $params['dst']; // convenience unset( $params['latest'] ); // sanity - $callback = isset( $params['callback'] ) - ? $params['callback'] - : function( Status $status, $segment ) {}; // Check that the specified temp file is valid... wfSuppressWarnings(); @@ -323,7 +319,6 @@ abstract class FileBackendStore extends FileBackend { wfRestoreWarnings(); if ( !$ok ) { // not present or not empty $status->fatal( 'backend-fail-opentemp', $tmpPath ); - $callback( $status, null ); // update progress return $status; } @@ -334,7 +329,6 @@ abstract class FileBackendStore extends FileBackend { $fsFile = $this->getLocalReference( array( 'src' => $path ) ); if ( !$fsFile ) { // retry failed? $status->fatal( 'backend-fail-read', $path ); - $callback( $status, null ); // update progress return $status; } } @@ -345,20 +339,16 @@ abstract class FileBackendStore extends FileBackend { $tmpHandle = fopen( $tmpPath, 'ab' ); if ( $tmpHandle === false ) { $status->fatal( 'backend-fail-opentemp', $tmpPath ); - $callback( $status, null ); // update progress return $status; } - $segment = 0; // segment number // Build up the temp file using the source chunks (in order)... foreach ( $fsFiles as $virtualSource => $fsFile ) { - ++$segment; // first segment is "1" // Get a handle to the local FS version $sourceHandle = fopen( $fsFile->getPath(), 'rb' ); if ( $sourceHandle === false ) { fclose( $tmpHandle ); $status->fatal( 'backend-fail-read', $virtualSource ); - $callback( $status, null ); // update progress return $status; } // Append chunk to file (pass chunk size to avoid magic quotes) @@ -366,20 +356,16 @@ abstract class FileBackendStore extends FileBackend { fclose( $sourceHandle ); fclose( $tmpHandle ); $status->fatal( 'backend-fail-writetemp', $tmpPath ); - $callback( $status , null ); return $status; } fclose( $sourceHandle ); - $callback( $status, $segment ); // update progress (chunk success) } if ( !fclose( $tmpHandle ) ) { $status->fatal( 'backend-fail-closetemp', $tmpPath ); - $callback( $status, null ); // update progress return $status; } clearstatcache(); // temp file changed - $callback( $status, null ); // update progress (full success) return $status; } diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php index 782ebf2dfc..651ee27127 100644 --- a/includes/filerepo/FileRepo.php +++ b/includes/filerepo/FileRepo.php @@ -988,10 +988,9 @@ class FileRepo { * @param $dstPath String Target file system path * @param $flags Integer: bitwise combination of the following flags: * self::DELETE_SOURCE Delete the source files - * @param $callback Closure Optional callback function (see FileBackend::concatenate()) * @return FileRepoStatus */ - public function concatenate( array $srcPaths, $dstPath, $flags = 0, Closure $callback = null ) { + public function concatenate( array $srcPaths, $dstPath, $flags = 0 ) { $this->assertWritableRepo(); // fail out if read-only $status = $this->newGood(); @@ -1004,7 +1003,7 @@ class FileRepo { } // Concatenate the chunks into one FS file - $params = array( 'srcs' => $sources, 'dst' => $dstPath, 'callback' => $callback ); + $params = array( 'srcs' => $sources, 'dst' => $dstPath ); $status->merge( $this->backend->concatenate( $params ) ); if ( !$status->isOK() ) { return $status; -- 2.20.1