From 99a2ea61f930ca895fc0e340c19a83339cdafd96 Mon Sep 17 00:00:00 2001 From: Aaron Schulz Date: Sun, 9 Jun 2019 08:22:45 +0100 Subject: [PATCH] rdbms: implement strictor ownership of LoadBalancer by LBFactory Change-Id: Idafd2cf05e016a5f88b90e4c4c74f82c212d61c9 --- includes/libs/rdbms/database/Database.php | 2 +- includes/libs/rdbms/lbfactory/LBFactory.php | 31 +++++++------ .../libs/rdbms/loadbalancer/ILoadBalancer.php | 31 +++++++++---- .../libs/rdbms/loadbalancer/LoadBalancer.php | 44 +++++++++++++++---- 4 files changed, 76 insertions(+), 32 deletions(-) diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index de9ea552a7..602ef52698 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -3976,7 +3976,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } } - final public function rollback( $fname = __METHOD__, $flush = '' ) { + final public function rollback( $fname = __METHOD__, $flush = self::FLUSHING_ONE ) { $trxActive = $this->trxLevel; if ( $flush !== self::FLUSHING_INTERNAL diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php index 8608a7d6f9..f48487ae1f 100644 --- a/includes/libs/rdbms/lbfactory/LBFactory.php +++ b/includes/libs/rdbms/lbfactory/LBFactory.php @@ -85,7 +85,9 @@ abstract class LBFactory implements ILBFactory { /** @var callable[] */ private $replicationWaitCallbacks = []; - /** @var mixed */ + /** var int An identifier for this class instance */ + private $id; + /** @var int|null Ticket used to delegate transaction ownership */ private $ticket; /** @var string|bool String if a requested DBO_TRX transaction round is active */ private $trxRoundId = false; @@ -153,6 +155,7 @@ abstract class LBFactory implements ILBFactory { $this->defaultGroup = $conf['defaultGroup'] ?? null; $this->secret = $conf['secret'] ?? ''; + $this->id = mt_rand(); $this->ticket = mt_rand(); } @@ -251,7 +254,7 @@ abstract class LBFactory implements ILBFactory { } $this->trxRoundId = $fname; // Set DBO_TRX flags on all appropriate DBs - $this->forEachLBCallMethod( 'beginMasterChanges', [ $fname ] ); + $this->forEachLBCallMethod( 'beginMasterChanges', [ $fname, $this->id ] ); $this->trxRoundStage = self::ROUND_CURSORY; } @@ -269,17 +272,17 @@ abstract class LBFactory implements ILBFactory { // Run pre-commit callbacks and suppress post-commit callbacks, aborting on failure do { $count = 0; // number of callbacks executed this iteration - $this->forEachLB( function ( ILoadBalancer $lb ) use ( &$count ) { - $count += $lb->finalizeMasterChanges(); + $this->forEachLB( function ( ILoadBalancer $lb ) use ( &$count, $fname ) { + $count += $lb->finalizeMasterChanges( $fname, $this->id ); } ); } while ( $count > 0 ); $this->trxRoundId = false; // Perform pre-commit checks, aborting on failure - $this->forEachLBCallMethod( 'approveMasterChanges', [ $options ] ); + $this->forEachLBCallMethod( 'approveMasterChanges', [ $options, $fname, $this->id ] ); // Log the DBs and methods involved in multi-DB transactions $this->logIfMultiDbTransaction(); // Actually perform the commit on all master DB connections and revert DBO_TRX - $this->forEachLBCallMethod( 'commitMasterChanges', [ $fname ] ); + $this->forEachLBCallMethod( 'commitMasterChanges', [ $fname, $this->id ] ); // Run all post-commit callbacks in a separate step $this->trxRoundStage = self::ROUND_COMMIT_CALLBACKS; $e = $this->executePostTransactionCallbacks(); @@ -294,7 +297,7 @@ abstract class LBFactory implements ILBFactory { $this->trxRoundStage = self::ROUND_ROLLING_BACK; $this->trxRoundId = false; // Actually perform the rollback on all master DB connections and revert DBO_TRX - $this->forEachLBCallMethod( 'rollbackMasterChanges', [ $fname ] ); + $this->forEachLBCallMethod( 'rollbackMasterChanges', [ $fname, $this->id ] ); // Run all post-commit callbacks in a separate step $this->trxRoundStage = self::ROUND_ROLLBACK_CALLBACKS; $this->executePostTransactionCallbacks(); @@ -305,17 +308,18 @@ abstract class LBFactory implements ILBFactory { * @return Exception|null */ private function executePostTransactionCallbacks() { + $fname = __METHOD__; // Run all post-commit callbacks until new ones stop getting added $e = null; // first callback exception do { - $this->forEachLB( function ( ILoadBalancer $lb ) use ( &$e ) { - $ex = $lb->runMasterTransactionIdleCallbacks(); + $this->forEachLB( function ( ILoadBalancer $lb ) use ( &$e, $fname ) { + $ex = $lb->runMasterTransactionIdleCallbacks( $fname, $this->id ); $e = $e ?: $ex; } ); } while ( $this->hasMasterChanges() ); // Run all listener callbacks once - $this->forEachLB( function ( ILoadBalancer $lb ) use ( &$e ) { - $ex = $lb->runMasterTransactionListenerCallbacks(); + $this->forEachLB( function ( ILoadBalancer $lb ) use ( &$e, $fname ) { + $ex = $lb->runMasterTransactionListenerCallbacks( $fname, $this->id ); $e = $e ?: $ex; } ); @@ -605,7 +609,8 @@ abstract class LBFactory implements ILBFactory { // being called later (but before the first connection attempt) (T192611) $this->getChronologyProtector()->applySessionReplicationPosition( $lb ); }, - 'roundStage' => $initStage + 'roundStage' => $initStage, + 'ownerId' => $this->id ]; } @@ -614,7 +619,7 @@ abstract class LBFactory implements ILBFactory { */ protected function initLoadBalancer( ILoadBalancer $lb ) { if ( $this->trxRoundId !== false ) { - $lb->beginMasterChanges( $this->trxRoundId ); // set DBO_TRX + $lb->beginMasterChanges( $this->trxRoundId, $this->id ); // set DBO_TRX } $lb->setTableAliases( $this->tableAliases ); diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php index faa9654372..025887879e 100644 --- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php @@ -116,6 +116,7 @@ interface ILoadBalancer { * - errorLogger : Callback that takes an Exception and logs it. [optional] * - deprecationLogger: Callback to log a deprecation warning. [optional] * - roundStage: STAGE_POSTCOMMIT_* class constant; for internal use [optional] + * - ownerId: integer ID of an LBFactory instance that manages this instance [optional] * @throws InvalidArgumentException */ public function __construct( array $params ); @@ -414,18 +415,21 @@ interface ILoadBalancer { /** * Commit transactions on all open connections * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @throws DBExpectedError */ - public function commitAll( $fname = __METHOD__ ); + public function commitAll( $fname = __METHOD__, $owner = null ); /** * Run pre-commit callbacks and defer execution of post-commit callbacks * * Use this only for mutli-database commits * + * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @return int Number of pre-commit callbacks run (since 1.32) */ - public function finalizeMasterChanges(); + public function finalizeMasterChanges( $fname = __METHOD__, $owner = null ); /** * Perform all pre-commit checks for things like replication safety @@ -434,9 +438,11 @@ interface ILoadBalancer { * * @param array $options Includes: * - maxWriteDuration : max write query duration time in seconds + * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @throws DBTransactionError */ - public function approveMasterChanges( array $options ); + public function approveMasterChanges( array $options, $fname, $owner = null ); /** * Flush any master transaction snapshots and set DBO_TRX (if DBO_DEFAULT is set) @@ -447,38 +453,45 @@ interface ILoadBalancer { * - commitAll() * This allows for custom transaction rounds from any outer transaction scope. * - * @param string $fname + * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @throws DBExpectedError */ - public function beginMasterChanges( $fname = __METHOD__ ); + public function beginMasterChanges( $fname = __METHOD__, $owner = null ); /** * Issue COMMIT on all open master connections to flush changes and view snapshots * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @throws DBExpectedError */ - public function commitMasterChanges( $fname = __METHOD__ ); + public function commitMasterChanges( $fname = __METHOD__, $owner = null ); /** * Consume and run all pending post-COMMIT/ROLLBACK callbacks and commit dangling transactions * + * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @return Exception|null The first exception or null if there were none */ - public function runMasterTransactionIdleCallbacks(); + public function runMasterTransactionIdleCallbacks( $fname = __METHOD__, $owner = null ); /** * Run all recurring post-COMMIT/ROLLBACK listener callbacks * + * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @return Exception|null The first exception or null if there were none */ - public function runMasterTransactionListenerCallbacks(); + public function runMasterTransactionListenerCallbacks( $fname = __METHOD__, $owner = null ); /** * Issue ROLLBACK only on master, only if queries were done on connection * @param string $fname Caller name + * @param int|null $owner ID of the calling instance (e.g. the LBFactory ID) * @throws DBExpectedError */ - public function rollbackMasterChanges( $fname = __METHOD__ ); + public function rollbackMasterChanges( $fname = __METHOD__, $owner = null ); /** * Commit all replica DB transactions so as to flush any REPEATABLE-READ or SSI snapshots diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index 51fda52148..3fd0189a72 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -122,6 +122,8 @@ class LoadBalancer implements ILoadBalancer { /** @var bool Whether any connection has been attempted yet */ private $connectionAttempted = false; + /** @var int|null An integer ID of the managing LBFactory instance or null */ + private $ownerId; /** @var string|bool String if a requested DBO_TRX transaction round is active */ private $trxRoundId = false; /** @var string Stage of the current transaction round in the transaction round life-cycle */ @@ -249,6 +251,7 @@ class LoadBalancer implements ILoadBalancer { } $this->defaultGroup = $params['defaultGroup'] ?? null; + $this->ownerId = $params['ownerId'] ?? null; } public function getLocalDomainID() { @@ -1328,13 +1331,14 @@ class LoadBalancer implements ILoadBalancer { $conn->close(); } - public function commitAll( $fname = __METHOD__ ) { - $this->commitMasterChanges( $fname ); + public function commitAll( $fname = __METHOD__, $owner = null ) { + $this->commitMasterChanges( $fname, $owner ); $this->flushMasterSnapshots( $fname ); $this->flushReplicaSnapshots( $fname ); } - public function finalizeMasterChanges() { + public function finalizeMasterChanges( $fname = __METHOD__, $owner = null ) { + $this->assertOwnership( $fname, $owner ); $this->assertTransactionRoundStage( [ self::ROUND_CURSORY, self::ROUND_FINALIZED ] ); $this->trxRoundStage = self::ROUND_ERROR; // "failed" until proven otherwise @@ -1358,7 +1362,8 @@ class LoadBalancer implements ILoadBalancer { return $total; } - public function approveMasterChanges( array $options ) { + public function approveMasterChanges( array $options, $fname = __METHOD__, $owner = null ) { + $this->assertOwnership( $fname, $owner ); $this->assertTransactionRoundStage( self::ROUND_FINALIZED ); $limit = $options['maxWriteDuration'] ?? 0; @@ -1391,7 +1396,8 @@ class LoadBalancer implements ILoadBalancer { $this->trxRoundStage = self::ROUND_APPROVED; } - public function beginMasterChanges( $fname = __METHOD__ ) { + public function beginMasterChanges( $fname = __METHOD__, $owner = null ) { + $this->assertOwnership( $fname, $owner ); if ( $this->trxRoundId !== false ) { throw new DBTransactionError( null, @@ -1415,7 +1421,8 @@ class LoadBalancer implements ILoadBalancer { $this->trxRoundStage = self::ROUND_CURSORY; } - public function commitMasterChanges( $fname = __METHOD__ ) { + public function commitMasterChanges( $fname = __METHOD__, $owner = null ) { + $this->assertOwnership( $fname, $owner ); $this->assertTransactionRoundStage( self::ROUND_APPROVED ); $failures = []; @@ -1453,7 +1460,8 @@ class LoadBalancer implements ILoadBalancer { $this->trxRoundStage = self::ROUND_COMMIT_CALLBACKS; } - public function runMasterTransactionIdleCallbacks() { + public function runMasterTransactionIdleCallbacks( $fname = __METHOD__, $owner = null ) { + $this->assertOwnership( $fname, $owner ); if ( $this->trxRoundStage === self::ROUND_COMMIT_CALLBACKS ) { $type = IDatabase::TRIGGER_COMMIT; } elseif ( $this->trxRoundStage === self::ROUND_ROLLBACK_CALLBACKS ) { @@ -1522,7 +1530,8 @@ class LoadBalancer implements ILoadBalancer { return $e; } - public function runMasterTransactionListenerCallbacks() { + public function runMasterTransactionListenerCallbacks( $fname = __METHOD__, $owner = null ) { + $this->assertOwnership( $fname, $owner ); if ( $this->trxRoundStage === self::ROUND_COMMIT_CALLBACKS ) { $type = IDatabase::TRIGGER_COMMIT; } elseif ( $this->trxRoundStage === self::ROUND_ROLLBACK_CALLBACKS ) { @@ -1549,7 +1558,9 @@ class LoadBalancer implements ILoadBalancer { return $e; } - public function rollbackMasterChanges( $fname = __METHOD__ ) { + public function rollbackMasterChanges( $fname = __METHOD__, $owner = null ) { + $this->assertOwnership( $fname, $owner ); + $restore = ( $this->trxRoundId !== false ); $this->trxRoundId = false; $this->trxRoundStage = self::ROUND_ERROR; // "failed" until proven otherwise @@ -1567,6 +1578,7 @@ class LoadBalancer implements ILoadBalancer { /** * @param string|string[] $stage + * @throws DBTransactionError */ private function assertTransactionRoundStage( $stage ) { $stages = (array)$stage; @@ -1585,6 +1597,20 @@ class LoadBalancer implements ILoadBalancer { } } + /** + * @param string $fname + * @param int|null $owner Owner ID of the caller + * @throws DBTransactionError + */ + private function assertOwnership( $fname, $owner ) { + if ( $this->ownerId !== null && $owner !== $this->ownerId ) { + throw new DBTransactionError( + null, + "$fname: LoadBalancer is owned by LBFactory #{$this->ownerId} (got '$owner')." + ); + } + } + /** * Make all DB servers with DBO_DEFAULT/DBO_TRX set join the transaction round * -- 2.20.1