From 6ded91313cb848d9b6c0c79036654f66b01980b1 Mon Sep 17 00:00:00 2001 From: Urbanecm Date: Sun, 25 Aug 2019 16:30:30 +0000 Subject: [PATCH] Revert "rdbms: make LoadBalancer::reallyOpenConnection() handle setting DBO_TRX" This reverts commit 45831e619c5e667ae1201bcdacfb8d4dcce10b41. Reason for revert: Caused beta not work at all. Bug: T231162 Change-Id: Icc5c1fa0dc01082a622641ad96c22c939cd56d48 --- includes/libs/rdbms/database/Database.php | 10 +- .../libs/rdbms/database/DatabasePostgres.php | 6 +- .../libs/rdbms/database/DatabaseSqlite.php | 25 +- includes/libs/rdbms/database/IDatabase.php | 13 +- includes/libs/rdbms/lbfactory/LBFactory.php | 5 +- .../libs/rdbms/lbfactory/LBFactoryMulti.php | 2 + .../libs/rdbms/lbfactory/LBFactorySimple.php | 8 + .../libs/rdbms/loadbalancer/LoadBalancer.php | 231 +++++++++--------- .../rdbms/loadbalancer/LoadBalancerSingle.php | 2 +- .../Revision/RevisionStoreDbTestBase.php | 8 +- .../includes/db/DatabasePostgresTest.php | 4 +- tests/phpunit/includes/db/LBFactoryTest.php | 23 +- .../rdbms/database/DatabaseMysqlBaseTest.php | 2 - 13 files changed, 173 insertions(+), 166 deletions(-) diff --git a/includes/libs/rdbms/database/Database.php b/includes/libs/rdbms/database/Database.php index f1b2ce277f..03180228a3 100644 --- a/includes/libs/rdbms/database/Database.php +++ b/includes/libs/rdbms/database/Database.php @@ -174,8 +174,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware /** @var float Query rount trip time estimate */ private $lastRoundTripEstimate = 0.0; - /** @var string Whether the database is a file on disk */ - const ATTR_DB_IS_FILE = 'db-is-file'; /** @var string Lock granularity is on the level of the entire database */ const ATTR_DB_LEVEL_LOCKING = 'db-level-locking'; /** @var string The SCHEMA keyword refers to a grouping of tables in a database */ @@ -247,6 +245,13 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware $this->cliMode = $params['cliMode']; $this->agent = $params['agent']; $this->flags = $params['flags']; + if ( $this->flags & self::DBO_DEFAULT ) { + if ( $this->cliMode ) { + $this->flags &= ~self::DBO_TRX; + } else { + $this->flags |= self::DBO_TRX; + } + } $this->nonNativeInsertSelectBatchSize = $params['nonNativeInsertSelectBatchSize'] ?? 10000; $this->srvCache = $params['srvCache'] ?? new HashBagOStuff(); @@ -420,7 +425,6 @@ abstract class Database implements IDatabase, IMaintainableDatabase, LoggerAware */ final public static function attributesFromType( $dbType, $driver = null ) { static $defaults = [ - self::ATTR_DB_IS_FILE => false, self::ATTR_DB_LEVEL_LOCKING => false, self::ATTR_SCHEMAS_AS_TABLE_GROUPS => false ]; diff --git a/includes/libs/rdbms/database/DatabasePostgres.php b/includes/libs/rdbms/database/DatabasePostgres.php index 197cdcc196..a7ebc86e50 100644 --- a/includes/libs/rdbms/database/DatabasePostgres.php +++ b/includes/libs/rdbms/database/DatabasePostgres.php @@ -95,8 +95,8 @@ class DatabasePostgres extends Database { $this->password = $password; $connectVars = [ - // A database must be specified in order to connect to Postgres. If $dbName is not - // specified, then use the standard "postgres" database that should exist by default. + // pg_connect() user $user as the default database. Since a database is required, + // then pick a "don't care" database that is more likely to exist than that one. 'dbname' => strlen( $dbName ) ? $dbName : 'postgres', 'user' => $user, 'password' => $password @@ -1442,7 +1442,7 @@ SQL; return $row ? ( strtolower( $row->default_transaction_read_only ) === 'on' ) : false; } - protected static function getAttributes() { + public static function getAttributes() { return [ self::ATTR_SCHEMAS_AS_TABLE_GROUPS => true ]; } diff --git a/includes/libs/rdbms/database/DatabaseSqlite.php b/includes/libs/rdbms/database/DatabaseSqlite.php index 8d417e659f..b1521dca2f 100644 --- a/includes/libs/rdbms/database/DatabaseSqlite.php +++ b/includes/libs/rdbms/database/DatabaseSqlite.php @@ -68,21 +68,21 @@ class DatabaseSqlite extends Database { * - dbDirectory : directory containing the DB and the lock file directory * - dbFilePath : use this to force the path of the DB file * - trxMode : one of (deferred, immediate, exclusive) - * @param array $params + * @param array $p */ - public function __construct( array $params ) { - if ( isset( $params['dbFilePath'] ) ) { - $this->dbPath = $params['dbFilePath']; - if ( !strlen( $params['dbname'] ) ) { - $params['dbname'] = self::generateDatabaseName( $this->dbPath ); + public function __construct( array $p ) { + if ( isset( $p['dbFilePath'] ) ) { + $this->dbPath = $p['dbFilePath']; + if ( !strlen( $p['dbname'] ) ) { + $p['dbname'] = self::generateDatabaseName( $this->dbPath ); } - } elseif ( isset( $params['dbDirectory'] ) ) { - $this->dbDir = $params['dbDirectory']; + } elseif ( isset( $p['dbDirectory'] ) ) { + $this->dbDir = $p['dbDirectory']; } - parent::__construct( $params ); + parent::__construct( $p ); - $this->trxMode = strtoupper( $params['trxMode'] ?? '' ); + $this->trxMode = strtoupper( $p['trxMode'] ?? '' ); $lockDirectory = $this->getLockFileDirectory(); if ( $lockDirectory !== null ) { @@ -96,10 +96,7 @@ class DatabaseSqlite extends Database { } protected static function getAttributes() { - return [ - self::ATTR_DB_IS_FILE => true, - self::ATTR_DB_LEVEL_LOCKING => true - ]; + return [ self::ATTR_DB_LEVEL_LOCKING => true ]; } /** diff --git a/includes/libs/rdbms/database/IDatabase.php b/includes/libs/rdbms/database/IDatabase.php index a64078abd4..876821319e 100644 --- a/includes/libs/rdbms/database/IDatabase.php +++ b/includes/libs/rdbms/database/IDatabase.php @@ -1082,9 +1082,8 @@ interface IDatabase { * * In systems like mysql/mariadb, different databases can easily be referenced on a single * connection merely by name, even in a single query via JOIN. On the other hand, Postgres - * treats databases as logically separate, with different database users, requiring special - * mechanisms like postgres_fdw to "mount" foreign DBs. This is true even among DBs on the - * same server. Changing the selected database via selectDomain() requires a new connection. + * treats databases as fully separate, only allowing mechanisms like postgres_fdw to + * effectively "mount" foreign DBs. This is true even among DBs on the same server. * * @return bool * @since 1.29 @@ -2033,11 +2032,11 @@ interface IDatabase { public function setSessionOptions( array $options ); /** - * Set schema variables to be used when streaming commands from SQL files or stdin + * Set variables to be used in sourceFile/sourceStream, in preference to the + * ones in $GLOBALS. If an array is set here, $GLOBALS will not be used at + * all. If it's set to false, $GLOBALS will be used. * - * Variables appear as SQL comments and are substituted by their corresponding values - * - * @param array|null $vars Map of (variable => value) or null to use the defaults + * @param bool|array $vars Mapping variable name to value. */ public function setSchemaVars( $vars ); diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php index c2e4c38cae..4426654ca2 100644 --- a/includes/libs/rdbms/lbfactory/LBFactory.php +++ b/includes/libs/rdbms/lbfactory/LBFactory.php @@ -155,9 +155,8 @@ abstract class LBFactory implements ILBFactory { $this->defaultGroup = $conf['defaultGroup'] ?? null; $this->secret = $conf['secret'] ?? ''; - static $nextId, $nextTicket; - $this->id = $nextId = ( is_int( $nextId ) ? $nextId++ : mt_rand() ); - $this->ticket = $nextTicket = ( is_int( $nextTicket ) ? $nextTicket++ : mt_rand() ); + $this->id = mt_rand(); + $this->ticket = mt_rand(); } public function destroy() { diff --git a/includes/libs/rdbms/lbfactory/LBFactoryMulti.php b/includes/libs/rdbms/lbfactory/LBFactoryMulti.php index 1a3495e4ac..f675b58778 100644 --- a/includes/libs/rdbms/lbfactory/LBFactoryMulti.php +++ b/includes/libs/rdbms/lbfactory/LBFactoryMulti.php @@ -312,11 +312,13 @@ class LBFactoryMulti extends LBFactory { foreach ( $loads as $serverName => $load ) { $serverInfo = $template; if ( $master ) { + $serverInfo['master'] = true; if ( $this->masterTemplateOverrides ) { $serverInfo = $this->masterTemplateOverrides + $serverInfo; } $master = false; } else { + $serverInfo['replica'] = true; } if ( isset( $this->templateOverridesByServer[$serverName] ) ) { $serverInfo = $this->templateOverridesByServer[$serverName] + $serverInfo; diff --git a/includes/libs/rdbms/lbfactory/LBFactorySimple.php b/includes/libs/rdbms/lbfactory/LBFactorySimple.php index eb4780288e..fd76d88dd0 100644 --- a/includes/libs/rdbms/lbfactory/LBFactorySimple.php +++ b/includes/libs/rdbms/lbfactory/LBFactorySimple.php @@ -58,6 +58,14 @@ class LBFactorySimple extends LBFactory { parent::__construct( $conf ); $this->servers = $conf['servers'] ?? []; + foreach ( $this->servers as $i => $server ) { + if ( $i == 0 ) { + $this->servers[$i]['master'] = true; + } else { + $this->servers[$i]['replica'] = true; + } + } + $this->externalClusters = $conf['externalClusters'] ?? []; $this->loadMonitorClass = $conf['loadMonitorClass'] ?? 'LoadMonitor'; } diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php index 955e3d83af..d088aa9e54 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php @@ -121,8 +121,6 @@ class LoadBalancer implements ILoadBalancer { /** @var bool Whether any connection has been attempted yet */ private $connectionAttempted = false; - /** var int An identifier for this class instance */ - private $id; /** @var int|null Integer ID of the managing LBFactory instance or null if none */ private $ownerId; /** @var string|bool Explicit DBO_TRX transaction round active or false if none */ @@ -173,6 +171,11 @@ class LoadBalancer implements ILoadBalancer { if ( ++$listKey !== $i ) { throw new UnexpectedValueException( 'List expected for "servers" parameter' ); } + if ( $i == 0 ) { + $server['master'] = true; + } else { + $server['replica'] = true; + } $this->servers[$i] = $server; foreach ( ( $server['groupLoads'] ?? [] ) as $group => $ratio ) { $this->groupLoads[$group][$i] = $ratio; @@ -235,8 +238,6 @@ class LoadBalancer implements ILoadBalancer { $group = $params['defaultGroup'] ?? self::GROUP_GENERIC; $this->defaultGroup = isset( $this->groupLoads[$group] ) ? $group : self::GROUP_GENERIC; - static $nextId; - $this->id = $nextId = ( is_int( $nextId ) ? $nextId++ : mt_rand() ); $this->ownerId = $params['ownerId'] ?? null; } @@ -956,7 +957,17 @@ class LoadBalancer implements ILoadBalancer { $serverIndex = $conn->getLBInfo( 'serverIndex' ); $refCount = $conn->getLBInfo( 'foreignPoolRefCount' ); if ( $serverIndex === null || $refCount === null ) { - return; // non-foreign connection; no domain-use tracking to update + /** + * This can happen in code like: + * foreach ( $dbs as $db ) { + * $conn = $lb->getConnection( $lb::DB_REPLICA, [], $db ); + * ... + * $lb->reuseConnection( $conn ); + * } + * When a connection to the local DB is opened in this way, reuseConnection() + * should be ignored + */ + return; } elseif ( $conn instanceof DBConnRef ) { // DBConnRef already handles calling reuseConnection() and only passes the live // Database instance to this method. Any caller passing in a DBConnRef is broken. @@ -1053,45 +1064,47 @@ class LoadBalancer implements ILoadBalancer { * * @note If disable() was called on this LoadBalancer, this method will throw a DBAccessError. * - * @param int $i Specific server index + * @param int $i Server index * @param int $flags Class CONN_* constant bitfield * @return Database * @throws InvalidArgumentException When the server index is invalid * @throws UnexpectedValueException When the DB domain of the connection is corrupted */ private function getLocalConnection( $i, $flags = 0 ) { - $autoCommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT ); // Connection handles required to be in auto-commit mode use a separate connection // pool since the main pool is effected by implicit and explicit transaction rounds - $connKey = $autoCommit ? self::KEY_LOCAL_NOROUND : self::KEY_LOCAL; + $autoCommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT ); + $connKey = $autoCommit ? self::KEY_LOCAL_NOROUND : self::KEY_LOCAL; if ( isset( $this->conns[$connKey][$i][0] ) ) { $conn = $this->conns[$connKey][$i][0]; } else { - $conn = $this->reallyOpenConnection( - $i, - $this->localDomain, - [ 'autoCommitOnly' => $autoCommit ] - ); + // Open a new connection + $server = $this->getServerInfoStrict( $i ); + $server['serverIndex'] = $i; + $server['autoCommitOnly'] = $autoCommit; + $conn = $this->reallyOpenConnection( $server, $this->localDomain ); + $host = $this->getServerName( $i ); if ( $conn->isOpen() ) { - $this->connLogger->debug( __METHOD__ . ": opened new connection for $i" ); + $this->connLogger->debug( + __METHOD__ . ": connected to database $i at '$host'." ); $this->conns[$connKey][$i][0] = $conn; } else { - $this->connLogger->warning( __METHOD__ . ": connection error for $i" ); + $this->connLogger->warning( + __METHOD__ . ": failed to connect to database $i at '$host'." ); $this->errorConnection = $conn; $conn = false; } } - // Sanity check to make sure that the right domain is selected + // Final sanity check to make sure the right domain is selected if ( $conn instanceof IDatabase && !$this->localDomain->isCompatible( $conn->getDomainID() ) ) { throw new UnexpectedValueException( "Got connection to '{$conn->getDomainID()}', " . - "but expected local domain ('{$this->localDomain}')" - ); + "but expected local domain ('{$this->localDomain}')" ); } return $conn; @@ -1113,7 +1126,7 @@ class LoadBalancer implements ILoadBalancer { * * @note If disable() was called on this LoadBalancer, this method will throw a DBAccessError. * - * @param int $i Specific server index + * @param int $i Server index * @param string $domain Domain ID to open * @param int $flags Class CONN_* constant bitfield * @return Database|bool Returns false on connection error @@ -1123,9 +1136,10 @@ class LoadBalancer implements ILoadBalancer { */ private function getForeignConnection( $i, $domain, $flags = 0 ) { $domainInstance = DatabaseDomain::newFromId( $domain ); - $autoCommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT ); // Connection handles required to be in auto-commit mode use a separate connection // pool since the main pool is effected by implicit and explicit transaction rounds + $autoCommit = ( ( $flags & self::CONN_TRX_AUTOCOMMIT ) == self::CONN_TRX_AUTOCOMMIT ); + if ( $autoCommit ) { $connFreeKey = self::KEY_FOREIGN_FREE_NOROUND; $connInUseKey = self::KEY_FOREIGN_INUSE_NOROUND; @@ -1178,28 +1192,26 @@ class LoadBalancer implements ILoadBalancer { } if ( !$conn ) { - $conn = $this->reallyOpenConnection( - $i, - $domainInstance, - [ - 'autoCommitOnly' => $autoCommit, - 'foreign' => true, - 'foreignPoolRefCount' => 0 - ] - ); - if ( $conn->isOpen() ) { - // Note that if $domain is an empty string, getDomainID() might not match it - $this->conns[$connInUseKey][$i][$conn->getDomainID()] = $conn; - $this->connLogger->debug( __METHOD__ . ": opened new connection for $i/$domain" ); - } else { + // Open a new connection + $server = $this->getServerInfoStrict( $i ); + $server['serverIndex'] = $i; + $server['foreignPoolRefCount'] = 0; + $server['foreign'] = true; + $server['autoCommitOnly'] = $autoCommit; + $conn = $this->reallyOpenConnection( $server, $domainInstance ); + if ( !$conn->isOpen() ) { $this->connLogger->warning( __METHOD__ . ": connection error for $i/$domain" ); $this->errorConnection = $conn; $conn = false; + } else { + // Note that if $domain is an empty string, getDomainID() might not match it + $this->conns[$connInUseKey][$i][$conn->getDomainID()] = $conn; + $this->connLogger->debug( __METHOD__ . ": opened new connection for $i/$domain" ); } } if ( $conn instanceof IDatabase ) { - // Sanity check to make sure that the right domain is selected + // Final sanity check to make sure the right domain is selected if ( !$domainInstance->isCompatible( $conn->getDomainID() ) ) { throw new UnexpectedValueException( "Got connection to '{$conn->getDomainID()}', but expected '$domain'" ); @@ -1234,101 +1246,94 @@ class LoadBalancer implements ILoadBalancer { * * Returns a Database object whether or not the connection was successful. * - * @param int $i Specific server index + * @param array $server * @param DatabaseDomain $domain Domain the connection is for, possibly unspecified - * @param array $lbInfo Additional information for setLBInfo() * @return Database * @throws DBAccessError * @throws InvalidArgumentException */ - protected function reallyOpenConnection( $i, DatabaseDomain $domain, array $lbInfo ) { + protected function reallyOpenConnection( array $server, DatabaseDomain $domain ) { if ( $this->disabled ) { throw new DBAccessError(); } - $server = $this->getServerInfoStrict( $i ); - $server = array_merge( $server, [ - // Use the database specified in $domain (null means "none or entrypoint DB"); - // fallback to the $server default if the RDBMs is an embedded library using a file - // on disk since there would be nothing to access to without a DB/file name. - 'dbname' => $this->getServerAttributes( $i )[Database::ATTR_DB_IS_FILE] - ? ( $domain->getDatabase() ?? $server['dbname'] ?? null ) - : $domain->getDatabase(), - // Override the $server default schema with that of $domain if specified - 'schema' => $domain->getSchema() ?? $server['schema'] ?? null, - // Use the table prefix specified in $domain - 'tablePrefix' => $domain->getTablePrefix(), - // Participate in transaction rounds if $server does not specify otherwise - 'flags' => $server['flags'] ?? IDatabase::DBO_DEFAULT, - // Inject the PHP execution mode and the agent string - 'cliMode' => $this->cliMode, - 'agent' => $this->agent, - // Inject object and callback dependencies - 'srvCache' => $this->srvCache, - 'connLogger' => $this->connLogger, - 'queryLogger' => $this->queryLogger, - 'errorLogger' => $this->errorLogger, - 'deprecationLogger' => $this->deprecationLogger, - 'profiler' => $this->profiler, - 'trxProfiler' => $this->trxProfiler - ] ); - - $lbInfo = array_merge( $lbInfo, [ - 'ownerId' => $this->id, - 'serverIndex' => $i, - 'master' => ( $i === $this->getWriterIndex() ), - 'replica' => ( $i !== $this->getWriterIndex() ), - // Name of the master server of the relevant DB cluster (e.g. "db1052") - 'clusterMasterHost' => $this->getServerName( $this->getWriterIndex() ) - ] ); - - $conn = Database::factory( $server['type'], $server, Database::NEW_UNCONNECTED ); + if ( $domain->getDatabase() === null ) { + // The database domain does not specify a DB name and some database systems require a + // valid DB specified on connection. The $server configuration array contains a default + // DB name to use for connections in such cases. + if ( $server['type'] === 'mysql' ) { + // For MySQL, DATABASE and SCHEMA are synonyms, connections need not specify a DB, + // and the DB name in $server might not exist due to legacy reasons (the default + // domain used to ignore the local LB domain, even when mismatched). + $server['dbname'] = null; + } + } else { + $server['dbname'] = $domain->getDatabase(); + } + + if ( $domain->getSchema() !== null ) { + $server['schema'] = $domain->getSchema(); + } + + // It is always possible to connect with any prefix, even the empty string + $server['tablePrefix'] = $domain->getTablePrefix(); + + // Let the handle know what the cluster master is (e.g. "db1052") + $masterName = $this->getServerName( $this->getWriterIndex() ); + $server['clusterMasterHost'] = $masterName; + + $server['srvCache'] = $this->srvCache; + // Set loggers and profilers + $server['connLogger'] = $this->connLogger; + $server['queryLogger'] = $this->queryLogger; + $server['errorLogger'] = $this->errorLogger; + $server['deprecationLogger'] = $this->deprecationLogger; + $server['profiler'] = $this->profiler; + $server['trxProfiler'] = $this->trxProfiler; + // Use the same agent and PHP mode for all DB handles + $server['cliMode'] = $this->cliMode; + $server['agent'] = $this->agent; + // Use DBO_DEFAULT flags by default for LoadBalancer managed databases. Assume that the + // application calls LoadBalancer::commitMasterChanges() before the PHP script completes. + $server['flags'] = $server['flags'] ?? IDatabase::DBO_DEFAULT; + + // Create a live connection object try { - $conn->initConnection(); + $db = Database::factory( $server['type'], $server ); + // Log when many connection are made on requests ++$this->connectionCounter; + $currentConnCount = $this->getCurrentConnectionCount(); + if ( $currentConnCount >= self::CONN_HELD_WARN_THRESHOLD ) { + $this->perfLogger->warning( + __METHOD__ . ": {connections}+ connections made (master={masterdb})", + [ 'connections' => $currentConnCount, 'masterdb' => $masterName ] + ); + } } catch ( DBConnectionError $e ) { - // ignore; let the DB handle the logging + // FIXME: This is probably the ugliest thing I have ever done to + // PHP. I'm half-expecting it to segfault, just out of disgust. -- TS + $db = $e->db; } - $conn->setLBInfo( $lbInfo ); - if ( $conn->getFlag( $conn::DBO_DEFAULT ) ) { - if ( $this->cliMode ) { - $conn->clearFlag( $conn::DBO_TRX ); - } else { - $conn->setFlag( $conn::DBO_TRX ); - } - } - if ( $i === $this->getWriterIndex() ) { + $db->setLBInfo( $server ); + $db->setLazyMasterHandle( + $this->getLazyConnectionRef( self::DB_MASTER, [], $db->getDomainID() ) + ); + $db->setTableAliases( $this->tableAliases ); + $db->setIndexAliases( $this->indexAliases ); + + if ( $server['serverIndex'] === $this->getWriterIndex() ) { if ( $this->trxRoundId !== false ) { - $this->applyTransactionRoundFlags( $conn ); + $this->applyTransactionRoundFlags( $db ); } foreach ( $this->trxRecurringCallbacks as $name => $callback ) { - $conn->setTransactionListener( $name, $callback ); + $db->setTransactionListener( $name, $callback ); } } - $conn->setTableAliases( $this->tableAliases ); - $conn->setIndexAliases( $this->indexAliases ); - - $conn->setLazyMasterHandle( - $this->getLazyConnectionRef( self::DB_MASTER, [], $conn->getDomainID() ) - ); $this->lazyLoadReplicationPositions(); // session consistency - // Log when many connection are made on requests - $count = $this->getCurrentConnectionCount(); - if ( $count >= self::CONN_HELD_WARN_THRESHOLD ) { - $this->perfLogger->warning( - __METHOD__ . ": {connections}+ connections made (master={masterdb})", - [ - 'connections' => $count, - 'dbserver' => $conn->getServer(), - 'masterdb' => $conn->getLBInfo( 'clusterMasterHost' ) - ] - ); - } - - return $conn; + return $db; } /** @@ -2104,8 +2109,8 @@ class LoadBalancer implements ILoadBalancer { public function waitForMasterPos( IDatabase $conn, $pos = false, $timeout = null ) { $timeout = max( 1, $timeout ?: $this->waitTimeout ); - if ( $conn->getLBInfo( 'serverIndex' ) === $this->getWriterIndex() ) { - return true; // not a replica DB server + if ( $this->getServerCount() <= 1 || !$conn->getLBInfo( 'replica' ) ) { + return true; // server is not a replica DB } if ( !$pos ) { @@ -2221,9 +2226,9 @@ class LoadBalancer implements ILoadBalancer { ) ); // Update the prefix for all local connections... - $this->forEachOpenConnection( function ( IDatabase $conn ) use ( $prefix ) { - if ( !$conn->getLBInfo( 'foreign' ) ) { - $conn->tablePrefix( $prefix ); + $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix ) { + if ( !$db->getLBInfo( 'foreign' ) ) { + $db->tablePrefix( $prefix ); } } ); } diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php b/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php index 2ad24b95d0..4c68833e7e 100644 --- a/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php +++ b/includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php @@ -83,7 +83,7 @@ class LoadBalancerSingle extends LoadBalancer { ) ); } - protected function reallyOpenConnection( $i, DatabaseDomain $domain, array $lbInfo = [] ) { + protected function reallyOpenConnection( array $server, DatabaseDomain $domain ) { return $this->db; } diff --git a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php index ba2d28ea0d..d4393dd3f6 100644 --- a/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php +++ b/tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php @@ -23,7 +23,6 @@ use MediaWiki\Storage\SqlBlobStore; use MediaWiki\User\UserIdentityValue; use MediaWikiTestCase; use PHPUnit_Framework_MockObject_MockObject; -use Psr\Log\NullLogger; use Revision; use TestUserRegistry; use Title; @@ -147,7 +146,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { ->getMock(); $lb->method( 'reallyOpenConnection' )->willReturnCallback( - function () use ( $server ) { + function ( array $server, $dbNameOverride ) { return $this->getDatabaseMock( $server ); } ); @@ -208,11 +207,10 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase { 'cliMode' => true, 'agent' => '', 'load' => 100, - 'srvCache' => new HashBagOStuff(), 'profiler' => null, 'trxProfiler' => new TransactionProfiler(), - 'connLogger' => new NullLogger(), - 'queryLogger' => new NullLogger(), + 'connLogger' => new \Psr\Log\NullLogger(), + 'queryLogger' => new \Psr\Log\NullLogger(), 'errorLogger' => function () { }, 'deprecationLogger' => function () { diff --git a/tests/phpunit/includes/db/DatabasePostgresTest.php b/tests/phpunit/includes/db/DatabasePostgresTest.php index 31a634370a..e0b81117f9 100644 --- a/tests/phpunit/includes/db/DatabasePostgresTest.php +++ b/tests/phpunit/includes/db/DatabasePostgresTest.php @@ -184,8 +184,6 @@ class DatabasePostgresTest extends MediaWikiTestCase { * @covers \Wikimedia\Rdbms\DatabasePostgres::getAttributes */ public function testAttributes() { - $this->assertTrue( - Database::attributesFromType( 'postgres' )[Database::ATTR_SCHEMAS_AS_TABLE_GROUPS] - ); + $this->assertTrue( DatabasePostgres::getAttributes()[Database::ATTR_SCHEMAS_AS_TABLE_GROUPS] ); } } diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php index 1296bb64b5..1016f28a38 100644 --- a/tests/phpunit/includes/db/LBFactoryTest.php +++ b/tests/phpunit/includes/db/LBFactoryTest.php @@ -23,7 +23,6 @@ * @copyright © 2013 Wikimedia Foundation Inc. */ -use Wikimedia\AtEase\AtEase; use Wikimedia\Rdbms\IDatabase; use Wikimedia\Rdbms\IMaintainableDatabase; use Wikimedia\Rdbms\LBFactory; @@ -475,7 +474,7 @@ class LBFactoryTest extends MediaWikiTestCase { unset( $db ); /** @var IMaintainableDatabase $db */ - $db = $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY ); + $db = $lb->getConnection( DB_MASTER, [], '' ); $this->assertEquals( '', @@ -555,7 +554,7 @@ class LBFactoryTest extends MediaWikiTestCase { ); $lb = $factory->getMainLB(); /** @var IMaintainableDatabase $db */ - $db = $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY ); + $db = $lb->getConnection( DB_MASTER, [], '' ); $this->assertEquals( '', $db->getDomainID(), "Null domain used" ); @@ -623,16 +622,16 @@ class LBFactoryTest extends MediaWikiTestCase { ); $lb = $factory->getMainLB(); /** @var IDatabase $db */ - $db = $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY ); + $db = $lb->getConnection( DB_MASTER, [], '' ); - AtEase::suppressWarnings(); + \Wikimedia\suppressWarnings(); try { - $this->assertFalse( $db->selectDomain( 'garbagedb' ) ); + $this->assertFalse( $db->selectDB( 'garbage-db' ) ); $this->fail( "No error thrown." ); } catch ( \Wikimedia\Rdbms\DBQueryError $e ) { - $this->assertRegExp( '/[\'"]garbagedb[\'"]/', $e->getMessage() ); + $this->assertRegExp( '/[\'"]garbage-db[\'"]/', $e->getMessage() ); } - AtEase::restoreWarnings(); + \Wikimedia\restoreWarnings(); } /** @@ -651,12 +650,12 @@ class LBFactoryTest extends MediaWikiTestCase { ); $lb = $factory->getMainLB(); - if ( !$factory->getMainLB()->getServerAttributes( 0 )[Database::ATTR_DB_IS_FILE] ) { - $this->markTestSkipped( "Not applicable per ATTR_DB_IS_FILE" ); + if ( !$lb->getConnection( DB_MASTER )->databasesAreIndependent() ) { + $this->markTestSkipped( "Not applicable per databasesAreIndependent()" ); } /** @var IDatabase $db */ - $this->assertNotNull( $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY ) ); + $lb->getConnection( DB_MASTER, [], '' ); } /** @@ -680,7 +679,7 @@ class LBFactoryTest extends MediaWikiTestCase { } $db = $lb->getConnection( DB_MASTER ); - $db->selectDomain( 'garbage-db' ); + $db->selectDB( 'garbage-db' ); } /** diff --git a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php index ee2d174f75..4c92545128 100644 --- a/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php +++ b/tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php @@ -362,8 +362,6 @@ class DatabaseMysqlBaseTest extends PHPUnit\Framework\TestCase { $db->method( 'getMasterServerInfo' ) ->willReturn( [ 'serverId' => 172, 'asOf' => time() ] ); - $db->setLBInfo( 'replica', true ); - // Fake the current time. list( $nowSecFrac, $nowSec ) = explode( ' ', microtime() ); $now = (float)$nowSec + (float)$nowSecFrac; -- 2.20.1