From 686aad4a8ca3dc5825c044bded4ad3414c2f77ea Mon Sep 17 00:00:00 2001
From: Aaron Schulz <aschulz@wikimedia.org>
Date: Thu, 8 Mar 2018 16:58:48 -0800
Subject: [PATCH] rdbms: upgrade transaction misuse warnings to exceptions

The last warnings in logstash for WMF have been cleaned up

Change-Id: I7d5bb624bc583191c3a0c95aa4e99322d6d5008c
---
 RELEASE-NOTES-1.31                        |  5 +++++
 includes/libs/rdbms/database/Database.php | 18 +++++++-----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/RELEASE-NOTES-1.31 b/RELEASE-NOTES-1.31
index e5226eca40..899f8b3d95 100644
--- a/RELEASE-NOTES-1.31
+++ b/RELEASE-NOTES-1.31
@@ -297,6 +297,11 @@ changes to languages because of Phabricator reports.
   wikitext table captions, wikitext table headings, wikitext table cells. HTML
   headings, HTML list items, HTML table captions, HTML table headings, HTML table cells
   will not have this trimming behavior.
+* Calling Database::begin() explicitly during an implicit transaction or when DBO_TRX
+  is set results in an exception. Calling Database::commit() explicitly for an implicit
+  transaction also results in an exception. Previously these were logged as errors.
+  The startAtomic() and endAtomic() methods, or AtomicSectionUpdate should be used
+  instead.
 
 == Compatibility ==
 MediaWiki 1.31 requires PHP 5.5.9 or later. Although HHVM 3.18.5 or later is supported,
diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php
index 2d90be610d..8cb726e6c6 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -3185,16 +3185,12 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 				$msg = "$fname: Explicit transaction already active (from {$this->trxFname}).";
 				throw new DBUnexpectedError( $this, $msg );
 			} else {
-				// @TODO: make this an exception at some point
 				$msg = "$fname: Implicit transaction already active (from {$this->trxFname}).";
-				$this->queryLogger->error( $msg );
-				return; // join the main transaction set
+				throw new DBUnexpectedError( $this, $msg );
 			}
 		} elseif ( $this->getFlag( self::DBO_TRX ) && $mode !== self::TRANSACTION_INTERNAL ) {
-			// @TODO: make this an exception at some point
 			$msg = "$fname: Implicit transaction expected (DBO_TRX set).";
-			$this->queryLogger->error( $msg );
-			return; // let any writes be in the main transaction
+			throw new DBUnexpectedError( $this, $msg );
 		}
 
 		// Avoid fatals if close() was called
@@ -3260,10 +3256,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 					"$fname: No transaction to commit, something got out of sync." );
 				return; // nothing to do
 			} elseif ( $this->trxAutomatic ) {
-				// @TODO: make this an exception at some point
-				$msg = "$fname: Explicit commit of implicit transaction.";
-				$this->queryLogger->error( $msg );
-				return; // wait for the main transaction set commit round
+				throw new DBUnexpectedError(
+					$this,
+					"$fname: Expected mass commit of all peer transactions (DBO_TRX set)."
+				);
 			}
 		}
 
@@ -3314,7 +3310,7 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 			} elseif ( $this->getFlag( self::DBO_TRX ) ) {
 				throw new DBUnexpectedError(
 					$this,
-					"$fname: Expected mass rollback of all peer databases (DBO_TRX set)."
+					"$fname: Expected mass rollback of all peer transactions (DBO_TRX set)."
 				);
 			}
 		}
-- 
2.20.1