Thijs/wiki.techinc.nl
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Bail out on FileBackend operations if the initial stat calls failed

Browse source 
* 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
This commit is contained in:
Aaron Schulz 2014-04-18 11:34:39 -07:00
parent d99bda31e3
commit 2cfce9fd8d
4 changed files with 22 additions and 9 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
includes/filebackend/FileBackend.php
View file

@ -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.

2
includes/filebackend/FileBackendMultiWrite.php
View file

@ -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 ) {

23
includes/filebackend/FileBackendStore.php
View file

@ -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;
}
/**

2
includes/filerepo/FileRepo.php
View file

@ -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',