Merge "rdbms: add setTempTablesOnlyMode() to suppress CONN_TRX_AUTOCOMMIT during...

diff --git a/includes/Revision/RevisionRenderer.php b/includes/Revision/RevisionRenderer.php
index ca4bb73..3c3b6a9 100644 (file)
--- a/includes/Revision/RevisionRenderer.php
+++ b/includes/Revision/RevisionRenderer.php
@@ -164,13 +164,9 @@ class RevisionRenderer {
        }
 
        private function getSpeculativeRevId( $dbIndex ) {
-               // Use a fresh master connection in order to see the latest data, by avoiding
+               // Use a separate master connection in order to see the latest data, by avoiding
                // stale data from REPEATABLE-READ snapshots.
-               // HACK: But don't use a fresh connection in unit tests, since it would not have
-               // the fake tables. This should be handled by the LoadBalancer!
-               $flags = defined( 'MW_PHPUNIT_TEST' ) || $dbIndex === DB_REPLICA
-                       ? 0
-                       : ILoadBalancer::CONN_TRX_AUTOCOMMIT;
+               $flags = ILoadBalancer::CONN_TRX_AUTOCOMMIT;
 
                $db = $this->loadBalancer->getConnectionRef( $dbIndex, [], $this->dbDomain, $flags );
 
@@ -183,13 +179,9 @@ class RevisionRenderer {
        }
 
        private function getSpeculativePageId( $dbIndex ) {
-               // Use a fresh master connection in order to see the latest data, by avoiding
+               // Use a separate master connection in order to see the latest data, by avoiding
                // stale data from REPEATABLE-READ snapshots.
-               // HACK: But don't use a fresh connection in unit tests, since it would not have
-               // the fake tables. This should be handled by the LoadBalancer!
-               $flags = defined( 'MW_PHPUNIT_TEST' ) || $dbIndex === DB_REPLICA
-                       ? 0
-                       : ILoadBalancer::CONN_TRX_AUTOCOMMIT;
+               $flags = ILoadBalancer::CONN_TRX_AUTOCOMMIT;
 
                $db = $this->loadBalancer->getConnectionRef( $dbIndex, [], $this->dbDomain, $flags );
 

diff --git a/includes/Storage/NameTableStore.php b/includes/Storage/NameTableStore.php
index f8a1f5c..88f301a 100644 (file)
--- a/includes/Storage/NameTableStore.php
+++ b/includes/Storage/NameTableStore.php
@@ -160,10 +160,7 @@ class NameTableStore {
                        if ( $id === null ) {
                                // RACE: $name was already in the db, probably just inserted, so load from master.
                                // Use DBO_TRX to avoid missing inserts due to other threads or REPEATABLE-READs.
-                               // ...but not during unit tests, because we need the fake DB tables of the default
-                               // connection.
-                               $connFlags = defined( 'MW_PHPUNIT_TEST' ) ? 0 : ILoadBalancer::CONN_TRX_AUTOCOMMIT;
-                               $table = $this->reloadMap( $connFlags );
+                               $table = $this->reloadMap( ILoadBalancer::CONN_TRX_AUTOCOMMIT );
 
                                $searchResult = array_search( $name, $table, true );
                                if ( $searchResult === false ) {

diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index 1125572..b21da72 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
@@ -155,6 +155,18 @@ interface ILoadBalancer {
         */
        public function redefineLocalDomain( $domain );
 
+       /**
+        * Indicate whether the tables on this domain are only temporary tables for testing
+        *
+        * In "temporary tables mode", the ILoadBalancer::CONN_TRX_AUTOCOMMIT flag is ignored
+        *
+        * @param bool $value
+        * @param string $domain
+        * @return bool Whether "temporary tables mode" was active
+        * @since 1.34
+        */
+       public function setTempTablesOnlyMode( $value, $domain );
+
        /**
         * Get the server index of the reader connection for a given group
         *

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 955e3d8..d277f22 100644 (file)
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -101,6 +101,8 @@ class LoadBalancer implements ILoadBalancer {
        private $indexAliases = [];
        /** @var array[] Map of (name => callable) */
        private $trxRecurringCallbacks = [];
+       /** @var bool[] Map of (domain => whether to use "temp tables only" mode) */
+       private $tempTablesOnlyMode = [];
 
        /** @var Database Connection handle that caused a problem */
        private $errorConnection;
@@ -296,9 +298,10 @@ class LoadBalancer implements ILoadBalancer {
        /**
         * @param int $flags Bitfield of class CONN_* constants
         * @param int $i Specific server index or DB_MASTER/DB_REPLICA
+        * @param string $domain Database domain
         * @return int Sanitized bitfield
         */
-       private function sanitizeConnectionFlags( $flags, $i ) {
+       private function sanitizeConnectionFlags( $flags, $i, $domain ) {
                // Whether an outside caller is explicitly requesting the master database server
                if ( $i === self::DB_MASTER || $i === $this->getWriterIndex() ) {
                        $flags |= self::CONN_INTENT_WRITABLE;
@@ -319,6 +322,12 @@ class LoadBalancer implements ILoadBalancer {
                                $flags &= ~self::CONN_TRX_AUTOCOMMIT;
                                $type = $this->getServerType( $this->getWriterIndex() );
                                $this->connLogger->info( __METHOD__ . ": CONN_TRX_AUTOCOMMIT disallowed ($type)" );
+                       } elseif ( isset( $this->tempTablesOnlyMode[$domain] ) ) {
+                               // T202116: integration tests are active and queries should be all be using
+                               // temporary clone tables (via prefix). Such tables are not visible accross
+                               // different connections nor can there be REPEATABLE-READ snapshot staleness,
+                               // so use the same connection for everything.
+                               $flags &= ~self::CONN_TRX_AUTOCOMMIT;
                        }
                }
 
@@ -873,7 +882,7 @@ class LoadBalancer implements ILoadBalancer {
        public function getConnection( $i, $groups = [], $domain = false, $flags = 0 ) {
                $domain = $this->resolveDomainID( $domain );
                $groups = $this->resolveGroups( $groups, $i );
-               $flags = $this->sanitizeConnectionFlags( $flags, $i );
+               $flags = $this->sanitizeConnectionFlags( $flags, $i, $domain );
                // If given DB_MASTER/DB_REPLICA, resolve it to a specific server index. Resolving
                // DB_REPLICA might trigger getServerConnection() calls due to the getReaderIndex()
                // connectivity checks or LoadMonitor::scaleLoads() server state cache regeneration.
@@ -2234,6 +2243,17 @@ class LoadBalancer implements ILoadBalancer {
                $this->setLocalDomain( DatabaseDomain::newFromId( $domain ) );
        }
 
+       public function setTempTablesOnlyMode( $value, $domain ) {
+               $old = $this->tempTablesOnlyMode[$domain] ?? false;
+               if ( $value ) {
+                       $this->tempTablesOnlyMode[$domain] = true;
+               } else {
+                       unset( $this->tempTablesOnlyMode[$domain] );
+               }
+
+               return $old;
+       }
+
        /**
         * @param DatabaseDomain $domain
         */

diff --git a/tests/phpunit/MediaWikiIntegrationTestCase.php b/tests/phpunit/MediaWikiIntegrationTestCase.php
index af5d88b..5261b19 100644 (file)
--- a/tests/phpunit/MediaWikiIntegrationTestCase.php
+++ b/tests/phpunit/MediaWikiIntegrationTestCase.php
@@ -1503,7 +1503,6 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 
                if ( !isset( $db->_originalTablePrefix ) ) {
                        $oldPrefix = $db->tablePrefix();
-
                        if ( $oldPrefix === $prefix ) {
                                // table already has the correct prefix, but presumably no cloned tables
                                $oldPrefix = self::$oldTablePrefix;
@@ -1513,11 +1512,13 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
                        $tablesCloned = self::listTables( $db );
                        $dbClone = new CloneDatabase( $db, $tablesCloned, $prefix, $oldPrefix );
                        $dbClone->useTemporaryTables( self::$useTemporaryTables );
-
                        $dbClone->cloneTableStructure();
 
                        $db->tablePrefix( $prefix );
                        $db->_originalTablePrefix = $oldPrefix;
+
+                       $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
+                       $lb->setTempTablesOnlyMode( self::$useTemporaryTables, $lb->getLocalDomainID() );
                }
 
                return true;
@@ -1864,8 +1865,10 @@ abstract class MediaWikiIntegrationTestCase extends PHPUnit\Framework\TestCase {
 
                $dbClone = new CloneDatabase( $db, $tables, $db->tablePrefix(), $db->_originalTablePrefix );
                $dbClone->useTemporaryTables( self::$useTemporaryTables );
-
                $dbClone->cloneTableStructure();
+
+               $lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
+               $lb->setTempTablesOnlyMode( self::$useTemporaryTables, $lb->getLocalDomainID() );
        }
 
        /**