From: Aaron Schulz Date: Wed, 29 May 2019 21:07:03 +0000 (-0700) Subject: Make sure that each DataUpdate still has outer transaction scope X-Git-Tag: 1.34.0-rc.0~1552^2 X-Git-Url: http://git.cyclocoop.org/%7B%24admin_url%7Dcompta/comptes/%27.%24imageUrl.%27?a=commitdiff_plain;h=3496f0fca3debf932598087607dc5547075e2cba;p=lhc%2Fweb%2Fwiklou.git Make sure that each DataUpdate still has outer transaction scope Bug: T221577 Change-Id: I620e461d791416ca37fa9ca4fca501e28d778cf5 --- diff --git a/includes/Storage/DerivedPageDataUpdater.php b/includes/Storage/DerivedPageDataUpdater.php index ff5541d800..53fe61573d 100644 --- a/includes/Storage/DerivedPageDataUpdater.php +++ b/includes/Storage/DerivedPageDataUpdater.php @@ -48,6 +48,7 @@ use MediaWiki\Revision\SlotRoleRegistry; use MediaWiki\Revision\SlotRecord; use MediaWiki\User\UserIdentity; use MessageCache; +use MWCallableUpdate; use ParserCache; use ParserOptions; use ParserOutput; @@ -1423,12 +1424,18 @@ class DerivedPageDataUpdater implements IDBAccessObject { } // Defer the getCannonicalParserOutput() call triggered by getSecondaryDataUpdates() - DeferredUpdates::addCallableUpdate( function () { - $this->doSecondaryDataUpdates( [ - // T52785 do not update any other pages on a null edit - 'recursive' => $this->options['changed'] - ] ); - } ); + // by wrapping the code that schedules the secondary updates in a callback itself + $wrapperUpdate = new MWCallableUpdate( + function () { + $this->doSecondaryDataUpdates( [ + // T52785 do not update any other pages on a null edit + 'recursive' => $this->options['changed'] + ] ); + }, + __METHOD__ + ); + $wrapperUpdate->setTransactionRoundRequirement( $wrapperUpdate::TRX_ROUND_ABSENT ); + DeferredUpdates::addUpdate( $wrapperUpdate ); // TODO: MCR: check if *any* changed slot supports categories! if ( $this->rcWatchCategoryMembership @@ -1569,24 +1576,15 @@ class DerivedPageDataUpdater implements IDBAccessObject { * current page or otherwise depend on it (default: false) * - defer: one of the DeferredUpdates constants, or false to run immediately after waiting * for replication of the changes from the SecondaryDataUpdates hooks (default: false) - * - transactionTicket: a transaction ticket from LBFactory::getEmptyTransactionTicket(), - * only when defer is false (default: null) * @since 1.32 */ public function doSecondaryDataUpdates( array $options = [] ) { $this->assertHasRevision( __METHOD__ ); - $options += [ - 'recursive' => false, - 'defer' => false, - 'transactionTicket' => null, - ]; + $options += [ 'recursive' => false, 'defer' => false ]; $deferValues = [ false, DeferredUpdates::PRESEND, DeferredUpdates::POSTSEND ]; if ( !in_array( $options['defer'], $deferValues, true ) ) { throw new InvalidArgumentException( 'Invalid value for defer: ' . $options['defer'] ); } - Assert::parameterType( - 'integer|null', $options['transactionTicket'], '$options[\'transactionTicket\']' ); - $updates = $this->getSecondaryDataUpdates( $options['recursive'] ); $triggeringUser = $this->options['triggeringUser'] ?? $this->user; @@ -1596,14 +1594,6 @@ class DerivedPageDataUpdater implements IDBAccessObject { $causeAction = $this->options['causeAction'] ?? 'unknown'; $causeAgent = $this->options['causeAgent'] ?? 'unknown'; $legacyRevision = new Revision( $this->revision ); - $ticket = $options['transactionTicket']; - - if ( $options['defer'] === false && $ticket !== null ) { - // For legacy hook handlers doing updates via LinksUpdateConstructed, make sure - // any pending writes they made get flushed before the doUpdate() calls below. - // This avoids snapshot-clearing errors in LinksUpdate::acquirePageLock(). - $this->loadbalancerFactory->commitAndWaitForReplication( __METHOD__, $ticket ); - } foreach ( $updates as $update ) { if ( $update instanceof DataUpdate ) { @@ -1613,13 +1603,16 @@ class DerivedPageDataUpdater implements IDBAccessObject { $update->setRevision( $legacyRevision ); $update->setTriggeringUser( $triggeringUser ); } + } - if ( $options['defer'] === false ) { - if ( $update instanceof DataUpdate && $ticket !== null ) { - $update->setTransactionTicket( $ticket ); - } - $update->doUpdate(); - } else { + if ( $options['defer'] === false ) { + // T221577: flush any transaction; each update needs outer transaction scope + $this->loadbalancerFactory->commitMasterChanges( __METHOD__ ); + foreach ( $updates as $update ) { + DeferredUpdates::attemptUpdate( $update, $this->loadbalancerFactory ); + } + } else { + foreach ( $updates as $update ) { DeferredUpdates::addUpdate( $update, $options['defer'] ); } } diff --git a/includes/deferred/DeferredUpdates.php b/includes/deferred/DeferredUpdates.php index 4b54378182..f5d22c15d6 100644 --- a/includes/deferred/DeferredUpdates.php +++ b/includes/deferred/DeferredUpdates.php @@ -184,8 +184,6 @@ class DeferredUpdates { $lbFactory = $services->getDBLoadBalancerFactory(); $method = RequestContext::getMain()->getRequest()->getMethod(); - $ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ ); - /** @var ErrorPageError $reportableError */ $reportableError = null; /** @var DeferrableUpdate[] $updates Snapshot of queue */ @@ -199,7 +197,6 @@ class DeferredUpdates { $updatesByType = [ 'data' => [], 'generic' => [] ]; foreach ( $updates as $du ) { if ( $du instanceof DataUpdate ) { - $du->setTransactionTicket( $ticket ); $updatesByType['data'][] = $du; } else { $updatesByType['generic'][] = $du; @@ -225,10 +222,6 @@ class DeferredUpdates { $firstKey = key( self::$executeContext['subqueue'] ); unset( self::$executeContext['subqueue'][$firstKey] ); - if ( $subUpdate instanceof DataUpdate ) { - $subUpdate->setTransactionTicket( $ticket ); - } - $guiError = self::handleUpdate( $subUpdate, $lbFactory, $mode, $stage ); $reportableError = $reportableError ?: $guiError; } @@ -300,6 +293,10 @@ class DeferredUpdates { * @since 1.34 */ public static function attemptUpdate( DeferrableUpdate $update, ILBFactory $lbFactory ) { + if ( $update instanceof DataUpdate ) { + $update->setTransactionTicket( $lbFactory->getEmptyTransactionTicket( __METHOD__ ) ); + } + if ( $update instanceof TransactionRoundAwareUpdate && $update->getTransactionRoundRequirement() == $update::TRX_ROUND_ABSENT diff --git a/includes/deferred/MWCallableUpdate.php b/includes/deferred/MWCallableUpdate.php index efca406881..707035c3f8 100644 --- a/includes/deferred/MWCallableUpdate.php +++ b/includes/deferred/MWCallableUpdate.php @@ -5,11 +5,15 @@ use Wikimedia\Rdbms\IDatabase; /** * Deferrable Update for closure/callback */ -class MWCallableUpdate implements DeferrableUpdate, DeferrableCallback { - /** @var callable|null */ +class MWCallableUpdate + implements DeferrableUpdate, DeferrableCallback, TransactionRoundAwareUpdate +{ + /** @var callable|null Callback, or null if it was cancelled */ private $callback; - /** @var string */ + /** @var string Calling method name */ private $fname; + /** @var int One of the class TRX_ROUND_* constants */ + private $trxRoundRequirement = self::TRX_ROUND_PRESENT; /** * @param callable $callback @@ -48,4 +52,16 @@ class MWCallableUpdate implements DeferrableUpdate, DeferrableCallback { public function getOrigin() { return $this->fname; } + + /** + * @since 1.34 + * @param int $mode One of the class TRX_ROUND_* constants + */ + public function setTransactionRoundRequirement( $mode ) { + $this->trxRoundRequirement = $mode; + } + + public function getTransactionRoundRequirement() { + return $this->trxRoundRequirement; + } } diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php b/includes/jobqueue/jobs/RefreshLinksJob.php index a0b6e07aef..89ecb0ee92 100644 --- a/includes/jobqueue/jobs/RefreshLinksJob.php +++ b/includes/jobqueue/jobs/RefreshLinksJob.php @@ -279,8 +279,7 @@ class RefreshLinksJob extends Job { 'recursive' => !empty( $this->params['useRecursiveLinksUpdate'] ), // Carry over cause so the update can do extra logging 'causeAction' => $this->params['causeAction'], - 'causeAgent' => $this->params['causeAgent'], - 'transactionTicket' => $ticket + 'causeAgent' => $this->params['causeAgent'] ]; if ( !empty( $this->params['triggeringUser'] ) ) { $userInfo = $this->params['triggeringUser']; diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php index 863a3f307e..3814112f6a 100644 --- a/includes/page/WikiPage.php +++ b/includes/page/WikiPage.php @@ -2106,8 +2106,6 @@ class WikiPage implements Page, IDBAccessObject { * - defer: one of the DeferredUpdates constants, or false to run immediately (default: false). * Note that even when this is set to false, some updates might still get deferred (as * some update might directly add child updates to DeferredUpdates). - * - transactionTicket: a transaction ticket from LBFactory::getEmptyTransactionTicket(), - * only when defer is false (default: null) * @since 1.32 */ public function doSecondaryDataUpdates( array $options = [] ) {