jenkins-bot has submitted this change and it was merged.
Change subject: Cleanups to DB transaction handling
......................................................................
Cleanups to DB transaction handling
* Only do requeries after automatic reconnection if there is no trx open
instead.
of doing possible half-broken changes and not fully updating trx state
variables
* Also made trxLevel() no longer take an argument.
* Avoid clearing trx fields in some redundant places to make the code simpler
Only mTrxLevel needs to be well tracked in various places. For the other
fields,
code just checks mTrxLevel before using those fields.
They do need to be cleared in begin() though.
* Moved endAtomic() up next to startAtomic().
* Improved mTrxLevel docs
Change-Id: I12adfb4fcb28a4832facd63baee2283ead500bd2
---
M includes/db/Database.php
1 file changed, 47 insertions(+), 32 deletions(-)
Approvals:
Chad: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/db/Database.php b/includes/db/Database.php
index cd907e9..bcf5724 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -243,7 +243,6 @@
protected $mTablePrefix;
protected $mFlags;
protected $mForeign;
- protected $mTrxLevel = 0;
protected $mErrorCount = 0;
protected $mLBInfo = array();
protected $mFakeSlaveLag = null, $mFakeMaster = false;
@@ -255,6 +254,14 @@
protected $htmlErrors;
protected $delimiter = ';';
+
+ /**
+ * Either 1 if a transaction is active or 0 otherwise.
+ * The other Trx fields may not be meaningfull if this is 0.
+ *
+ * @var int
+ */
+ protected $mTrxLevel = 0;
/**
* Remembers the function name given for starting the most recent
transaction via begin().
@@ -385,16 +392,15 @@
}
/**
- * Gets or sets the current transaction level.
+ * Gets the current transaction level.
*
* Historically, transactions were allowed to be "nested". This is no
* longer supported, so this function really only returns a boolean.
*
- * @param int $level An integer (0 or 1), or omitted to leave it
unchanged.
* @return int The previous value
*/
- public function trxLevel( $level = null ) {
- return wfSetVar( $this->mTrxLevel, $level );
+ public function trxLevel() {
+ return $this->mTrxLevel;
}
/**
@@ -1023,9 +1029,9 @@
# Try reconnecting if the connection was lost
if ( false === $ret && $this->wasErrorReissuable() ) {
# Transaction is gone, like it or not
+ $hadTrx = $this->mTrxLevel; // possible lost transaction
+ wfDebug( "Connection lost, reconnecting...\n" );
$this->mTrxLevel = 0;
- $this->mTrxIdleCallbacks = array(); // cancel
- $this->mTrxPreCommitCallbacks = array(); // cancel
wfDebug( "Connection lost, reconnecting...\n" );
if ( $this->ping() ) {
@@ -1038,7 +1044,10 @@
# Not a database error to lose a
transaction after a minute or two
wfLogDBError( "Connection lost and
reconnected after {$elapsed}s, query: $sqlx\n" );
}
- $ret = $this->doQuery( $commentedSql );
+ if ( !$hadTrx ) {
+ # Should be safe to silently retry
+ $ret = $this->doQuery( $commentedSql );
+ }
} else {
wfDebug( "Failed\n" );
}
@@ -3223,6 +3232,7 @@
*
* @since 1.23
* @param string $fname
+ * @throws DBError
*/
final public function startAtomic( $fname = __METHOD__ ) {
if ( !$this->mTrxLevel ) {
@@ -3232,6 +3242,32 @@
}
$this->mTrxAtomicLevels->push( $fname );
+ }
+
+ /**
+ * Ends an atomic section of SQL statements
+ *
+ * Ends the next section of atomic SQL statements and commits the
transaction
+ * if necessary.
+ *
+ * @since 1.23
+ * @see DatabaseBase::startAtomic
+ * @param string $fname
+ * @throws DBError
+ */
+ final public function endAtomic( $fname = __METHOD__ ) {
+ if ( !$this->mTrxLevel ) {
+ throw new DBUnexpectedError( $this, 'No atomic
transaction is open.' );
+ }
+ if ( $this->mTrxAtomicLevels->isEmpty() ||
+ $this->mTrxAtomicLevels->pop() !== $fname
+ ) {
+ throw new DBUnexpectedError( $this, 'Invalid atomic
section ended.' );
+ }
+
+ if ( $this->mTrxAtomicLevels->isEmpty() &&
$this->mTrxAutomaticAtomic ) {
+ $this->commit( $fname, 'flush' );
+ }
}
/**
@@ -3245,6 +3281,7 @@
* transaction was started automatically because of the DBO_TRX flag.
*
* @param $fname string
+ * @throws DBError
*/
final public function begin( $fname = __METHOD__ ) {
global $wgDebugDBTransactions;
@@ -3287,6 +3324,8 @@
$this->mTrxAutomatic = false;
$this->mTrxAutomaticAtomic = false;
$this->mTrxAtomicLevels = new SplStack;
+ $this->mTrxIdleCallbacks = array();
+ $this->mTrxPreCommitCallbacks = array();
}
/**
@@ -3298,28 +3337,6 @@
protected function doBegin( $fname ) {
$this->query( 'BEGIN', $fname );
$this->mTrxLevel = 1;
- }
-
- /**
- * Ends an atomic section of SQL statements
- *
- * Ends the next section of atomic SQL statements and commits the
transaction
- * if necessary.
- *
- * @since 1.23
- * @see DatabaseBase::startAtomic
- * @param string $fname
- */
- final public function endAtomic( $fname = __METHOD__ ) {
- if ( $this->mTrxAtomicLevels->isEmpty() ||
- $this->mTrxAtomicLevels->pop() !== $fname
- ) {
- throw new DBUnexpectedError( $this, 'Invalid atomic
section ended.' );
- }
-
- if ( $this->mTrxAtomicLevels->isEmpty() &&
$this->mTrxAutomaticAtomic ) {
- $this->commit( $fname, 'flush' );
- }
}
/**
@@ -3359,7 +3376,6 @@
if ( $this->mTrxDoneWrites ) {
Profiler::instance()->transactionWritingOut(
$this->mServer, $this->mDBname );
}
- $this->mTrxDoneWrites = false;
$this->runOnTransactionIdleCallbacks();
}
@@ -3395,7 +3411,6 @@
if ( $this->mTrxDoneWrites ) {
Profiler::instance()->transactionWritingOut(
$this->mServer, $this->mDBname );
}
- $this->mTrxDoneWrites = false;
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/93627
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I12adfb4fcb28a4832facd63baee2283ead500bd2
Gerrit-PatchSet: 3
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Chad <[email protected]>
Gerrit-Reviewer: jenkins-bot
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
