From 67f08d69904f248594d20a9513d5d9fd3d80e6c4 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sun, 21 Aug 2016 22:35:12 -0700 Subject: [PATCH] Add LBFactory::beginMasterChanges() for doing DBO_TRX rounds This is in intended to replace the DataUpdate transaction round logic. It could also be useful for doing transaction rounds in maintenance scripts. Also renamed $db => $conn in a few LB methods for consistency. Change-Id: If21c2ba5e8bac48c250b96137279e7edaa8289f7 --- includes/db/DBConnRef.php | 8 +++- includes/db/Database.php | 26 +++++++++- includes/db/IDatabase.php | 23 ++++++++- includes/db/loadbalancer/LBFactory.php | 16 +++++++ includes/db/loadbalancer/LoadBalancer.php | 48 ++++++++++++++++--- tests/phpunit/includes/db/DatabaseTest.php | 39 +++++++++++++++ .../installer/DatabaseUpdaterTest.php | 4 +- 7 files changed, 150 insertions(+), 14 deletions(-) diff --git a/includes/db/DBConnRef.php b/includes/db/DBConnRef.php index e5b6d050d8..790a073d7f 100644 --- a/includes/db/DBConnRef.php +++ b/includes/db/DBConnRef.php @@ -115,11 +115,15 @@ class DBConnRef implements IDatabase { return $this->__call( __FUNCTION__, func_get_args() ); } - public function setFlag( $flag ) { + public function setFlag( $flag, $remember = self::REMEMBER_NOTHING ) { return $this->__call( __FUNCTION__, func_get_args() ); } - public function clearFlag( $flag ) { + public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING ) { + return $this->__call( __FUNCTION__, func_get_args() ); + } + + public function restoreFlags( $state = self::RESTORE_PRIOR ) { return $this->__call( __FUNCTION__, func_get_args() ); } diff --git a/includes/db/Database.php b/includes/db/Database.php index 6ddc9f7a41..59c1207e49 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -198,6 +198,9 @@ abstract class DatabaseBase implements IDatabase { /** @var float UNIX timestamp */ protected $lastPing = 0.0; + /** @var int[] Prior mFlags values */ + private $priorFlags = []; + /** @var TransactionProfiler */ protected $trxProfiler; @@ -430,14 +433,33 @@ abstract class DatabaseBase implements IDatabase { return $this->mOpened; } - public function setFlag( $flag ) { + public function setFlag( $flag, $remember = self::REMEMBER_NOTHING ) { + if ( $remember === self::REMEMBER_PRIOR ) { + array_push( $this->priorFlags, $this->mFlags ); + } $this->mFlags |= $flag; } - public function clearFlag( $flag ) { + public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING ) { + if ( $remember === self::REMEMBER_PRIOR ) { + array_push( $this->priorFlags, $this->mFlags ); + } $this->mFlags &= ~$flag; } + public function restoreFlags( $state = self::RESTORE_PRIOR ) { + if ( !$this->priorFlags ) { + return; + } + + if ( $state === self::RESTORE_INITIAL ) { + $this->mFlags = reset( $this->priorFlags ); + $this->priorFlags = []; + } else { + $this->mFlags = array_pop( $this->priorFlags ); + } + } + public function getFlag( $flag ) { return !!( $this->mFlags & $flag ); } diff --git a/includes/db/IDatabase.php b/includes/db/IDatabase.php index 25100db0dc..1aa931e63f 100644 --- a/includes/db/IDatabase.php +++ b/includes/db/IDatabase.php @@ -50,6 +50,15 @@ interface IDatabase { /** @var string Transaction operation comes from the database class internally */ const FLUSHING_INTERNAL = 'flush'; + /** @var string No not remember the prior flags */ + const REMEMBER_NOTHING = ''; + /** @var string Remember the prior flags */ + const REMEMBER_PRIOR = 'remember'; + /** @var string Restore to the prior flag state */ + const RESTORE_PRIOR = 'prior'; + /** @var string Restore to the initial flag state */ + const RESTORE_INITIAL = 'initial'; + /** * A string describing the current software version, and possibly * other details in a user-friendly way. Will be listed on Special:Version, etc. @@ -230,8 +239,9 @@ interface IDatabase { * - DBO_DEFAULT: automatically sets DBO_TRX if not in command line mode * and removes it in command line mode * - DBO_PERSISTENT: use persistant database connection + * @param string $remember IDatabase::REMEMBER_* constant [default: REMEMBER_NOTHING] */ - public function setFlag( $flag ); + public function setFlag( $flag, $remember = self::REMEMBER_NOTHING ); /** * Clear a flag for this connection @@ -243,8 +253,17 @@ interface IDatabase { * - DBO_DEFAULT: automatically sets DBO_TRX if not in command line mode * and removes it in command line mode * - DBO_PERSISTENT: use persistant database connection + * @param string $remember IDatabase::REMEMBER_* constant [default: REMEMBER_NOTHING] + */ + public function clearFlag( $flag, $remember = self::REMEMBER_NOTHING ); + + /** + * Restore the flags to their prior state before the last setFlag/clearFlag call + * + * @param string $state IDatabase::RESTORE_* constant. [default: RESTORE_PRIOR] + * @since 1.28 */ - public function clearFlag( $flag ); + public function restoreFlags( $state = self::RESTORE_PRIOR ); /** * Returns a boolean whether the flag $flag is set for this connection diff --git a/includes/db/loadbalancer/LBFactory.php b/includes/db/loadbalancer/LBFactory.php index b320544e01..f560cf17ab 100644 --- a/includes/db/loadbalancer/LBFactory.php +++ b/includes/db/loadbalancer/LBFactory.php @@ -216,6 +216,22 @@ abstract class LBFactory implements DestructibleService { ); } + /** + * Flush any master transaction snapshots and set DBO_TRX (if DBO_DEFAULT is set) + * + * The DBO_TRX setting will be reverted to the default in each of these methods: + * - commitMasterChanges() + * - rollbackMasterChanges() + * - commitAll() + * This allows for custom transaction rounds from any outer transaction scope. + * + * @param string $fname + * @since 1.28 + */ + public function beginMasterChanges( $fname = __METHOD__ ) { + $this->forEachLBCallMethod( 'beginMasterChanges', [ $fname ] ); + } + /** * Commit on all connections. Done for two reasons: * 1. To commit changes to the masters. diff --git a/includes/db/loadbalancer/LoadBalancer.php b/includes/db/loadbalancer/LoadBalancer.php index 13a08796cb..65cd3b3a8d 100644 --- a/includes/db/loadbalancer/LoadBalancer.php +++ b/includes/db/loadbalancer/LoadBalancer.php @@ -1062,7 +1062,7 @@ class LoadBalancer { */ public function commitAll( $fname = __METHOD__ ) { $this->forEachOpenConnection( function ( DatabaseBase $conn ) use ( $fname ) { - $conn->commit( $fname, IDatabase::FLUSHING_ALL_PEERS ); + $conn->commit( $fname, $conn::FLUSHING_ALL_PEERS ); } ); } @@ -1119,6 +1119,42 @@ class LoadBalancer { } ); } + /** + * Flush any master transaction snapshots and set DBO_TRX (if DBO_DEFAULT is set) + * + * The DBO_TRX setting will be reverted to the default in each of these methods: + * - commitMasterChanges() + * - rollbackMasterChanges() + * - commitAll() + * This allows for custom transaction rounds from any outer transaction scope. + * + * @param string $fname + * @since 1.28 + */ + public function beginMasterChanges( $fname = __METHOD__ ) { + $this->forEachOpenMasterConnection( function ( DatabaseBase $conn ) use ( $fname ) { + if ( $conn->writesOrCallbacksPending() ) { + throw new DBTransactionError( + $conn, + "Transaction with pending writes still active." + ); + } elseif ( $conn->trxLevel() ) { + $conn->commit( $fname, $conn::FLUSHING_ALL_PEERS ); + } + if ( $conn->getFlag( DBO_DEFAULT ) ) { + // DBO_TRX is controlled entirely by CLI mode presence with DBO_DEFAULT. + // Force DBO_TRX even in CLI mode since a commit round is expected soon. + $conn->setFlag( DBO_TRX, $conn::REMEMBER_PRIOR ); + $conn->onTransactionResolution( function () use ( $conn ) { + $conn->restoreFlags( $conn::RESTORE_PRIOR ); + } ); + } else { + // Config has explicitly requested DBO_TRX be either on or off; respect that. + // This is useful for things like blob stores which use auto-commit mode. + } + } ); + } + /** * Issue COMMIT on all master connections where writes where done * @param string $fname Caller name @@ -1126,7 +1162,7 @@ class LoadBalancer { public function commitMasterChanges( $fname = __METHOD__ ) { $this->forEachOpenMasterConnection( function ( DatabaseBase $conn ) use ( $fname ) { if ( $conn->writesOrCallbacksPending() ) { - $conn->commit( $fname, IDatabase::FLUSHING_ALL_PEERS ); + $conn->commit( $fname, $conn::FLUSHING_ALL_PEERS ); } } ); } @@ -1138,10 +1174,10 @@ class LoadBalancer { */ public function runMasterPostCommitCallbacks() { $e = null; // first exception - $this->forEachOpenMasterConnection( function ( DatabaseBase $db ) use ( &$e ) { - $db->setPostCommitCallbackSupression( false ); + $this->forEachOpenMasterConnection( function ( DatabaseBase $conn ) use ( &$e ) { + $conn->setPostCommitCallbackSupression( false ); try { - $db->runOnTransactionIdleCallbacks( IDatabase::TRIGGER_COMMIT ); + $conn->runOnTransactionIdleCallbacks( $conn::TRIGGER_COMMIT ); } catch ( Exception $ex ) { $e = $e ?: $ex; } @@ -1168,7 +1204,7 @@ class LoadBalancer { foreach ( $conns2[$masterIndex] as $conn ) { if ( $conn->trxLevel() && $conn->writesOrCallbacksPending() ) { try { - $conn->rollback( $fname, IDatabase::FLUSHING_ALL_PEERS ); + $conn->rollback( $fname, $conn::FLUSHING_ALL_PEERS ); } catch ( DBError $e ) { MWExceptionHandler::logException( $e ); $failedServers[] = $conn->getServer(); diff --git a/tests/phpunit/includes/db/DatabaseTest.php b/tests/phpunit/includes/db/DatabaseTest.php index 7e55a7327a..07514092de 100644 --- a/tests/phpunit/includes/db/DatabaseTest.php +++ b/tests/phpunit/includes/db/DatabaseTest.php @@ -23,6 +23,7 @@ class DatabaseTest extends MediaWikiTestCase { $this->dropFunctions(); $this->functionTest = false; } + $this->db->restoreFlags( IDatabase::RESTORE_INITIAL ); } /** * @covers DatabaseBase::dropTable @@ -323,4 +324,42 @@ class DatabaseTest extends MediaWikiTestCase { $db->begin( __METHOD__ ); throw new RunTimeException( "Uh oh!" ); } + + /** + * @covers DatabaseBase::getFlag( + * @covers DatabaseBase::setFlag() + * @covers DatabaseBase::restoreFlags() + */ + public function testFlagSetting() { + $db = $this->db; + $origTrx = $db->getFlag( DBO_TRX ); + $origSsl = $db->getFlag( DBO_SSL ); + + if ( $origTrx ) { + $db->clearFlag( DBO_TRX, $db::REMEMBER_PRIOR ); + } else { + $db->setFlag( DBO_TRX, $db::REMEMBER_PRIOR ); + } + $this->assertEquals( !$origTrx, $db->getFlag( DBO_TRX ) ); + + if ( $origSsl ) { + $db->clearFlag( DBO_SSL, $db::REMEMBER_PRIOR ); + } else { + $db->setFlag( DBO_SSL, $db::REMEMBER_PRIOR ); + } + $this->assertEquals( !$origSsl, $db->getFlag( DBO_SSL ) ); + + $db2 = clone $db; + $db2->restoreFlags( $db::RESTORE_INITIAL ); + $this->assertEquals( $origTrx, $db2->getFlag( DBO_TRX ) ); + $this->assertEquals( $origSsl, $db2->getFlag( DBO_SSL ) ); + + $db->restoreFlags(); + $this->assertEquals( $origSsl, $db->getFlag( DBO_SSL ) ); + $this->assertEquals( !$origTrx, $db->getFlag( DBO_TRX ) ); + + $db->restoreFlags(); + $this->assertEquals( $origSsl, $db->getFlag( DBO_SSL ) ); + $this->assertEquals( $origTrx, $db->getFlag( DBO_TRX ) ); + } } diff --git a/tests/phpunit/includes/installer/DatabaseUpdaterTest.php b/tests/phpunit/includes/installer/DatabaseUpdaterTest.php index 09d31fcb32..81c9fafac3 100644 --- a/tests/phpunit/includes/installer/DatabaseUpdaterTest.php +++ b/tests/phpunit/includes/installer/DatabaseUpdaterTest.php @@ -23,10 +23,10 @@ class FakeDatabase extends DatabaseBase { function __construct() { } - function clearFlag( $arg ) { + function clearFlag( $arg, $remember = self::REMEMBER_NOTHING ) { } - function setFlag( $arg ) { + function setFlag( $arg, $remember = self::REMEMBER_NOTHING ) { } public function insert( $table, $a, $fname = __METHOD__, $options = [] ) { -- 2.20.1