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

diff --git a/includes/filebackend/FileBackend.php b/includes/filebackend/FileBackend.php
index c759725..c9b18d1 100644 (file)
--- 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 c39bbaf..bfffcc0 100644 (file)
--- 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 2fd1bf6..bcb0146 100644 (file)
--- 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 888af37..1d824a7 100644 (file)
--- 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',