From: Aaron Schulz <aschulz@wikimedia.org>
Date: Tue, 30 Oct 2012 02:04:03 +0000 (-0700)
Subject: [FileRepo] Changed "publishBatch" to handle failure better.
X-Git-Tag: 1.31.0-rc.0~21798
X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_ecrire%28%22articles_versions%22%2C%22id_article=%24id_article%22%29%20.%20%22?a=commitdiff_plain;h=1e9fd0103734bb264ef517e3133690ef6a9a9c3e;p=lhc%2Fweb%2Fwiklou.git

[FileRepo] Changed "publishBatch" to handle failure better.

* Instead of moving the current file to the archive name,
  and then storing the new one, copy the current file to the
  archive name and overwrite the new one. When and object store
  backend is used, this reduces the number of operations from
  COPY, DELETE, PUT to just COPY and PUT. This reduces the RTTs,
  chances for failures, and avoids the period of time where the
  file has no current version.
* Also removed the "force" option to make file changes more likely
  to be all or nothing.

Change-Id: I46fc5c5c1fda5b386958b57557942f500de9dc2c
---

diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index e8aa5a6335..6cd93cc146 100644
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -1094,47 +1094,42 @@ class FileRepo {
 				return $this->newFatal( 'directorycreateerror', $archiveDir );
 			}
 
-			// Archive destination file if it exists
-			if ( $backend->fileExists( array( 'src' => $dstPath ) ) ) {
-				// Check if the archive file exists
-				// This is a sanity check to avoid data loss. In UNIX, the rename primitive
-				// unlinks the destination file if it exists. DB-based synchronisation in
-				// publishBatch's caller should prevent races. In Windows there's no
-				// problem because the rename primitive fails if the destination exists.
-				if ( $backend->fileExists( array( 'src' => $archivePath ) ) ) {
-					$operations[] = array( 'op' => 'null' );
-					continue;
-				} else {
-					$operations[] = array(
-						'op'           => 'move',
-						'src'          => $dstPath,
-						'dst'          => $archivePath
-					);
-				}
-				$status->value[$i] = 'archived';
-			} else {
-				$status->value[$i] = 'new';
-			}
+			// Archive destination file if it exists.
+			// This will check if the archive file also exists and fail if does.
+			// This is a sanity check to avoid data loss. On Windows and Linux,
+			// copy() will overwrite, so the existence check is vulnerable to
+			// race conditions unless an functioning LockManager is used.
+			// LocalFile also uses SELECT FOR UPDATE for synchronization.
+			$operations[] = array(
+				'op'                  => 'copy',
+				'src'                 => $dstPath,
+				'dst'                 => $archivePath,
+				'ignoreMissingSource' => true
+			);
+
 			// Copy (or move) the source file to the destination
 			if ( FileBackend::isStoragePath( $srcPath ) ) {
 				if ( $flags & self::DELETE_SOURCE ) {
 					$operations[] = array(
 						'op'           => 'move',
 						'src'          => $srcPath,
-						'dst'          => $dstPath
+						'dst'          => $dstPath,
+						'overwrite'    => true // replace current
 					);
 				} else {
 					$operations[] = array(
 						'op'           => 'copy',
 						'src'          => $srcPath,
-						'dst'          => $dstPath
+						'dst'          => $dstPath,
+						'overwrite'    => true // replace current
 					);
 				}
 			} else { // FS source path
 				$operations[] = array(
 					'op'           => 'store',
 					'src'          => $srcPath,
-					'dst'          => $dstPath
+					'dst'          => $dstPath,
+					'overwrite'    => true // replace current
 				);
 				if ( $flags & self::DELETE_SOURCE ) {
 					$sourceFSFilesToDelete[] = $srcPath;
@@ -1143,8 +1138,17 @@ class FileRepo {
 		}
 
 		// Execute the operations for each triplet
-		$opts = array( 'force' => true );
-		$status->merge( $backend->doOperations( $operations, $opts ) );
+		$status->merge( $backend->doOperations( $operations ) );
+		// Find out which files were archived...
+		foreach ( $triplets as $i => $triplet ) {
+			list( $srcPath, $dstRel, $archiveRel ) = $triplet;
+			$archivePath = $this->getZonePath( 'public' ) . "/$archiveRel";
+			if ( $this->fileExists( $archivePath ) ) {
+				$status->value[$i] = 'archived';
+			} else {
+				$status->value[$i] = 'new';
+			}
+		}
 		// Cleanup for disk source files...
 		foreach ( $sourceFSFilesToDelete as $file ) {
 			wfSuppressWarnings();