From 2cfce9fd8d124db9720da1f34ffac7841e28b741 Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 18 Apr 2014 11:34:39 -0700
Subject: [PATCH] Bail out on FileBackend operations if the initial stat calls
 failed

* The preloadFileStat() call acts as a good canary for whether all
  affected servers are up. If anything failed in it, then it's not
  worth bothering to send the actual write requests.
* Also made FileBackend::preloadFileStat abstract while at it since
  the two subclasses implemented it and so should any others.
* Fixed a silly comment typo.

Change-Id: I5bd427f654aa4a9d6bfe4ed7566276e8ac520b30
---
 includes/filebackend/FileBackend.php          |  4 ++--
 .../filebackend/FileBackendMultiWrite.php     |  2 +-
 includes/filebackend/FileBackendStore.php     | 23 +++++++++++++++----
 includes/filerepo/FileRepo.php                |  2 +-
 4 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php
index c75972562c..c9b18d1e55 100644
--- a/includes/filebackend/FileBackend.php
+++ b/includes/filebackend/FileBackend.php
@@ -1217,10 +1217,10 @@ abstract class FileBackend {
 	 * @param array $params Parameters include:
 	 *   - srcs        : list of source storage paths
 	 *   - latest      : use the latest available data
+	 * @return bool All requests proceeded without I/O errors (since 1.24)
 	 * @since 1.23
 	 */
-	public function preloadFileStat( array $params ) {
-	}
+	abstract public function preloadFileStat( array $params );
 
 	/**
 	 * Lock the files at the given storage paths in the backend.
diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php
index c39bbaf40e..bfffcc0f2a 100644
--- a/includes/filebackend/FileBackendMultiWrite.php
+++ b/includes/filebackend/FileBackendMultiWrite.php
@@ -669,7 +669,7 @@ class FileBackendMultiWrite extends FileBackend {
 
 	public function preloadFileStat( array $params ) {
 		$realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] );
-		$this->backends[$this->masterIndex]->preloadFileStat( $realParams );
+		return $this->backends[$this->masterIndex]->preloadFileStat( $realParams );
 	}
 
 	public function getScopedLocksForOps( array $ops, Status $status ) {
diff --git a/includes/filebackend/FileBackendStore.php b/includes/filebackend/FileBackendStore.php
index 2fd1bf66a7..bcb0146006 100644
--- a/includes/filebackend/FileBackendStore.php
+++ b/includes/filebackend/FileBackendStore.php
@@ -1105,11 +1105,20 @@ abstract class FileBackendStore extends FileBackend {
 		// Load from the persistent container caches
 		$this->primeContainerCache( $paths );
 		// Get the latest stat info for all the files (having locked them)
-		$this->preloadFileStat( array( 'srcs' => $paths, 'latest' => true ) );
+		$ok = $this->preloadFileStat( array( 'srcs' => $paths, 'latest' => true ) );
 
-		// Actually attempt the operation batch...
-		$opts = $this->setConcurrencyFlags( $opts );
-		$subStatus = FileOpBatch::attempt( $performOps, $opts, $this->fileJournal );
+		if ( $ok ) {
+			// Actually attempt the operation batch...
+			$opts = $this->setConcurrencyFlags( $opts );
+			$subStatus = FileOpBatch::attempt( $performOps, $opts, $this->fileJournal );
+		} else {
+			// If we could not even stat some files, then bail out...
+			$subStatus = Status::newFatal( 'backend-fail-internal', $this->name );
+			foreach ( $ops as $i => $op ) { // mark each op as failed
+				$subStatus->success[$i] = false;
+				++$subStatus->failCount;
+			}
+		}
 
 		// Merge errors into status fields
 		$status->merge( $subStatus );
@@ -1282,11 +1291,12 @@ abstract class FileBackendStore extends FileBackend {
 
 	final public function preloadFileStat( array $params ) {
 		$section = new ProfileSection( __METHOD__ . "-{$this->name}" );
+		$success = true; // no network errors
 
 		$params['concurrency'] = ( $this->parallelize !== 'off' ) ? $this->concurrency : 1;
 		$stats = $this->doGetFileStatMulti( $params );
 		if ( $stats === null ) {
-			return; // not supported
+			return true; // not supported
 		}
 
 		$latest = !empty( $params['latest'] ); // use latest data?
@@ -1318,9 +1328,12 @@ abstract class FileBackendStore extends FileBackend {
 					array( 'hash' => false, 'latest' => $latest ) );
 				wfDebug( __METHOD__ . ": File $path does not exist.\n" );
 			} else { // an error occurred
+				$success = false;
 				wfDebug( __METHOD__ . ": Could not stat file $path.\n" );
 			}
 		}
+
+		return $success;
 	}
 
 	/**
diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index 888af377be..1d824a7a98 100644
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -1244,7 +1244,7 @@ class FileRepo {
 			// 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.
+			// race conditions unless a functioning LockManager is used.
 			// LocalFile also uses SELECT FOR UPDATE for synchronization.
 			$operations[] = array(
 				'op' => 'copy',
-- 
2.20.1