Seb35 has uploaded a new change for review. ( 
https://gerrit.wikimedia.org/r/358461 )

Change subject: Make DeferredUpdates detect LBFactory transaction rounds
......................................................................

Make DeferredUpdates detect LBFactory transaction rounds

Previously, tryOpportunisticExecute() tried to nest transaction rounds,
which would fail. Added LBFactory::hasTransactionRound() as needed.

Also cleaned up some unqualified class names in callbacks and set the
PRESEND flag for the JobQueueDB AutoCommitUpdate callback. Use the
proper getMasterDB() method while at it. These follow up 24842cfac.

Bug: T154425
Change-Id: Ib1d38f68bd217903d1a7d46fb15b7d7d9620daa6
(cherry picked from commit 95fdff36c251d9d1807c7b5b0bf84acc457f0d1e)
---
M includes/MediaWiki.php
M includes/deferred/DeferredUpdates.php
M includes/jobqueue/JobQueueDB.php
M includes/jobqueue/JobRunner.php
M includes/libs/rdbms/lbfactory/ILBFactory.php
M includes/libs/rdbms/lbfactory/LBFactory.php
6 files changed, 32 insertions(+), 17 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/61/358461/1

diff --git a/includes/MediaWiki.php b/includes/MediaWiki.php
index ba1c8c8..f129a8b 100644
--- a/includes/MediaWiki.php
+++ b/includes/MediaWiki.php
@@ -887,9 +887,8 @@
                        __METHOD__
                );
 
-               // Push lazilly-pushed jobs
                // Important: this must be the last deferred update added 
(T100085, T154425)
-               DeferredUpdates::addCallableUpdate( [ 'JobQueueGroup', 
'pushLazyJobs' ] );
+               DeferredUpdates::addCallableUpdate( [ JobQueueGroup::class, 
'pushLazyJobs' ] );
 
                // Do any deferred jobs
                DeferredUpdates::doUpdates( 'enqueue' );
diff --git a/includes/deferred/DeferredUpdates.php 
b/includes/deferred/DeferredUpdates.php
index fd3a1af..069b092 100644
--- a/includes/deferred/DeferredUpdates.php
+++ b/includes/deferred/DeferredUpdates.php
@@ -283,7 +283,7 @@
                }
 
                // Avoiding running updates without them having outer scope
-               if ( !self::getBusyDbConnections() ) {
+               if ( !self::areDatabaseTransactionsActive() ) {
                        self::doUpdates( $mode );
                        return true;
                }
@@ -337,16 +337,19 @@
        }
 
        /**
-        * @return IDatabase[] Connection where commit() cannot be called yet
+        * @return bool If a transaction round is active or connection is not 
ready for commit()
         */
-       private static function getBusyDbConnections() {
-               $connsBusy = [];
-
+       private static function areDatabaseTransactionsActive() {
                $lbFactory = 
MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
+               if ( $lbFactory->hasTransactionRound() ) {
+                       return true;
+               }
+
+               $connsBusy = false;
                $lbFactory->forEachLB( function ( LoadBalancer $lb ) use ( 
&$connsBusy ) {
                        $lb->forEachOpenMasterConnection( function ( IDatabase 
$conn ) use ( &$connsBusy ) {
                                if ( $conn->writesOrCallbacksPending() || 
$conn->explicitTrxActive() ) {
-                                       $connsBusy[] = $conn;
+                                       $connsBusy = true;
                                }
                        } );
                } );
diff --git a/includes/jobqueue/JobQueueDB.php b/includes/jobqueue/JobQueueDB.php
index ea2b113..345b591 100644
--- a/includes/jobqueue/JobQueueDB.php
+++ b/includes/jobqueue/JobQueueDB.php
@@ -181,13 +181,16 @@
         * @return void
         */
        protected function doBatchPush( array $jobs, $flags ) {
-               DeferredUpdates::addUpdate( new AutoCommitUpdate(
-                       wfGetDB( DB_MASTER ),
-                       __METHOD__,
-                       function ( IDatabase $dbw, $fname ) use ( $jobs, $flags 
) {
-                               $this->doBatchPushInternal( $dbw, $jobs, 
$flags, $fname );
-                       }
-               ) );
+               DeferredUpdates::addUpdate(
+                       new AutoCommitUpdate(
+                               $this->getMasterDB(),
+                               __METHOD__,
+                               function ( IDatabase $dbw, $fname ) use ( 
$jobs, $flags ) {
+                                       $this->doBatchPushInternal( $dbw, 
$jobs, $flags, $fname );
+                               }
+                       ),
+                       DeferredUpdates::PRESEND
+               );
        }
 
        /**
diff --git a/includes/jobqueue/JobRunner.php b/includes/jobqueue/JobRunner.php
index 127f929..5ed31e0 100644
--- a/includes/jobqueue/JobRunner.php
+++ b/includes/jobqueue/JobRunner.php
@@ -275,9 +275,8 @@
                        $status = $job->run();
                        $error = $job->getLastError();
                        $this->commitMasterChanges( $lbFactory, $job, 
$fnameTrxOwner );
-                       // Push lazilly-pushed jobs
                        // Important: this must be the last deferred update 
added (T100085, T154425)
-                       DeferredUpdates::addCallableUpdate( [ 'JobQueueGroup', 
'pushLazyJobs' ] );
+                       DeferredUpdates::addCallableUpdate( [ 
JobQueueGroup::class, 'pushLazyJobs' ] );
                        // Run any deferred update tasks; doUpdates() manages 
transactions itself
                        DeferredUpdates::doUpdates();
                } catch ( Exception $e ) {
diff --git a/includes/libs/rdbms/lbfactory/ILBFactory.php 
b/includes/libs/rdbms/lbfactory/ILBFactory.php
index ff1bd43..ceab5b9 100644
--- a/includes/libs/rdbms/lbfactory/ILBFactory.php
+++ b/includes/libs/rdbms/lbfactory/ILBFactory.php
@@ -173,6 +173,13 @@
        public function rollbackMasterChanges( $fname = __METHOD__ );
 
        /**
+        * Check if a transaction round is active
+        * @return bool
+        * @since 1.29
+        */
+       public function hasTransactionRound();
+
+       /**
         * Determine if any master connection has pending changes
         * @return bool
         */
diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php 
b/includes/libs/rdbms/lbfactory/LBFactory.php
index d21289b..784c2d1 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -243,6 +243,10 @@
                } );
        }
 
+       public function hasTransactionRound() {
+               return ( $this->trxRoundId !== false );
+       }
+
        /**
         * Log query info if multi DB transactions are going to be committed now
         */

-- 
To view, visit https://gerrit.wikimedia.org/r/358461
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib1d38f68bd217903d1a7d46fb15b7d7d9620daa6
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: REL1_28
Gerrit-Owner: Seb35 <se...@seb35.fr>
Gerrit-Reviewer: Aaron Schulz <asch...@wikimedia.org>

_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits

Reply via email to