Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/155575
Change subject: Made LocalFile move/delete/restore handle network partitions
better
......................................................................
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
(cherry picked from commit 8dca7488eceb295708555e732b0d719c143efb41)
---
M includes/filerepo/FileRepo.php
M includes/filerepo/file/LocalFile.php
2 files changed, 43 insertions(+), 13 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/75/155575/1
diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index 25a06d2..5929525 100644
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -1346,13 +1346,16 @@
* 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 07f1f09..8824b66 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -2284,7 +2284,12 @@
$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
);
@@ -2316,7 +2321,7 @@
/**
* Removes non-existent files from a deletion batch.
* @param array $batch
- * @return array
+ * @return Status
*/
function removeNonexistentFiles( $batch ) {
$files = $newBatch = array();
@@ -2327,6 +2332,10 @@
}
$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]] ) {
@@ -2334,7 +2343,7 @@
}
}
- return $newBatch;
+ return Status::newGood( $newBatch );
}
}
@@ -2575,7 +2584,12 @@
}
// 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
@@ -2635,7 +2649,7 @@
/**
* Removes non-existent files from a store batch.
* @param array $triplets
- * @return array
+ * @return Status
*/
function removeNonexistentFiles( $triplets ) {
$files = $filteredTriplets = array();
@@ -2644,6 +2658,10 @@
}
$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]] ) {
@@ -2651,7 +2669,7 @@
}
}
- return $filteredTriplets;
+ return Status::newGood( $filteredTriplets );
}
/**
@@ -2824,7 +2842,12 @@
$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
@@ -2951,7 +2974,7 @@
/**
* Removes non-existent files from move batch.
* @param array $triplets
- * @return array
+ * @return Status
*/
function removeNonexistentFiles( $triplets ) {
$files = array();
@@ -2961,8 +2984,12 @@
}
$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;
@@ -2971,7 +2998,7 @@
}
}
- return $filteredTriplets;
+ return Status::newGood( $filteredTriplets );
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/155575
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I9132fd5591bb7a3d5852f17514dcf51a3d8b7812
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: wmf/1.24wmf18
Gerrit-Owner: Aaron Schulz <[email protected]>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
