From 8dca7488eceb295708555e732b0d719c143efb41 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 13 Aug 2014 16:04:34 -0700
Subject: [PATCH] Made LocalFile move/delete/restore handle network partitions
 better

* Previously, the file existence checks would not distinguish an answer
  in the negative from a non-answer. This was a long-standing problem.
  This avoids moving DB entries without moving the files (unless the
  partition happens in the middle of the moves of course).
* Optimized fileExistsBatch() to do concurrent stats if possible.

bug: 40927
bug: 69312
Change-Id: I9132fd5591bb7a3d5852f17514dcf51a3d8b7812
---
 includes/filerepo/FileRepo.php       |  9 ++++--
 includes/filerepo/file/LocalFile.php | 47 ++++++++++++++++++++++------
 2 files changed, 43 insertions(+), 13 deletions(-)

diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index 3bff91fe55..71e57f2e45 100644
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -1346,13 +1346,16 @@ class FileRepo {
 	 * Checks existence of an array of files.
 	 *
 	 * @param array $files Virtual URLs (or storage paths) of files to check
-	 * @return array|bool Either array of files and existence flags, or false
+	 * @return array Map of files and existence flags, or false
 	 */
 	public function fileExistsBatch( array $files ) {
+		$paths = array_map( array( $this, 'resolveToStoragePath' ), $files );
+		$this->backend->preloadFileStat( array( 'srcs' => $paths ) );
+
 		$result = array();
 		foreach ( $files as $key => $file ) {
-			$file = $this->resolveToStoragePath( $file );
-			$result[$key] = $this->backend->fileExists( array( 'src' => $file ) );
+			$path = $this->resolveToStoragePath( $file );
+			$result[$key] = $this->backend->fileExists( array( 'src' => $path ) );
 		}
 
 		return $result;
diff --git a/includes/filerepo/file/LocalFile.php b/includes/filerepo/file/LocalFile.php
index 3ba47ff8e9..f6ab35454b 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -2282,7 +2282,12 @@ class LocalFileDeleteBatch {
 		$this->doDBInserts();
 
 		// Removes non-existent file from the batch, so we don't get errors.
-		$this->deletionBatch = $this->removeNonexistentFiles( $this->deletionBatch );
+		$checkStatus = $this->removeNonexistentFiles( $this->deletionBatch );
+		if ( !$checkStatus->isGood() ) {
+			$this->status->merge( $checkStatus );
+			return $this->status;
+		}
+		$this->deletionBatch = $checkStatus->value;
 
 		// Execute the file deletion batch
 		$status = $this->file->repo->deleteBatch( $this->deletionBatch );
@@ -2314,7 +2319,7 @@ class LocalFileDeleteBatch {
 	/**
 	 * Removes non-existent files from a deletion batch.
 	 * @param array $batch
-	 * @return array
+	 * @return Status
 	 */
 	function removeNonexistentFiles( $batch ) {
 		$files = $newBatch = array();
@@ -2325,6 +2330,10 @@ class LocalFileDeleteBatch {
 		}
 
 		$result = $this->file->repo->fileExistsBatch( $files );
+		if ( in_array( null, $result, true ) ) {
+			return Status::newFatal( 'backend-fail-internal',
+				$this->file->repo->getBackend()->getName() );
+		}
 
 		foreach ( $batch as $batchItem ) {
 			if ( $result[$batchItem[0]] ) {
@@ -2332,7 +2341,7 @@ class LocalFileDeleteBatch {
 			}
 		}
 
-		return $newBatch;
+		return Status::newGood( $newBatch );
 	}
 }
 
@@ -2571,7 +2580,12 @@ class LocalFileRestoreBatch {
 		}
 
 		// Remove missing files from batch, so we don't get errors when undeleting them
-		$storeBatch = $this->removeNonexistentFiles( $storeBatch );
+		$checkStatus = $this->removeNonexistentFiles( $storeBatch );
+		if ( !$checkStatus->isGood() ) {
+			$status->merge( $checkStatus );
+			return $status;
+		}
+		$storeBatch = $checkStatus->value;
 
 		// Run the store batch
 		// Use the OVERWRITE_SAME flag to smooth over a common error
@@ -2631,7 +2645,7 @@ class LocalFileRestoreBatch {
 	/**
 	 * Removes non-existent files from a store batch.
 	 * @param array $triplets
-	 * @return array
+	 * @return Status
 	 */
 	function removeNonexistentFiles( $triplets ) {
 		$files = $filteredTriplets = array();
@@ -2640,6 +2654,10 @@ class LocalFileRestoreBatch {
 		}
 
 		$result = $this->file->repo->fileExistsBatch( $files );
+		if ( in_array( null, $result, true ) ) {
+			return Status::newFatal( 'backend-fail-internal',
+				$this->file->repo->getBackend()->getName() );
+		}
 
 		foreach ( $triplets as $file ) {
 			if ( $result[$file[0]] ) {
@@ -2647,7 +2665,7 @@ class LocalFileRestoreBatch {
 			}
 		}
 
-		return $filteredTriplets;
+		return Status::newGood( $filteredTriplets );
 	}
 
 	/**
@@ -2820,7 +2838,12 @@ class LocalFileMoveBatch {
 		$status = $repo->newGood();
 
 		$triplets = $this->getMoveTriplets();
-		$triplets = $this->removeNonexistentFiles( $triplets );
+		$checkStatus = $this->removeNonexistentFiles( $triplets );
+		if ( !$checkStatus->isGood() ) {
+			$status->merge( $checkStatus );
+			return $status;
+		}
+		$triplets = $checkStatus->value;
 		$destFile = wfLocalFile( $this->target );
 
 		$this->file->lock(); // begin
@@ -2947,7 +2970,7 @@ class LocalFileMoveBatch {
 	/**
 	 * Removes non-existent files from move batch.
 	 * @param array $triplets
-	 * @return array
+	 * @return Status
 	 */
 	function removeNonexistentFiles( $triplets ) {
 		$files = array();
@@ -2957,8 +2980,12 @@ class LocalFileMoveBatch {
 		}
 
 		$result = $this->file->repo->fileExistsBatch( $files );
-		$filteredTriplets = array();
+		if ( in_array( null, $result, true ) ) {
+			return Status::newFatal( 'backend-fail-internal',
+				$this->file->repo->getBackend()->getName() );
+		}
 
+		$filteredTriplets = array();
 		foreach ( $triplets as $file ) {
 			if ( $result[$file[0]] ) {
 				$filteredTriplets[] = $file;
@@ -2967,7 +2994,7 @@ class LocalFileMoveBatch {
 			}
 		}
 
-		return $filteredTriplets;
+		return Status::newGood( $filteredTriplets );
 	}
 
 	/**
-- 
2.20.1