Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/273561
Change subject: Allow FSFile objects for src in FileBackend::do*Operations
......................................................................
Allow FSFile objects for src in FileBackend::do*Operations
Convenience aside, this lets multiwrite backends do async replication for
"store" operations safely, buy keeping a handle to the source file that
prevents it from getting prematurely deleted before the post-send writes
to the secondary backends can happen.
Change-Id: I1254de527c47835c35fed6e526b42953c1b2b2ca
---
M includes/filebackend/FileBackend.php
M includes/filebackend/FileBackendMultiWrite.php
M includes/filerepo/FileRepo.php
3 files changed, 52 insertions(+), 13 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/61/273561/1
diff --git a/includes/filebackend/FileBackend.php
b/includes/filebackend/FileBackend.php
index fd9c8de..03974f7 100644
--- a/includes/filebackend/FileBackend.php
+++ b/includes/filebackend/FileBackend.php
@@ -237,6 +237,8 @@
* - describe (since 1.21)
* - null
*
+ * FSFile/TempFSFile object support was added in 1.27.
+ *
* a) Create a new file in storage with the contents of a string
* @code
* array(
@@ -253,7 +255,7 @@
* @code
* array(
* 'op' => 'store',
- * 'src' => <file system path>,
+ * 'src' => <file system path, FSFile, or
TempFSFile>,
* 'dst' => <storage path>,
* 'overwrite' => <boolean>,
* 'overwriteSame' => <boolean>,
@@ -372,11 +374,15 @@
if ( !count( $ops ) ) {
return Status::newGood(); // nothing to do
}
+
+ $ops = $this->resolveFSFileObjects( $ops );
if ( empty( $opts['force'] ) ) { // sanity
unset( $opts['nonLocking'] );
}
+
/** @noinspection PhpUnusedLocalVariableInspection */
$scope = $this->getScopedPHPBehaviorForOps(); // try to ignore
client aborts
+
return $this->doOperationsInternal( $ops, $opts );
}
@@ -503,6 +509,8 @@
* - describe (since 1.21)
* - null
*
+ * FSFile/TempFSFile object support was added in 1.27.
+ *
* a) Create a new file in storage with the contents of a string
* @code
* array(
@@ -517,7 +525,7 @@
* @code
* array(
* 'op' => 'store',
- * 'src' => <file system path>,
+ * 'src' => <file system path, FSFile, or
TempFSFile>,
* 'dst' => <storage path>,
* 'headers' => <HTTP header name/value map> #
since 1.21
* )
@@ -604,11 +612,15 @@
if ( !count( $ops ) ) {
return Status::newGood(); // nothing to do
}
+
+ $ops = $this->resolveFSFileObjects( $ops );
foreach ( $ops as &$op ) {
$op['overwrite'] = true; // avoids RTTs in key/value
stores
}
+
/** @noinspection PhpUnusedLocalVariableInspection */
$scope = $this->getScopedPHPBehaviorForOps(); // try to ignore
client aborts
+
return $this->doQuickOperationsInternal( $ops );
}
@@ -1331,6 +1343,28 @@
}
/**
+ * Convert FSFile 'src' paths to string paths (with an 'srcRef' field
set to the FSFile)
+ *
+ * The 'srcRef' field keeps any TempFSFile objects in scope for the
backend to have it
+ * around as long it needs (which may vary greatly depending on
configuration)
+ *
+ * @param array $ops File operation batch for
FileBaclend::doOperations()
+ * @return array File operation batch
+ */
+ protected function resolveFSFileObjects( array $ops ) {
+ foreach ( $ops as &$op ) {
+ $src = isset( $op['src'] ) ? $op['src'] : null;
+ if ( $src instanceof FSFile ) {
+ $op['srcRef'] = $src;
+ $op['src'] = $src->getPath();
+ }
+ }
+ unset( $op );
+
+ return $ops;
+ }
+
+ /**
* Check if a given path is a "mwstore://" path.
* This does not do any further validation or any existence checks.
*
diff --git a/includes/filebackend/FileBackendMultiWrite.php
b/includes/filebackend/FileBackendMultiWrite.php
index 2d27450..5a103c6 100644
--- a/includes/filebackend/FileBackendMultiWrite.php
+++ b/includes/filebackend/FileBackendMultiWrite.php
@@ -199,7 +199,7 @@
}
$realOps = $this->substOpBatchPaths( $ops,
$backend );
- if ( $this->asyncWrites &&
!$this->hasStoreOperation( $ops ) ) {
+ if ( $this->asyncWrites &&
!$this->hasVolatileSources( $ops ) ) {
// Bind $scopeLock to the callback to
preserve locks
DeferredUpdates::addCallableUpdate(
function() use ( $backend,
$realOps, $opts, $scopeLock ) {
@@ -468,13 +468,13 @@
}
/**
- * @param array $ops File operation batch map
- * @return bool
+ * @param array $ops File operations for FileBackend::doOperations()
+ * @return bool Whether there are file path sources with outside
lifetime/ownership
*/
- protected function hasStoreOperation( array $ops ) {
+ protected function hasVolatileSources( array $ops ) {
foreach ( $ops as $op ) {
- if ( $op['op'] === 'store' ) {
- return true;
+ if ( $op['op'] === 'store' && !isset( $op['srcRef'] ) )
{
+ return true; // source file might be deleted
anytime after do*Operations()
}
}
@@ -494,7 +494,7 @@
}
$realOps = $this->substOpBatchPaths( $ops, $backend );
- if ( $this->asyncWrites && !$this->hasStoreOperation(
$ops ) ) {
+ if ( $this->asyncWrites && !$this->hasVolatileSources(
$ops ) ) {
DeferredUpdates::addCallableUpdate(
function() use ( $backend, $realOps ) {
$backend->doQuickOperations(
$realOps );
diff --git a/includes/filerepo/FileRepo.php b/includes/filerepo/FileRepo.php
index 24c3964..789803f 100644
--- a/includes/filerepo/FileRepo.php
+++ b/includes/filerepo/FileRepo.php
@@ -956,7 +956,7 @@
* This function can be used to write to otherwise read-only foreign
repos.
* This is intended for copying generated thumbnails into the repo.
*
- * @param string $src Source file system path, storage path, or virtual
URL
+ * @param string|FSFile $src Source file system path, storage path, or
virtual URL
* @param string $dst Virtual URL or storage path
* @param array|string|null $options An array consisting of a key named
headers
* listing extra headers. If a string, taken as content-disposition
header.
@@ -1003,7 +1003,7 @@
* All path parameters may be a file system path, storage path, or
virtual URL.
* When "headers" are given they are used as HTTP headers if supported.
*
- * @param array $triples List of (source path, destination path,
disposition)
+ * @param array $triples List of (source path or FSFile, destination
path, disposition)
* @return FileRepoStatus
*/
public function quickImportBatch( array $triples ) {
@@ -1011,7 +1011,12 @@
$operations = [];
foreach ( $triples as $triple ) {
list( $src, $dst ) = $triple;
- $src = $this->resolveToStoragePath( $src );
+ if ( $src instanceof FSFile ) {
+ $op = 'store';
+ } else {
+ $src = $this->resolveToStoragePath( $src );
+ $op = FileBackend::isStoragePath( $src ) ?
'copy' : 'store';
+ }
$dst = $this->resolveToStoragePath( $dst );
if ( !isset( $triple[2] ) ) {
@@ -1026,7 +1031,7 @@
}
$operations[] = [
- 'op' => FileBackend::isStoragePath( $src ) ?
'copy' : 'store',
+ 'op' => $op,
'src' => $src,
'dst' => $dst,
'headers' => $headers
--
To view, visit https://gerrit.wikimedia.org/r/273561
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I1254de527c47835c35fed6e526b42953c1b2b2ca
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
