From 169b7b98b5c43db55acfc7e31b089300c0026c80 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sat, 19 Sep 2015 21:09:26 -0700 Subject: [PATCH] Added replication=async option to FileBackendMultiWrite * This will defer writes to non-master backends till the end up the web request. This is useful for multi-DC setups. Bug: T112708 Change-Id: I118c07764dd4a4f4f2590d4548238df12860e750 --- includes/deferred/DeferredUpdates.php | 18 +++- .../filebackend/FileBackendMultiWrite.php | 90 +++++++++++++------ .../includes/filebackend/FileBackendTest.php | 53 +++++++++++ 3 files changed, 131 insertions(+), 30 deletions(-) diff --git a/includes/deferred/DeferredUpdates.php b/includes/deferred/DeferredUpdates.php index cd0266f6b8..7d02a7a9cf 100644 --- a/includes/deferred/DeferredUpdates.php +++ b/includes/deferred/DeferredUpdates.php @@ -44,10 +44,10 @@ interface DeferrableUpdate { * @since 1.19 */ class DeferredUpdates { - /** - * @var array Updates to be deferred until the end of the request. - */ + /** @var array Updates to be deferred until the end of the request */ private static $updates = array(); + /** @var bool Defer updates fully even in CLI mode */ + private static $forceDeferral = false; /** * Add an update to the deferred list @@ -57,6 +57,9 @@ class DeferredUpdates { global $wgCommandLineMode; array_push( self::$updates, $update ); + if ( self::$forceDeferral ) { + return; + } // CLI scripts may forget to periodically flush these updates, // so try to handle that rather than OOMing and losing them. @@ -129,4 +132,13 @@ class DeferredUpdates { public static function clearPendingUpdates() { self::$updates = array(); } + + /** + * @note This method is intended for testing purposes + * @param bool $value Whether to *always* defer updates, even in CLI mode + * @since 1.26 + */ + public static function forceDeferral( $value ) { + self::$forceDeferral = $value; + } } diff --git a/includes/filebackend/FileBackendMultiWrite.php b/includes/filebackend/FileBackendMultiWrite.php index 1603d44f96..3841f2ed37 100644 --- a/includes/filebackend/FileBackendMultiWrite.php +++ b/includes/filebackend/FileBackendMultiWrite.php @@ -52,10 +52,12 @@ class FileBackendMultiWrite extends FileBackend { /** @var int Bitfield */ protected $syncChecks = 0; - /** @var string|bool */ protected $autoResync = false; + /** @var bool */ + protected $asyncWrites = false; + /* Possible internal backend consistency checks */ const CHECK_SIZE = 1; const CHECK_TIME = 2; @@ -85,6 +87,9 @@ class FileBackendMultiWrite extends FileBackend { * Use "conservative" to limit resyncing to copying newer master * backend files over older (or non-existing) clone backend files. * Cases that cannot be handled will result in operation abortion. + * - replication : Set to 'async' to defer file operations on the non-master backends. + * This will apply such updates post-send for web requests. Note that + * any checks from "syncChecks" are still synchronous. * * @param array $config * @throws FileBackendError @@ -97,6 +102,7 @@ class FileBackendMultiWrite extends FileBackend { $this->autoResync = isset( $config['autoResync'] ) ? $config['autoResync'] : false; + $this->asyncWrites = isset( $config['replication'] ) && $config['replication'] === 'async'; // Construct backends here rather than via registration // to keep these backends hidden from outside the proxy. $namesUsed = array(); @@ -147,6 +153,7 @@ class FileBackendMultiWrite extends FileBackend { $mbe = $this->backends[$this->masterIndex]; // convenience // Try to lock those files for the scope of this function... + $scopeLock = null; if ( empty( $opts['nonLocking'] ) ) { // Try to lock those files for the scope of this function... /** @noinspection PhpUnusedLocalVariableInspection */ @@ -187,8 +194,19 @@ class FileBackendMultiWrite extends FileBackend { // If $ops only had one operation, this might avoid backend sync inconsistencies. if ( $masterStatus->isOK() && $masterStatus->successCount > 0 ) { foreach ( $this->backends as $index => $backend ) { - if ( $index !== $this->masterIndex ) { // not done already - $realOps = $this->substOpBatchPaths( $ops, $backend ); + if ( $index === $this->masterIndex ) { + continue; // done already + } + + $realOps = $this->substOpBatchPaths( $ops, $backend ); + if ( $this->asyncWrites ) { + // Bind $scopeLock to the callback to preserve locks + DeferredUpdates::addCallableUpdate( + function() use ( $backend, $realOps, $opts, $scopeLock ) { + $backend->doOperations( $realOps, $opts ); + } + ); + } else { $status->merge( $backend->doOperations( $realOps, $opts ) ); } } @@ -457,8 +475,18 @@ class FileBackendMultiWrite extends FileBackend { $status->merge( $masterStatus ); // Propagate the operations to the clone backends... foreach ( $this->backends as $index => $backend ) { - if ( $index !== $this->masterIndex ) { // not done already - $realOps = $this->substOpBatchPaths( $ops, $backend ); + if ( $index === $this->masterIndex ) { + continue; // done already + } + + $realOps = $this->substOpBatchPaths( $ops, $backend ); + if ( $this->asyncWrites ) { + DeferredUpdates::addCallableUpdate( + function() use ( $backend, $realOps ) { + $backend->doQuickOperations( $realOps ); + } + ); + } else { $status->merge( $backend->doQuickOperations( $realOps ) ); } } @@ -473,40 +501,48 @@ class FileBackendMultiWrite extends FileBackend { } protected function doPrepare( array $params ) { - $status = Status::newGood(); - foreach ( $this->backends as $index => $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $status->merge( $backend->doPrepare( $realParams ) ); - } - - return $status; + return $this->doDirectoryOp( 'prepare', $params ); } protected function doSecure( array $params ) { - $status = Status::newGood(); - foreach ( $this->backends as $index => $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $status->merge( $backend->doSecure( $realParams ) ); - } - - return $status; + return $this->doDirectoryOp( 'secure', $params ); } protected function doPublish( array $params ) { - $status = Status::newGood(); - foreach ( $this->backends as $index => $backend ) { - $realParams = $this->substOpPaths( $params, $backend ); - $status->merge( $backend->doPublish( $realParams ) ); - } - - return $status; + return $this->doDirectoryOp( 'publish', $params ); } protected function doClean( array $params ) { + return $this->doDirectoryOp( 'clean', $params ); + } + + /** + * @param string $method One of (doPrepare,doSecure,doPublish,doClean) + * @param array $params Method arguments + * @return Status + */ + protected function doDirectoryOp( $method, array $params ) { $status = Status::newGood(); + + $realParams = $this->substOpPaths( $params, $this->backends[$this->masterIndex] ); + $masterStatus = $this->backends[$this->masterIndex]->$method( $realParams ); + $status->merge( $masterStatus ); + foreach ( $this->backends as $index => $backend ) { + if ( $index === $this->masterIndex ) { + continue; // already done + } + $realParams = $this->substOpPaths( $params, $backend ); - $status->merge( $backend->doClean( $realParams ) ); + if ( $this->asyncWrites ) { + DeferredUpdates::addCallableUpdate( + function() use ( $backend, $method, $realParams ) { + $backend->$method( $realParams ); + } + ); + } else { + $status->merge( $backend->$method( $realParams ) ); + } } return $status; diff --git a/tests/phpunit/includes/filebackend/FileBackendTest.php b/tests/phpunit/includes/filebackend/FileBackendTest.php index d40e5274fa..0d15b75bfb 100644 --- a/tests/phpunit/includes/filebackend/FileBackendTest.php +++ b/tests/phpunit/includes/filebackend/FileBackendTest.php @@ -80,6 +80,11 @@ class FileBackendTest extends MediaWikiTestCase { ) ); } + protected function tearDown() { + parent::tearDown(); + DeferredUpdates::forceDeferral( false ); + } + private static function baseStorePath() { return 'mwstore://localtesting'; } @@ -2442,6 +2447,54 @@ class FileBackendTest extends MediaWikiTestCase { ); } + public function testAsyncWrites() { + $be = TestingAccessWrapper::newFromObject( + new FileBackendMultiWrite( array( + 'name' => 'localtesting', + 'wikiId' => wfWikiId() . mt_rand(), + 'backends' => array( + array( // backend 0 + 'name' => 'multitesting0', + 'class' => 'MemoryFileBackend', + 'isMultiMaster' => false + ), + array( // backend 1 + 'name' => 'multitesting1', + 'class' => 'MemoryFileBackend', + 'isMultiMaster' => true + ) + ), + 'replication' => 'async' + ) ) + ); + + DeferredUpdates::forceDeferral( true ); + + $p = 'container/test-cont/file.txt'; + $be->quickCreate( array( + 'dst' => "mwstore://localtesting/$p", 'content' => 'cattitude' ) ); + + $this->assertEquals( + false, + $be->backends[0]->getFileContents( array( 'src' => "mwstore://multitesting0/$p" ) ), + "File not yet written to backend 0" + ); + $this->assertEquals( + 'cattitude', + $be->backends[1]->getFileContents( array( 'src' => "mwstore://multitesting1/$p" ) ), + "File already written to backend 1" + ); + + DeferredUpdates::doUpdates(); + DeferredUpdates::forceDeferral( false ); + + $this->assertEquals( + 'cattitude', + $be->backends[0]->getFileContents( array( 'src' => "mwstore://multitesting0/$p" ) ), + "File now written to backend 0" + ); + } + // helper function private function listToArray( $iter ) { return is_array( $iter ) ? $iter : iterator_to_array( $iter ); -- 2.20.1