Revert "[FileBackend] Added optional callback parameter to concatenate()."
authorAaron Schulz <aschulz@wikimedia.org>
Sun, 25 Nov 2012 00:59:16 +0000 (00:59 +0000)
committerAaron Schulz <aschulz@wikimedia.org>
Sun, 25 Nov 2012 01:01:12 +0000 (17:01 -0800)
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
includes/filebackend/FileBackendStore.php
includes/filerepo/FileRepo.php

index f1a67c8..b5e2315 100644 (file)
@@ -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 );
index 38d58ac..0f435a3 100644 (file)
@@ -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;
        }
index 782ebf2..651ee27 100644 (file)
@@ -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;