jenkins-bot has submitted this change and it was merged.
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
---
M includes/filerepo/FileRepo.php
M includes/filerepo/file/LocalFile.php
2 files changed, 43 insertions(+), 13 deletions(-)
Approvals:
Chad: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index 3bff91f..71e57f2 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 3ba47ff..f6ab354 100644
--- a/includes/filerepo/file/LocalFile.php
+++ b/includes/filerepo/file/LocalFile.php
@@ -2282,7 +2282,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
);
@@ -2314,7 +2319,7 @@
/**
* 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 @@
}
$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 @@
}
}
- return $newBatch;
+ return Status::newGood( $newBatch );
}
}
@@ -2571,7 +2580,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
@@ -2631,7 +2645,7 @@
/**
* 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 @@
}
$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 @@
}
}
- return $filteredTriplets;
+ return Status::newGood( $filteredTriplets );
}
/**
@@ -2820,7 +2838,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
@@ -2947,7 +2970,7 @@
/**
* Removes non-existent files from move batch.
* @param array $triplets
- * @return array
+ * @return Status
*/
function removeNonexistentFiles( $triplets ) {
$files = array();
@@ -2957,8 +2980,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;
@@ -2967,7 +2994,7 @@
}
}
- return $filteredTriplets;
+ return Status::newGood( $filteredTriplets );
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/153990
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I9132fd5591bb7a3d5852f17514dcf51a3d8b7812
Gerrit-PatchSet: 2
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Brian Wolff <[email protected]>
Gerrit-Reviewer: Chad <[email protected]>
Gerrit-Reviewer: Giuseppe Lavagetto <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
