From fddf37e0917cabbcde1143bb9ff5bf733da707ad Mon Sep 17 00:00:00 2001 From: Amir Sarabadani Date: Mon, 21 Feb 2022 03:02:13 +0100 Subject: [PATCH] rdbms: Migrate $this->trxFname to TransactionManager Breaking up monster class: Database Bug: T299698 Change-Id: I57912af106499b35180f0da3ee3cd9997ea6250e --- includes/libs/rdbms/database/Database.php | 30 +++++++---------- .../rdbms/database/TransactionManager.php | 33 +++++++++++++++---- 2 files changed, 38 insertions(+), 25 deletions(-) diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index 86d822fd23d..0094ca23a6d 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -131,8 +131,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware /** @var array|null Replication lag estimate at the time of BEGIN for the last transaction */ private $trxReplicaLagStatus = null; - /** @var string|null Name of the function that start the last transaction */ - private $trxFname = null; /** @var bool Whether possible write queries were done in the last transaction started */ private $trxDoneWrites = false; /** @var int Counter for atomic savepoint identifiers (reset with each transaction) */ @@ -911,7 +909,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware if ( $this->conn ) { // Roll back any dangling transaction first if ( $this->trxLevel() ) { - $error = $this->transactionManager->trxCheckBeforeClose( $this, $fname, $this->trxFname ); + $error = $this->transactionManager->trxCheckBeforeClose( $this, $fname ); if ( $this->trxEndCallbacksSuppressed && $error === null ) { $error = "$fname: callbacks are suppressed; cannot properly commit"; @@ -4496,15 +4494,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware */ private function nextSavepointId( $fname ) { $savepointId = self::SAVEPOINT_PREFIX . ++$this->trxAtomicCounter; - if ( strlen( $savepointId ) > 30 ) { - // 30 == Oracle's identifier length limit (pre 12c) - // With a 22 character prefix, that puts the highest number at 99999999. - throw new DBUnexpectedError( - $this, - 'There have been an excessively large number of atomic sections in a transaction' - . " started by $this->trxFname (at $fname)" - ); - } + $this->transactionManager->onNextSavePointId( $this, $fname, $savepointId ); return $savepointId; } @@ -4697,7 +4687,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware // Protect against mismatched atomic section, transaction nesting, and snapshot loss if ( $this->trxLevel() ) { - $this->transactionManager->onBeginTransaction( $this, $fname, $this->trxFname ); + $this->transactionManager->onBeginTransaction( $this, $fname ); } elseif ( $this->getFlag( self::DBO_TRX ) && $mode !== self::TRANSACTION_INTERNAL ) { $msg = "$fname: implicit transaction expected (DBO_TRX set)"; throw new DBUnexpectedError( $this, $msg ); @@ -4712,10 +4702,9 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $this->completeCriticalSection( __METHOD__, $cs ); throw $e; } - $this->transactionManager->newTrxId( $mode ); + $this->transactionManager->newTrxId( $mode, $fname ); $this->trxAtomicCounter = 0; $this->transactionManager->setTrxTimestamp( microtime( true ) ); - $this->trxFname = $fname; $this->trxDoneWrites = false; // With REPEATABLE-READ isolation, the first SELECT establishes the read snapshot, // so get the replication lag estimate before any transaction SELECT queries come in. @@ -4872,12 +4861,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware } public function flushSnapshot( $fname = __METHOD__, $flush = self::FLUSHING_ONE ) { + $trxFname = $this->transactionManager->getTrxFname(); if ( $this->transactionManager->explicitTrxActive() ) { // Committing this transaction would break callers that assume it is still open throw new DBUnexpectedError( $this, "$fname: Cannot flush snapshot; " . - "explicit transaction '{$this->trxFname}' is still open" + "explicit transaction '{$trxFname}' is still open" ); } elseif ( $this->writesOrCallbacksPending() ) { // This only flushes transactions to clear snapshots, not to write data @@ -4885,7 +4875,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware throw new DBUnexpectedError( $this, "$fname: Cannot flush snapshot; " . - "writes from transaction {$this->trxFname} are still pending ($fnames)" + "writes from transaction {$trxFname} are still pending ($fnames)" ); } elseif ( $this->trxLevel() && @@ -5486,12 +5476,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware public function getScopedLockAndFlush( $lockKey, $fname, $timeout ) { if ( $this->writesOrCallbacksPending() ) { + $trxFname = $this->transactionManager->getTrxFname(); // This only flushes transactions to clear snapshots, not to write data $fnames = implode( ', ', $this->pendingWriteAndCallbackCallers() ); throw new DBUnexpectedError( $this, "$fname: Cannot flush pre-lock snapshot; " . - "writes from transaction {$this->trxFname} are still pending ($fnames)" + "writes from transaction {$trxFname} are still pending ($fnames)" ); } @@ -5907,7 +5898,8 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware */ public function __destruct() { if ( $this->trxLevel() && $this->trxDoneWrites ) { - trigger_error( "Uncommitted DB writes (transaction from {$this->trxFname})" ); + $trxFname = $this->transactionManager->getTrxFname(); + trigger_error( "Uncommitted DB writes (transaction from {$trxFname})" ); } $danglingWriters = $this->pendingWriteAndCallbackCallers(); diff --git a/includes/libs/rdbms/database/TransactionManager.php b/includes/libs/rdbms/database/TransactionManager.php index 77f3c331817..5ab8b655938 100644 --- a/includes/libs/rdbms/database/TransactionManager.php +++ b/includes/libs/rdbms/database/TransactionManager.php @@ -77,6 +77,9 @@ class TransactionManager { /** @var bool Whether the current transaction was started implicitly by startAtomic() */ private $trxAutomaticAtomic = false; + /** @var string|null Name of the function that start the last transaction */ + private $trxFname = null; + /** @var LoggerInterface */ private $logger; @@ -99,8 +102,9 @@ class TransactionManager { /** * TODO: This should be removed once all usages have been migrated here * @param string $mode One of IDatabase::TRANSACTION_* values + * @param string $fname method name */ - public function newTrxId( $mode ) { + public function newTrxId( $mode, $fname ) { static $nextTrxId; $nextTrxId = ( $nextTrxId !== null ? $nextTrxId++ : mt_rand() ) % 0xffff; $this->trxId = sprintf( '%06x', mt_rand( 0, 0xffffff ) ) . sprintf( '%04x', $nextTrxId ); @@ -119,6 +123,7 @@ class TransactionManager { // tracking fields (e.g. trxAtomicLevels). $this->trxAutomatic = ( $mode === IDatabase::TRANSACTION_INTERNAL ); $this->trxAutomaticAtomic = false; + $this->trxFname = $fname; } /** @@ -293,7 +298,7 @@ class TransactionManager { return $this->trxLevel() && ( $this->trxAtomicLevels || !$this->trxAutomatic ); } - public function trxCheckBeforeClose( IDatabase $db, $fname, $trxFname ) { + public function trxCheckBeforeClose( IDatabase $db, $fname ) { $error = null; if ( $this->trxAtomicLevels ) { // Cannot let incomplete atomic sections be committed @@ -309,7 +314,7 @@ class TransactionManager { } else { // Manual transactions should have been committed or rolled // back, even if empty. - $error = "$fname: transaction is still open (from {$trxFname})"; + $error = "$fname: transaction is still open (from {$this->trxFname})"; } return $error; @@ -340,17 +345,17 @@ class TransactionManager { ( count( $this->trxAtomicLevels ) - 1 ) . " ($fname)", [ 'db_log_category' => 'trx' ] ); } - public function onBeginTransaction( IDatabase $db, $fname, $trxFname ): void { + public function onBeginTransaction( IDatabase $db, $fname ): void { // @phan-suppress-previous-line PhanPluginNeverReturnMethod if ( $this->trxAtomicLevels ) { $levels = $this->flatAtomicSectionList(); $msg = "$fname: got explicit BEGIN while atomic section(s) $levels are open"; throw new DBUnexpectedError( $db, $msg ); } elseif ( !$this->trxAutomatic ) { - $msg = "$fname: explicit transaction already active (from {$trxFname})"; + $msg = "$fname: explicit transaction already active (from {$this->trxFname})"; throw new DBUnexpectedError( $db, $msg ); } else { - $msg = "$fname: implicit transaction already active (from {$trxFname})"; + $msg = "$fname: implicit transaction already active (from {$this->trxFname})"; throw new DBUnexpectedError( $db, $msg ); } } @@ -479,4 +484,20 @@ class TransactionManager { public function turnOnAutomatic() { $this->trxAutomatic = true; } + + public function onNextSavePointId( IDatabase $db, $fname, $savepointId ) { + if ( strlen( $savepointId ) > 30 ) { + // 30 == Oracle's identifier length limit (pre 12c) + // With a 22 character prefix, that puts the highest number at 99999999. + throw new DBUnexpectedError( + $db, + 'There have been an excessively large number of atomic sections in a transaction' + . " started by $this->trxFname (at $fname)" + ); + } + } + + public function getTrxFname() { + return $this->trxFname; + } }