[MediaWiki-commits] [Gerrit] mediawiki/core[REL1_28]: Make DeferredUpdates detect LBFactory transaction rounds

[jenkins-bot (Code Review)]

jenkins-bot has submitted this change and it was merged. ( 
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(-)

Approvals:
  Seb35: Looks good to me, but someone else must approve
  Chad: Looks good to me, approved
  jenkins-bot: Verified



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 @@

[MediaWiki-commits] [Gerrit] mediawiki/core[REL1_28]: Make DeferredUpdates detect LBFactory transaction rounds

[Seb35 (Code Review)]

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 =