From 205cfc185446ad9dd355d3a57f4ee60d0dc1de57 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Wed, 9 May 2018 14:44:34 -0700 Subject: [PATCH] rdbms: fix LBFactory::commitAll() round handling This avoids "Transaction round stage must be approved (not cursory)". Bug: T194308 Change-Id: I9dbfe9cede02b1b1904c1d5e5a9802306c2492a2 --- includes/libs/rdbms/lbfactory/LBFactory.php | 3 ++- includes/libs/rdbms/loadbalancer/ILoadBalancer.php | 11 ++++++++++- includes/libs/rdbms/loadbalancer/LoadBalancer.php | 2 +- tests/phpunit/includes/db/LBFactoryTest.php | 8 ++++++++ 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php index fe185361a2..38c7a5c7e9 100644 --- a/includes/libs/rdbms/lbfactory/LBFactory.php +++ b/includes/libs/rdbms/lbfactory/LBFactory.php @@ -216,7 +216,8 @@ abstract class LBFactory implements ILBFactory { final public function commitAll( $fname = __METHOD__, array $options = [] ) { $this->commitMasterChanges( $fname, $options ); - $this->forEachLBCallMethod( 'commitAll', [ $fname ] ); + $this->forEachLBCallMethod( 'flushMasterSnapshots', [ $fname ] ); + $this->forEachLBCallMethod( 'flushReplicaSnapshots', [ $fname ] ); } final public function beginMasterChanges( $fname = __METHOD__ ) { diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index 5d217e201c..850f9afab8 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -439,12 +439,21 @@ interface ILoadBalancer { public function rollbackMasterChanges( $fname = __METHOD__ ); /** - * Commit all replica DB transactions so as to flush any REPEATABLE-READ or SSI snapshot + * Commit all replica DB transactions so as to flush any REPEATABLE-READ or SSI snapshots * * @param string $fname Caller name */ public function flushReplicaSnapshots( $fname = __METHOD__ ); + /** + * Commit all master DB transactions so as to flush any REPEATABLE-READ or SSI snapshots + * + * An error will be thrown if a connection has pending writes or callbacks + * + * @param string $fname Caller name + */ + public function flushMasterSnapshots( $fname = __METHOD__ ); + /** * @return bool Whether a master connection is already open */ diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index cb6e4f4fa9..405ed14160 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -1565,7 +1565,7 @@ class LoadBalancer implements ILoadBalancer { } ); } - private function flushMasterSnapshots( $fname = __METHOD__ ) { + public function flushMasterSnapshots( $fname = __METHOD__ ) { $this->forEachOpenMasterConnection( function ( IDatabase $conn ) use ( $fname ) { $conn->flushSnapshot( $fname ); } ); diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php index b715b1f92f..a84cc04514 100644 --- a/tests/phpunit/includes/db/LBFactoryTest.php +++ b/tests/phpunit/includes/db/LBFactoryTest.php @@ -211,6 +211,14 @@ class LBFactoryTest extends MediaWikiTestCase { } ); $this->assertEquals( 2, $count ); + // DBTransactionError should not be thrown + $ran = 0; + $dbw->onTransactionPreCommitOrIdle( function () use ( &$ran ) { + ++$ran; + } ); + $factory->commitAll( __METHOD__ ); + $this->assertEquals( 1, $ran ); + $factory->shutdown(); $factory->closeAll(); } -- 2.20.1