From: Aaron Schulz Date: Mon, 19 Aug 2019 22:24:15 +0000 (-0400) Subject: rdbms: add setTempTablesOnlyMode() to suppress CONN_TRX_AUTOCOMMIT during tests X-Git-Tag: 1.34.0-rc.0~591^2 X-Git-Url: http://git.cyclocoop.org/%7B%24www_url%7Dadmin/compta/exercices/modifier.php?a=commitdiff_plain;h=0b8a4daaa4c324797b748269f96e07480e5a1194;p=lhc%2Fweb%2Fwiklou.git rdbms: add setTempTablesOnlyMode() to suppress CONN_TRX_AUTOCOMMIT during tests Bug: T202116 Change-Id: Ib3e80eb04580b2d5a75ede8fb5546409c09a751a --- diff --git a/includes/Revision/RevisionRenderer.php b/includes/Revision/RevisionRenderer.php index ca4bb73bb0..3c3b6a99e6 100644 --- 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 5ef03042dc..6b724c7d42 100644 --- 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 990705c45f..75299eab3e 100644 --- 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 98607ce807..ccc09b8ce0 100644 --- 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; @@ -297,9 +299,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; @@ -320,6 +323,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; } } @@ -875,7 +884,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. @@ -2245,6 +2254,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 496f265b37..f564d2e3d7 100644 --- a/tests/phpunit/MediaWikiIntegrationTestCase.php +++ b/tests/phpunit/MediaWikiIntegrationTestCase.php @@ -1501,7 +1501,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; @@ -1511,11 +1510,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; @@ -1862,8 +1863,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() ); } /**