Introduce TransactionRoundAwareUpdate interface

Refactor the deferred update transaction round handling code to use
a new TransactionRoundAwareUpdate interface. Also, rename a few
DeferredUpdates methods so they do not give the impression that
doUpdates() is always called.

Change-Id: Idc4c6d81c4e2ca0ce41bca1e7800f797fa7e37f6
This commit is contained in:
Aaron Schulz 2019-05-21 14:52:57 -07:00
parent 07a883388a
commit ef949d79d2
5 changed files with 81 additions and 28 deletions

View file

@ -1496,6 +1496,7 @@ $wgAutoloadLocalClasses = [
'TrackBlobs' => __DIR__ . '/maintenance/storage/trackBlobs.php',
'TrackingCategories' => __DIR__ . '/includes/TrackingCategories.php',
'TraditionalImageGallery' => __DIR__ . '/includes/gallery/TraditionalImageGallery.php',
'TransactionRoundAwareUpdate' => __DIR__ . '/includes/deferred/TransactionRoundAwareUpdate.php',
'TransactionRoundDefiningUpdate' => __DIR__ . '/includes/deferred/TransactionRoundDefiningUpdate.php',
'TransformParameterError' => __DIR__ . '/includes/media/TransformParameterError.php',
'TransformTooBigImageAreaError' => __DIR__ . '/includes/media/TransformTooBigImageAreaError.php',

View file

@ -22,6 +22,7 @@
use Wikimedia\Rdbms\IDatabase;
use MediaWiki\MediaWikiServices;
use Wikimedia\Rdbms\LBFactory;
use Wikimedia\Rdbms\ILBFactory;
use Wikimedia\Rdbms\LoadBalancer;
/**
@ -136,11 +137,11 @@ class DeferredUpdates {
// Normally, these use the subqueue, but that isn't true for MergeableUpdate items.
do {
if ( $stage === self::ALL || $stage === self::PRESEND ) {
self::execute( self::$preSendUpdates, $mode, $stageEffective );
self::handleUpdateQueue( self::$preSendUpdates, $mode, $stageEffective );
}
if ( $stage === self::ALL || $stage == self::POSTSEND ) {
self::execute( self::$postSendUpdates, $mode, $stageEffective );
self::handleUpdateQueue( self::$postSendUpdates, $mode, $stageEffective );
}
} while ( $stage === self::ALL && self::$preSendUpdates );
}
@ -169,15 +170,15 @@ class DeferredUpdates {
}
/**
* Immediately run/queue a list of updates
* Immediately run or enqueue a list of updates
*
* @param DeferrableUpdate[] &$queue List of DeferrableUpdate objects
* @param string $mode Use "enqueue" to use the job queue when possible
* @param string $mode Either "run" or "enqueue" (to use the job queue when possible)
* @param int $stage Class constant (PRESEND, POSTSEND) (since 1.28)
* @throws ErrorPageError Happens on top-level calls
* @throws Exception Happens on second-level calls
*/
protected static function execute( array &$queue, $mode, $stage ) {
protected static function handleUpdateQueue( array &$queue, $mode, $stage ) {
$services = MediaWikiServices::getInstance();
$stats = $services->getStatsdDataFactory();
$lbFactory = $services->getDBLoadBalancerFactory();
@ -216,7 +217,7 @@ class DeferredUpdates {
self::$executeContext = [ 'stage' => $stage, 'subqueue' => [] ];
try {
/** @var DeferrableUpdate $update */
$guiError = self::runUpdate( $update, $lbFactory, $mode, $stage );
$guiError = self::handleUpdate( $update, $lbFactory, $mode, $stage );
$reportableError = $reportableError ?: $guiError;
// Do the subqueue updates for $update until there are none
while ( self::$executeContext['subqueue'] ) {
@ -228,7 +229,7 @@ class DeferredUpdates {
$subUpdate->setTransactionTicket( $ticket );
}
$guiError = self::runUpdate( $subUpdate, $lbFactory, $mode, $stage );
$guiError = self::handleUpdate( $subUpdate, $lbFactory, $mode, $stage );
$reportableError = $reportableError ?: $guiError;
}
} finally {
@ -249,13 +250,15 @@ class DeferredUpdates {
}
/**
* Run or enqueue an update
*
* @param DeferrableUpdate $update
* @param LBFactory $lbFactory
* @param string $mode
* @param int $stage
* @return ErrorPageError|null
*/
private static function runUpdate(
private static function handleUpdate(
DeferrableUpdate $update, LBFactory $lbFactory, $mode, $stage
) {
$guiError = null;
@ -265,21 +268,15 @@ class DeferredUpdates {
$spec = $update->getAsJobSpecification();
$domain = $spec['domain'] ?? $spec['wiki'];
JobQueueGroup::singleton( $domain )->push( $spec['job'] );
} elseif ( $update instanceof TransactionRoundDefiningUpdate ) {
$update->doUpdate();
} else {
// Run the bulk of the update now
$fnameTrxOwner = get_class( $update ) . '::doUpdate';
$lbFactory->beginMasterChanges( $fnameTrxOwner );
$update->doUpdate();
$lbFactory->commitMasterChanges( $fnameTrxOwner );
self::attemptUpdate( $update, $lbFactory );
}
} catch ( Exception $e ) {
// Reporting GUI exceptions does not work post-send
if ( $e instanceof ErrorPageError && $stage === self::PRESEND ) {
$guiError = $e;
}
MWExceptionHandler::rollbackMasterChangesAndLog( $e );
$lbFactory->rollbackMasterChanges( __METHOD__ );
// VW-style hack to work around T190178, so we can make sure
// PageMetaDataUpdater doesn't throw exceptions.
@ -291,6 +288,32 @@ class DeferredUpdates {
return $guiError;
}
/**
* Attempt to run an update with the appropriate transaction round state it expects
*
* DeferredUpdate classes that wrap the execution of bundles of other DeferredUpdate
* instances can use this method to run the updates. Any such wrapper class should
* always use TRX_ROUND_ABSENT itself.
*
* @param DeferrableUpdate $update
* @param ILBFactory $lbFactory
* @since 1.34
*/
public static function attemptUpdate( DeferrableUpdate $update, ILBFactory $lbFactory ) {
if (
$update instanceof TransactionRoundAwareUpdate &&
$update->getTransactionRoundRequirement() == $update::TRX_ROUND_ABSENT
) {
$update->doUpdate();
} else {
// Run the bulk of the update now
$fnameTrxOwner = get_class( $update ) . '::doUpdate';
$lbFactory->beginMasterChanges( $fnameTrxOwner );
$update->doUpdate();
$lbFactory->commitMasterChanges( $fnameTrxOwner );
}
}
/**
* Run all deferred updates immediately if there are no DB writes active
*

View file

@ -0,0 +1,19 @@
<?php
/**
* Deferrable update that specifies whether it must run outside of any explicit
* LBFactory transaction round or must run inside of a round owned by doUpdate().
*
* @since 1.34
*/
interface TransactionRoundAwareUpdate {
/** @var int No transaction round should be used */
const TRX_ROUND_ABSENT = 1;
/** @var int A transaction round owned by self::doUpdate should be used */
const TRX_ROUND_PRESENT = 2;
/**
* @return int One of the class TRX_ROUND_* constants
*/
public function getTransactionRoundRequirement();
}

View file

@ -5,7 +5,9 @@
*
* @since 1.31
*/
class TransactionRoundDefiningUpdate implements DeferrableUpdate, DeferrableCallback {
class TransactionRoundDefiningUpdate
implements DeferrableUpdate, DeferrableCallback, TransactionRoundAwareUpdate
{
/** @var callable|null */
private $callback;
/** @var string */
@ -27,4 +29,12 @@ class TransactionRoundDefiningUpdate implements DeferrableUpdate, DeferrableCall
public function getOrigin() {
return $this->fname;
}
/**
* @return int One of the class TRX_ROUND_* constants
* @since 1.34
*/
final public function getTransactionRoundRequirement() {
return self::TRX_ROUND_ABSENT;
}
}

View file

@ -8,8 +8,8 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
* @covers DeferredUpdates::addUpdate
* @covers DeferredUpdates::push
* @covers DeferredUpdates::doUpdates
* @covers DeferredUpdates::execute
* @covers DeferredUpdates::runUpdate
* @covers DeferredUpdates::handleUpdateQueue
* @covers DeferredUpdates::attemptUpdate
*/
public function testAddAndRun() {
$update = $this->getMockBuilder( DeferrableUpdate::class )
@ -92,7 +92,7 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
/**
* @covers DeferredUpdates::doUpdates
* @covers DeferredUpdates::execute
* @covers DeferredUpdates::handleUpdateQueue
* @covers DeferredUpdates::addUpdate
*/
public function testDoUpdatesWeb() {
@ -189,7 +189,7 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
/**
* @covers DeferredUpdates::doUpdates
* @covers DeferredUpdates::execute
* @covers DeferredUpdates::handleUpdateQueue
* @covers DeferredUpdates::addUpdate
*/
public function testDoUpdatesCLI() {
@ -263,7 +263,7 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
/**
* @covers DeferredUpdates::doUpdates
* @covers DeferredUpdates::execute
* @covers DeferredUpdates::handleUpdateQueue
* @covers DeferredUpdates::addUpdate
*/
public function testPresendAddOnPostsendRun() {
@ -295,7 +295,7 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
}
/**
* @covers DeferredUpdates::runUpdate
* @covers DeferredUpdates::attemptUpdate
*/
public function testRunUpdateTransactionScope() {
$this->setMwGlobals( 'wgCommandLineMode', false );
@ -315,7 +315,7 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
}
/**
* @covers DeferredUpdates::runUpdate
* @covers DeferredUpdates::attemptUpdate
* @covers TransactionRoundDefiningUpdate::getOrigin
*/
public function testRunOuterScopeUpdate() {
@ -326,10 +326,10 @@ class DeferredUpdatesTest extends MediaWikiTestCase {
$ran = 0;
DeferredUpdates::addUpdate( new TransactionRoundDefiningUpdate(
function () use ( &$ran, $lbFactory ) {
$ran++;
$this->assertFalse( $lbFactory->hasTransactionRound(), 'No transaction' );
} )
function () use ( &$ran, $lbFactory ) {
$ran++;
$this->assertFalse( $lbFactory->hasTransactionRound(), 'No transaction' );
} )
);
DeferredUpdates::doUpdates();