Merge "rdbms: track active transaction IDs for named locks and temp tables"

Author: jenkins-bot

autoload.php

@@ -1870,6 +1870,7 @@
 	'Wikimedia\\Rdbms\\Subquery' => __DIR__ . '/includes/libs/rdbms/encasing/Subquery.php',
 	'Wikimedia\\Rdbms\\TimestampType' => __DIR__ . '/includes/libs/rdbms/dbal/TimestampType.php',
 	'Wikimedia\\Rdbms\\TinyIntType' => __DIR__ . '/includes/libs/rdbms/dbal/TinyIntType.php',
+	'Wikimedia\\Rdbms\\TransactionIdentifier' => __DIR__ . '/includes/libs/rdbms/database/utils/TransactionIdentifier.php',
 	'Wikimedia\\Rdbms\\TransactionManager' => __DIR__ . '/includes/libs/rdbms/database/TransactionManager.php',
 	'Wikimedia\\Rdbms\\TransactionProfiler' => __DIR__ . '/includes/libs/rdbms/TransactionProfiler.php',
 	'Wikimedia\\Reflection\\GhostFieldAccessTrait' => __DIR__ . '/includes/libs/GhostFieldAccessTrait.php',

includes/libs/rdbms/database/Database.php

@@ -119,12 +119,10 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware
 	/** @var int[] Prior flags member variable values */
 	private $priorFlags = [];
 
-	/** @var array<string,float> Map of (name => UNIX timestamp) for locks obtained via lock() */
+	/** @var array<string,array> Map of (name => (UNIX time,trx ID)) for current lock() mutexes */
 	protected $sessionNamedLocks = [];
-	/** @var array Map of (table name => 1) for current TEMPORARY tables */
+	/** @var array<string,array> Map of (name => (type,pristine,trx ID)) for current temp tables */
 	protected $sessionTempTables = [];
-	/** @var array Map of (table name => 1) for current TEMPORARY tables */
-	protected $sessionDirtyTempTables = [];
 
 	/** @var array|null Replication lag estimate at the time of BEGIN for the last transaction */
 	private $trxReplicaLagStatus = null;
@@ -1086,8 +1084,10 @@ protected function getTempTableWrites( $sql, $pseudoPermanent ) {
 			if ( $queryVerb === 'CREATE' ) {
 				// Record the type of temporary table being created
 				$tableType = $pseudoPermanent ? self::TEMP_PSEUDO_PERMANENT : self::TEMP_NORMAL;
+			} elseif ( isset( $this->sessionTempTables[$table] ) ) {
+				$tableType = $this->sessionTempTables[$table]['type'];
 			} else {
-				$tableType = $this->sessionTempTables[$table] ?? null;
+				$tableType = null;
 			}
 
 			if ( $tableType !== null ) {
@@ -1110,17 +1110,24 @@ protected function registerTempWrites( $ret, array $changes ) {
 		foreach ( $changes as list( $tmpTableType, $verb, $table ) ) {
 			switch ( $verb ) {
 				case 'CREATE':
-					$this->sessionTempTables[$table] = $tmpTableType;
+					$this->sessionTempTables[$table] = [
+						'type' => $tmpTableType,
+						'pristine' => true,
+						'trxId' => $this->transactionManager->getTrxId()
+					];
 					break;
 				case 'DROP':
 					unset( $this->sessionTempTables[$table] );
-					unset( $this->sessionDirtyTempTables[$table] );
 					break;
 				case 'TRUNCATE':
-					unset( $this->sessionDirtyTempTables[$table] );
+					if ( isset( $this->sessionTempTables[$table] ) ) {
+						$this->sessionTempTables[$table]['pristine'] = true;
+					}
 					break;
 				default:
-					$this->sessionDirtyTempTables[$table] = 1;
+					if ( isset( $this->sessionTempTables[$table] ) ) {
+						$this->sessionTempTables[$table]['pristine'] = false;
+					}
 					break;
 			}
 		}
@@ -1136,10 +1143,9 @@ protected function registerTempWrites( $ret, array $changes ) {
 	protected function isPristineTemporaryTable( $table ) {
 		$rawTable = $this->tableName( $table, 'raw' );
 
-		return (
-			isset( $this->sessionTempTables[$rawTable] ) &&
-			!isset( $this->sessionDirtyTempTables[$rawTable] )
-		);
+		return isset( $this->sessionTempTables[$rawTable] )
+			? $this->sessionTempTables[$rawTable]['pristine']
+			: false;
 	}
 
 	public function query( $sql, $fname = __METHOD__, $flags = self::QUERY_NORMAL ) {
@@ -1503,7 +1509,6 @@ private function handleSessionLossPreconnect() {
 		// https://dev.mysql.com/doc/refman/5.7/en/implicit-commit.html
 		// https://www.postgresql.org/docs/9.2/static/sql-createtable.html (ignoring ON COMMIT)
 		$this->sessionTempTables = [];
-		$this->sessionDirtyTempTables = [];
 		// https://dev.mysql.com/doc/refman/5.7/en/miscellaneous-functions.html#function_get-lock
 		// https://www.postgresql.org/docs/9.4/static/functions-admin.html#FUNCTIONS-ADVISORY-LOCKS
 		$this->sessionNamedLocks = [];
@@ -1512,7 +1517,7 @@ private function handleSessionLossPreconnect() {
 		// Clear additional subclass fields
 		$oldTrxId = $this->transactionManager->consumeTrxId();
 		$this->doHandleSessionLossPreconnect();
-		$this->transactionManager->transactionWritingOut( $this, $oldTrxId );
+		$this->transactionManager->transactionWritingOut( $this, (string)$oldTrxId );
 	}
 
 	/**
@@ -5111,22 +5116,22 @@ public function lock( $lockName, $method, $timeout = 5, $flags = 0 ) {
 		$lockTsUnix = $this->doLock( $lockName, $method, $timeout );
 		if ( $lockTsUnix !== null ) {
 			$locked = true;
-			$this->sessionNamedLocks[$lockName] = $lockTsUnix;
+			$this->sessionNamedLocks[$lockName] = [
+				'ts' => $lockTsUnix,
+				'trxId' => $this->transactionManager->getTrxId()
+			];
 		} else {
 			$locked = false;
-			$this->queryLogger->info( __METHOD__ . " failed to acquire lock '{lockname}'",
+			$this->queryLogger->info(
+				__METHOD__ . " failed to acquire lock '{lockname}'",
 				[
 					'lockname' => $lockName,
 					'db_log_category' => 'locking'
-				] );
+				]
+			);
 		}
 
-		if ( $this->fieldHasBit( $flags, self::LOCK_TIMESTAMP ) ) {
-			// @phan-suppress-next-line PhanTypeMismatchReturnNullable Possible null is documented on the constant
-			return $lockTsUnix;
-		} else {
-			return $locked;
-		}
+		return $this->fieldHasBit( $flags, self::LOCK_TIMESTAMP ) ? $lockTsUnix : $locked;
 	}
 
 	/**

includes/libs/rdbms/database/IDatabase.php

@@ -2134,7 +2134,7 @@ public function lockIsFree( $lockName, $method );
 	 * @param string $method Name of the calling method
 	 * @param int $timeout Acquisition timeout in seconds (0 means non-blocking)
 	 * @param int $flags Bit field of IDatabase::LOCK_* constants
-	 * @return bool|float Success
+	 * @return bool|float|null Success (bool); acquisition time (float/null) if LOCK_TIMESTAMP
 	 * @throws DBError If an error occurs, {@see query}
 	 */
 	public function lock( $lockName, $method, $timeout = 5, $flags = 0 );

includes/libs/rdbms/database/TransactionManager.php

@@ -50,8 +50,8 @@ class TransactionManager {
 	/** @var string Prefix to the atomic section counter used to make savepoint IDs */
 	private const SAVEPOINT_PREFIX = 'wikimedia_rdbms_atomic';
 
-	/** @var string Application-side ID of the active transaction or an empty string otherwise */
-	private $trxId = '';
+	/** @var TransactionIdentifier|null Application-side ID of the active transaction; null if none */
+	private $trxId;
 	/** @var float|null UNIX timestamp at the time of BEGIN for the last transaction */
 	private $trxTimestamp = null;
 	/** @var int Transaction status */
@@ -115,7 +115,7 @@ public function __construct( LoggerInterface $logger = null, $profiler = null )
 	}
 
 	public function trxLevel() {
-		return ( $this->trxId != '' ) ? 1 : 0;
+		return $this->trxId ? 1 : 0;
 	}
 
 	/**
@@ -124,9 +124,7 @@ public function trxLevel() {
 	 * @param string $fname method name
 	 */
 	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 );
+		$this->trxId = new TransactionIdentifier();
 		$this->trxStatus = self::STATUS_TRX_OK;
 		$this->trxStatusIgnoredCause = null;
 		$this->trxWriteDuration = 0.0;
@@ -149,13 +147,23 @@ public function newTrxId( $mode, $fname ) {
 	}
 
 	/**
-	 * Reset the application-side transaction ID and return the old one
+	 * Get the application-side transaction identifier instance
+	 *
+	 * @return TransactionIdentifier Token for the active transaction; null if there isn't one
+	 */
+	public function getTrxId() {
+		return $this->trxId;
+	}
+
+	/**
+	 * Reset the application-side transaction identifier instance and return the old one
+	 *
 	 * This will become private soon.
-	 * @return string The old transaction ID or an empty string if there wasn't one
+	 * @return TransactionIdentifier|null The old transaction token; null if there wasn't one
 	 */
 	public function consumeTrxId() {
 		$old = $this->trxId;
-		$this->trxId = '';
+		$this->trxId = null;
 		$this->trxAtomicCounter = 0;
 
 		return $old;
@@ -547,7 +555,7 @@ public function transactionWritingIn( $serverName, $domainId ) {
 			$this->profiler->transactionWritingIn(
 				$serverName,
 				$domainId,
-				$this->trxId
+				(string)$this->trxId
 			);
 		}
 	}
@@ -570,7 +578,7 @@ public function recordQueryCompletion( $sql, $startTime, $isPermWrite, $rowCount
 			$startTime,
 			$isPermWrite,
 			$rowCount,
-			$this->trxId,
+			(string)$this->trxId,
 			$serverName
 		);
 	}
@@ -839,7 +847,7 @@ public function onRollback( IDatabase $db ) {
 		$this->setTrxStatusToNone();
 		$this->resetTrxAtomicLevels();
 		$this->clearPreEndCallbacks();
-		$this->transactionWritingOut( $db, $oldTrxId );
+		$this->transactionWritingOut( $db, (string)$oldTrxId );
 	}
 
 	public function onCommitInCriticalSection( IDatabase $db ) {
@@ -848,7 +856,7 @@ public function onCommitInCriticalSection( IDatabase $db ) {
 		$this->setTrxStatusToNone();
 		if ( $this->trxDoneWrites ) {
 			$lastWriteTime = microtime( true );
-			$this->transactionWritingOut( $db, $oldTrxId );
+			$this->transactionWritingOut( $db, (string)$oldTrxId );
 		}
 		return $lastWriteTime;
 	}

includes/libs/rdbms/database/utils/AtomicSectionIdentifier.php

@@ -22,6 +22,9 @@
 
 /**
  * Class used for token representing identifiers for atomic sections from IDatabase instances
+ *
+ * @ingroup Database
+ * @internal
  */
 class AtomicSectionIdentifier {
 }

includes/libs/rdbms/database/utils/TransactionIdentifier.php

@@ -0,0 +1,42 @@
+<?php
+/**
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Database
+ */
+namespace Wikimedia\Rdbms;
+
+/**
+ * Class used for token representing identifiers for atomic transactions from IDatabase instances
+ *
+ * @ingroup Database
+ * @internal
+ */
+class TransactionIdentifier {
+	/** @var string Application-side ID of the active transaction or an empty string otherwise */
+	private $id = '';
+
+	public function __construct() {
+		static $nextId;
+		$nextId = ( $nextId !== null ? $nextId++ : mt_rand() ) % 0xffff;
+		$this->id = sprintf( '%06x', mt_rand( 0, 0xffffff ) ) . sprintf( '%04x', $nextId );
+	}
+
+	public function __toString() {
+		return $this->id;
+	}
+}