From 149c25370481c572aa5ede1fe198abe96bec48a8 Mon Sep 17 00:00:00 2001 From: daniel Date: Fri, 5 Oct 2012 10:19:42 +0200 Subject: [PATCH] Disable transaction warnings for automatic trx. This tracks which transaction was opened automatically because of the DBO_TRX flag, and then supresses any warnings about committing these transactions implicitely, even if write operations were performed by that transaction. To get warnings about implicitely committed writes from automatic transactions, enable $wgDebugDBTransactions. This change is a follow-up to I1e746322 and the older I3f1dd5fd, and should be considered an alternative to I3eacc5a9. The new warning behavior is: * when beginning a transaction, warn if there is a transaction pending that has also been started explicitely by calling begin(). * when beginning a transaction and $wgDebugDBTransactions is on, log any implicit commit of an automatic transaction if write operations where performed within that transaction. The idea is to provide warnings about nested explicite transactions while staying quiet about implicite commits to automatic transactions. Change-Id: Idbe4a9034b13413e351f432408d358a704f6b77d --- includes/Defines.php | 2 +- includes/db/Database.php | 56 ++++++++++++++++++++++++++++------------ 2 files changed, 40 insertions(+), 18 deletions(-) diff --git a/includes/Defines.php b/includes/Defines.php index be9f981602..ea91ea836e 100644 --- a/includes/Defines.php +++ b/includes/Defines.php @@ -39,7 +39,7 @@ define( 'MW_SPECIALPAGE_VERSION', 2 ); define( 'DBO_DEBUG', 1 ); define( 'DBO_NOBUFFER', 2 ); define( 'DBO_IGNORE', 4 ); -define( 'DBO_TRX', 8 ); +define( 'DBO_TRX', 8 ); // automatically start transaction on first query define( 'DBO_DEFAULT', 16 ); define( 'DBO_PERSISTENT', 32 ); define( 'DBO_SYSDBA', 64 ); //for oracle maintenance diff --git a/includes/db/Database.php b/includes/db/Database.php index 5271208fb2..141a324f62 100644 --- a/includes/db/Database.php +++ b/includes/db/Database.php @@ -266,6 +266,14 @@ abstract class DatabaseBase implements DatabaseType { */ private $mTrxDoneWrites = false; + /** + * Record if the current transaction was started implicitly due to DBO_TRX being set. + * + * @var Bool + * @see DatabaseBase::mTrxLevel + */ + private $mTrxAutomatic = false; + # ------------------------------------------------------------------------------ # Accessors # ------------------------------------------------------------------------------ @@ -745,8 +753,17 @@ abstract class DatabaseBase implements DatabaseType { $this->mOpened = false; if ( $this->mConn ) { if ( $this->trxLevel() ) { + if ( $this->mTrxAutomatic ) { + // clear flag, so commit() doesn't complain about an explicit commit of an implicit transaction. + $this->mTrxAutomatic = false; + } else { + wfWarn( "Transaction still in progress (from {$this->mTrxFname}), " . + " performing implicit commit before closing connection!" ); + } + $this->commit( __METHOD__ ); } + $ret = $this->closeConnection(); $this->mConn = false; return $ret; @@ -869,6 +886,7 @@ abstract class DatabaseBase implements DatabaseType { wfDebug("Implicit transaction start.\n"); } $this->begin( __METHOD__ . " ($fname)" ); + $this->mTrxAutomatic = true; } } @@ -903,6 +921,7 @@ abstract class DatabaseBase implements DatabaseType { if ( false === $ret && $this->wasErrorReissuable() ) { # Transaction is gone, like it or not $this->mTrxLevel = 0; + $this->trxIdleCallbacks = array(); // cancel wfDebug( "Connection lost, reconnecting...\n" ); if ( $this->ping() ) { @@ -2904,8 +2923,8 @@ abstract class DatabaseBase implements DatabaseType { * Note that when the DBO_TRX flag is set (which is usually the case for web requests, but not for maintenance scripts), * any previous database query will have started a transaction automatically. * - * Nesting of transactions is not supported. Attempts to nest transactions will cause warnings if DBO_TRX is not set - * or the extsting transaction contained write operations. + * Nesting of transactions is not supported. Attempts to nest transactions will cause a warning, unless the current + * transaction was started automatically because of the DBO_TRX flag. * * @param $fname string */ @@ -2913,21 +2932,20 @@ abstract class DatabaseBase implements DatabaseType { global $wgDebugDBTransactions; if ( $this->mTrxLevel ) { // implicit commit - if ( $this->mTrxDoneWrites || ( $this->mFlags & DBO_TRX ) === 0 ) { - // In theory, we should always warn about nesting BEGIN statements. - // However, it is sometimes hard to avoid so we only warn if: - // - // a) the transaction has done writes. This gives warnings about bad transactions - // that could cause partial writes but not about read queries seeing more - // than one DB snapshot (when in REPEATABLE-READ) due to nested BEGINs. - // - // b) the DBO_TRX flag is not set. Explicit transactions should always be properly - // started and comitted. - /*wfWarn( "$fname: Transaction already in progress (from {$this->mTrxFname}), " . - " performing implicit commit!" );*/ - } elseif ( $wgDebugDBTransactions ) { - wfDebug( "$fname: Transaction already in progress (from {$this->mTrxFname}), " . - " performing implicit commit!\n" ); + if ( !$this->mTrxAutomatic ) { + // We want to warn about inadvertently nested begin/commit pairs, but not about auto-committing + // implicit transactions that were started by query() because DBO_TRX was set. + + wfWarn( "$fname: Transaction already in progress (from {$this->mTrxFname}), " . + " performing implicit commit!" ); + } else { + // if the transaction was automatic and has done write operations, + // log it if $wgDebugDBTransactions is enabled. + + if ( $this->mTrxDoneWrites && $wgDebugDBTransactions ) { + wfDebug( "$fname: Automatic transaction with writes in progress (from {$this->mTrxFname}), " . + " performing implicit commit!\n" ); + } } $this->doCommit( $fname ); @@ -2937,6 +2955,7 @@ abstract class DatabaseBase implements DatabaseType { $this->doBegin( $fname ); $this->mTrxFname = $fname; $this->mTrxDoneWrites = false; + $this->mTrxAutomatic = false; } /** @@ -2961,7 +2980,10 @@ abstract class DatabaseBase implements DatabaseType { final public function commit( $fname = 'DatabaseBase::commit' ) { if ( !$this->mTrxLevel ) { wfWarn( "$fname: No transaction to commit, something got out of sync!" ); + } elseif( $this->mTrxAutomatic ) { + wfWarn( "$fname: Explicit commit of implicit transaction. Something may be out of sync!" ); } + $this->doCommit( $fname ); $this->runOnTransactionIdleCallbacks(); } -- 2.20.1