rdbms: make LoadBalancer::reallyOpenConnection() handle setting DBO_TRX
authorAaron Schulz <aschulz@wikimedia.org>
Sun, 14 Jul 2019 22:43:26 +0000 (15:43 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Sat, 24 Aug 2019 20:06:17 +0000 (20:06 +0000)
Make LoadBalancer::reallyOpenConnection() handle initializing DBO_TRX
instead of Database::__construct().

Also:
* Avoid having the "catch" block appear like it returns a
  half-constructed Database.
* Use the variable name $conn instead of $db to be consistent
  throughout the class. Only send Database::__construct() parameters
  that it recognizes instead of mixing in setLBInfo() data.

Change-Id: Iffc3d1d0713051a164adb51a4c4ee12e4ac887c3

13 files changed:
includes/libs/rdbms/database/Database.php
includes/libs/rdbms/database/DatabasePostgres.php
includes/libs/rdbms/database/DatabaseSqlite.php
includes/libs/rdbms/database/IDatabase.php
includes/libs/rdbms/lbfactory/LBFactory.php
includes/libs/rdbms/lbfactory/LBFactoryMulti.php
includes/libs/rdbms/lbfactory/LBFactorySimple.php
includes/libs/rdbms/loadbalancer/LoadBalancer.php
includes/libs/rdbms/loadbalancer/LoadBalancerSingle.php
tests/phpunit/includes/Revision/RevisionStoreDbTestBase.php
tests/phpunit/includes/db/DatabasePostgresTest.php
tests/phpunit/includes/db/LBFactoryTest.php
tests/phpunit/includes/libs/rdbms/database/DatabaseMysqlBaseTest.php

index 0318022..f1b2ce2 100644 (file)
@@ -174,6 +174,8 @@ 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 */
@@ -245,13 +247,6 @@ 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();
@@ -425,6 +420,7 @@ 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
                ];
index a7ebc86..197cdcc 100644 (file)
@@ -95,8 +95,8 @@ class DatabasePostgres extends Database {
                $this->password = $password;
 
                $connectVars = [
-                       // 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.
+                       // 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.
                        'dbname' => strlen( $dbName ) ? $dbName : 'postgres',
                        'user' => $user,
                        'password' => $password
@@ -1442,7 +1442,7 @@ SQL;
                return $row ? ( strtolower( $row->default_transaction_read_only ) === 'on' ) : false;
        }
 
-       public static function getAttributes() {
+       protected static function getAttributes() {
                return [ self::ATTR_SCHEMAS_AS_TABLE_GROUPS => true ];
        }
 
index b1521dc..8d417e6 100644 (file)
@@ -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 $p
+        * @param array $params
         */
-       public function __construct( array $p ) {
-               if ( isset( $p['dbFilePath'] ) ) {
-                       $this->dbPath = $p['dbFilePath'];
-                       if ( !strlen( $p['dbname'] ) ) {
-                               $p['dbname'] = self::generateDatabaseName( $this->dbPath );
+       public function __construct( array $params ) {
+               if ( isset( $params['dbFilePath'] ) ) {
+                       $this->dbPath = $params['dbFilePath'];
+                       if ( !strlen( $params['dbname'] ) ) {
+                               $params['dbname'] = self::generateDatabaseName( $this->dbPath );
                        }
-               } elseif ( isset( $p['dbDirectory'] ) ) {
-                       $this->dbDir = $p['dbDirectory'];
+               } elseif ( isset( $params['dbDirectory'] ) ) {
+                       $this->dbDir = $params['dbDirectory'];
                }
 
-               parent::__construct( $p );
+               parent::__construct( $params );
 
-               $this->trxMode = strtoupper( $p['trxMode'] ?? '' );
+               $this->trxMode = strtoupper( $params['trxMode'] ?? '' );
 
                $lockDirectory = $this->getLockFileDirectory();
                if ( $lockDirectory !== null ) {
@@ -96,7 +96,10 @@ class DatabaseSqlite extends Database {
        }
 
        protected static function getAttributes() {
-               return [ self::ATTR_DB_LEVEL_LOCKING => true ];
+               return [
+                       self::ATTR_DB_IS_FILE => true,
+                       self::ATTR_DB_LEVEL_LOCKING => true
+               ];
        }
 
        /**
index 8768213..a64078a 100644 (file)
@@ -1082,8 +1082,9 @@ 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 fully separate, only allowing mechanisms like postgres_fdw to
-        * effectively "mount" foreign DBs. This is true even among DBs on the same server.
+        * 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.
         *
         * @return bool
         * @since 1.29
@@ -2032,11 +2033,11 @@ interface IDatabase {
        public function setSessionOptions( array $options );
 
        /**
-        * 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.
+        * Set schema variables to be used when streaming commands from SQL files or stdin
         *
-        * @param bool|array $vars Mapping variable name to value.
+        * 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
         */
        public function setSchemaVars( $vars );
 
index 4426654..c2e4c38 100644 (file)
@@ -155,8 +155,9 @@ abstract class LBFactory implements ILBFactory {
                $this->defaultGroup = $conf['defaultGroup'] ?? null;
                $this->secret = $conf['secret'] ?? '';
 
-               $this->id = mt_rand();
-               $this->ticket = mt_rand();
+               static $nextId, $nextTicket;
+               $this->id = $nextId = ( is_int( $nextId ) ? $nextId++ : mt_rand() );
+               $this->ticket = $nextTicket = ( is_int( $nextTicket ) ? $nextTicket++ : mt_rand() );
        }
 
        public function destroy() {
index f675b58..1a3495e 100644 (file)
@@ -312,13 +312,11 @@ 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;
index fd76d88..eb47802 100644 (file)
@@ -58,14 +58,6 @@ 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';
        }
index d088aa9..955e3d8 100644 (file)
@@ -121,6 +121,8 @@ 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 */
@@ -171,11 +173,6 @@ 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;
@@ -238,6 +235,8 @@ 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;
        }
 
@@ -957,17 +956,7 @@ class LoadBalancer implements ILoadBalancer {
                $serverIndex = $conn->getLBInfo( 'serverIndex' );
                $refCount = $conn->getLBInfo( 'foreignPoolRefCount' );
                if ( $serverIndex === null || $refCount === null ) {
-                       /**
-                        * 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;
+                       return; // non-foreign connection; no domain-use tracking to update
                } 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.
@@ -1064,47 +1053,45 @@ class LoadBalancer implements ILoadBalancer {
         *
         * @note If disable() was called on this LoadBalancer, this method will throw a DBAccessError.
         *
-        * @param int $i Server index
+        * @param int $i Specific 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
-               $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 {
-                       // Open a new connection
-                       $server = $this->getServerInfoStrict( $i );
-                       $server['serverIndex'] = $i;
-                       $server['autoCommitOnly'] = $autoCommit;
-                       $conn = $this->reallyOpenConnection( $server, $this->localDomain );
-                       $host = $this->getServerName( $i );
+                       $conn = $this->reallyOpenConnection(
+                               $i,
+                               $this->localDomain,
+                               [ 'autoCommitOnly' => $autoCommit ]
+                       );
                        if ( $conn->isOpen() ) {
-                               $this->connLogger->debug(
-                                       __METHOD__ . ": connected to database $i at '$host'." );
+                               $this->connLogger->debug( __METHOD__ . ": opened new connection for $i" );
                                $this->conns[$connKey][$i][0] = $conn;
                        } else {
-                               $this->connLogger->warning(
-                                       __METHOD__ . ": failed to connect to database $i at '$host'." );
+                               $this->connLogger->warning( __METHOD__ . ": connection error for $i" );
                                $this->errorConnection = $conn;
                                $conn = false;
                        }
                }
 
-               // Final sanity check to make sure the right domain is selected
+               // Sanity check to make sure that 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;
@@ -1126,7 +1113,7 @@ class LoadBalancer implements ILoadBalancer {
         *
         * @note If disable() was called on this LoadBalancer, this method will throw a DBAccessError.
         *
-        * @param int $i Server index
+        * @param int $i Specific server index
         * @param string $domain Domain ID to open
         * @param int $flags Class CONN_* constant bitfield
         * @return Database|bool Returns false on connection error
@@ -1136,10 +1123,9 @@ 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;
@@ -1192,26 +1178,28 @@ class LoadBalancer implements ILoadBalancer {
                }
 
                if ( !$conn ) {
-                       // 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 {
+                       $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 {
+                               $this->connLogger->warning( __METHOD__ . ": connection error for $i/$domain" );
+                               $this->errorConnection = $conn;
+                               $conn = false;
                        }
                }
 
                if ( $conn instanceof IDatabase ) {
-                       // Final sanity check to make sure the right domain is selected
+                       // Sanity check to make sure that the right domain is selected
                        if ( !$domainInstance->isCompatible( $conn->getDomainID() ) ) {
                                throw new UnexpectedValueException(
                                        "Got connection to '{$conn->getDomainID()}', but expected '$domain'" );
@@ -1246,94 +1234,101 @@ class LoadBalancer implements ILoadBalancer {
         *
         * Returns a Database object whether or not the connection was successful.
         *
-        * @param array $server
+        * @param int $i Specific server index
         * @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( array $server, DatabaseDomain $domain ) {
+       protected function reallyOpenConnection( $i, DatabaseDomain $domain, array $lbInfo ) {
                if ( $this->disabled ) {
                        throw new DBAccessError();
                }
 
-               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
+               $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 );
                try {
-                       $db = Database::factory( $server['type'], $server );
-                       // Log when many connection are made on requests
+                       $conn->initConnection();
                        ++$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 ) {
-                       // 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;
+                       // ignore; let the DB handle the logging
                }
 
-               $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() ) {
+               $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() ) {
                        if ( $this->trxRoundId !== false ) {
-                               $this->applyTransactionRoundFlags( $db );
+                               $this->applyTransactionRoundFlags( $conn );
                        }
                        foreach ( $this->trxRecurringCallbacks as $name => $callback ) {
-                               $db->setTransactionListener( $name, $callback );
+                               $conn->setTransactionListener( $name, $callback );
                        }
                }
+               $conn->setTableAliases( $this->tableAliases );
+               $conn->setIndexAliases( $this->indexAliases );
+
+               $conn->setLazyMasterHandle(
+                       $this->getLazyConnectionRef( self::DB_MASTER, [], $conn->getDomainID() )
+               );
 
                $this->lazyLoadReplicationPositions(); // session consistency
 
-               return $db;
+               // 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;
        }
 
        /**
@@ -2109,8 +2104,8 @@ class LoadBalancer implements ILoadBalancer {
        public function waitForMasterPos( IDatabase $conn, $pos = false, $timeout = null ) {
                $timeout = max( 1, $timeout ?: $this->waitTimeout );
 
-               if ( $this->getServerCount() <= 1 || !$conn->getLBInfo( 'replica' ) ) {
-                       return true; // server is not a replica DB
+               if ( $conn->getLBInfo( 'serverIndex' ) === $this->getWriterIndex() ) {
+                       return true; // not a replica DB server
                }
 
                if ( !$pos ) {
@@ -2226,9 +2221,9 @@ class LoadBalancer implements ILoadBalancer {
                ) );
 
                // Update the prefix for all local connections...
-               $this->forEachOpenConnection( function ( IDatabase $db ) use ( $prefix ) {
-                       if ( !$db->getLBInfo( 'foreign' ) ) {
-                               $db->tablePrefix( $prefix );
+               $this->forEachOpenConnection( function ( IDatabase $conn ) use ( $prefix ) {
+                       if ( !$conn->getLBInfo( 'foreign' ) ) {
+                               $conn->tablePrefix( $prefix );
                        }
                } );
        }
index 4c68833..2ad24b9 100644 (file)
@@ -83,7 +83,7 @@ class LoadBalancerSingle extends LoadBalancer {
                ) );
        }
 
-       protected function reallyOpenConnection( array $server, DatabaseDomain $domain ) {
+       protected function reallyOpenConnection( $i, DatabaseDomain $domain, array $lbInfo = [] ) {
                return $this->db;
        }
 
index d4393dd..ba2d28e 100644 (file)
@@ -23,6 +23,7 @@ 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;
@@ -146,7 +147,7 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                        ->getMock();
 
                $lb->method( 'reallyOpenConnection' )->willReturnCallback(
-                       function ( array $server, $dbNameOverride ) {
+                       function () use ( $server ) {
                                return $this->getDatabaseMock( $server );
                        }
                );
@@ -207,10 +208,11 @@ abstract class RevisionStoreDbTestBase extends MediaWikiTestCase {
                                'cliMode' => true,
                                'agent' => '',
                                'load' => 100,
+                               'srvCache' => new HashBagOStuff(),
                                'profiler' => null,
                                'trxProfiler' => new TransactionProfiler(),
-                               'connLogger' => new \Psr\Log\NullLogger(),
-                               'queryLogger' => new \Psr\Log\NullLogger(),
+                               'connLogger' => new NullLogger(),
+                               'queryLogger' => new NullLogger(),
                                'errorLogger' => function () {
                                },
                                'deprecationLogger' => function () {
index e0b8111..31a6343 100644 (file)
@@ -184,6 +184,8 @@ class DatabasePostgresTest extends MediaWikiTestCase {
         * @covers \Wikimedia\Rdbms\DatabasePostgres::getAttributes
         */
        public function testAttributes() {
-               $this->assertTrue( DatabasePostgres::getAttributes()[Database::ATTR_SCHEMAS_AS_TABLE_GROUPS] );
+               $this->assertTrue(
+                       Database::attributesFromType( 'postgres' )[Database::ATTR_SCHEMAS_AS_TABLE_GROUPS]
+               );
        }
 }
index 1016f28..1296bb6 100644 (file)
@@ -23,6 +23,7 @@
  * @copyright © 2013 Wikimedia Foundation Inc.
  */
 
+use Wikimedia\AtEase\AtEase;
 use Wikimedia\Rdbms\IDatabase;
 use Wikimedia\Rdbms\IMaintainableDatabase;
 use Wikimedia\Rdbms\LBFactory;
@@ -474,7 +475,7 @@ class LBFactoryTest extends MediaWikiTestCase {
                unset( $db );
 
                /** @var IMaintainableDatabase $db */
-               $db = $lb->getConnection( DB_MASTER, [], '' );
+               $db = $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY );
 
                $this->assertEquals(
                        '',
@@ -554,7 +555,7 @@ class LBFactoryTest extends MediaWikiTestCase {
                );
                $lb = $factory->getMainLB();
                /** @var IMaintainableDatabase $db */
-               $db = $lb->getConnection( DB_MASTER, [], '' );
+               $db = $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY );
 
                $this->assertEquals( '', $db->getDomainID(), "Null domain used" );
 
@@ -622,16 +623,16 @@ class LBFactoryTest extends MediaWikiTestCase {
                );
                $lb = $factory->getMainLB();
                /** @var IDatabase $db */
-               $db = $lb->getConnection( DB_MASTER, [], '' );
+               $db = $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY );
 
-               \Wikimedia\suppressWarnings();
+               AtEase::suppressWarnings();
                try {
-                       $this->assertFalse( $db->selectDB( 'garbage-db' ) );
+                       $this->assertFalse( $db->selectDomain( 'garbagedb' ) );
                        $this->fail( "No error thrown." );
                } catch ( \Wikimedia\Rdbms\DBQueryError $e ) {
-                       $this->assertRegExp( '/[\'"]garbage-db[\'"]/', $e->getMessage() );
+                       $this->assertRegExp( '/[\'"]garbagedb[\'"]/', $e->getMessage() );
                }
-               \Wikimedia\restoreWarnings();
+               AtEase::restoreWarnings();
        }
 
        /**
@@ -650,12 +651,12 @@ class LBFactoryTest extends MediaWikiTestCase {
                );
                $lb = $factory->getMainLB();
 
-               if ( !$lb->getConnection( DB_MASTER )->databasesAreIndependent() ) {
-                       $this->markTestSkipped( "Not applicable per databasesAreIndependent()" );
+               if ( !$factory->getMainLB()->getServerAttributes( 0 )[Database::ATTR_DB_IS_FILE] ) {
+                       $this->markTestSkipped( "Not applicable per ATTR_DB_IS_FILE" );
                }
 
                /** @var IDatabase $db */
-               $lb->getConnection( DB_MASTER, [], '' );
+               $this->assertNotNull( $lb->getConnection( DB_MASTER, [], $lb::DOMAIN_ANY ) );
        }
 
        /**
@@ -679,7 +680,7 @@ class LBFactoryTest extends MediaWikiTestCase {
                }
 
                $db = $lb->getConnection( DB_MASTER );
-               $db->selectDB( 'garbage-db' );
+               $db->selectDomain( 'garbage-db' );
        }
 
        /**
index 4c92545..ee2d174 100644 (file)
@@ -362,6 +362,8 @@ 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;