From: Aaron Schulz <aschulz@wikimedia.org>
Date: Wed, 22 Aug 2018 09:28:09 +0000 (-0700)
Subject: rdbms: make schema/prefix handling more consistent in LoadBalancer
X-Git-Tag: 1.34.0-rc.0~4327^2
X-Git-Url: http://git.cyclocoop.org/%22%20.%20generer_url_aide%28?a=commitdiff_plain;h=a3160ceeba58961c29d977717c58b41d2a7a0360;p=lhc%2Fweb%2Fwiklou.git

rdbms: make schema/prefix handling more consistent in LoadBalancer

Move "tablePrefix" logic to LoadBalancer::reallyOpenConnection and
make it more consistent for local vs foreign connections. The prefix
for local connections is now set even if $domain does not provide a
database.

Also set the schema even if $domain does not provide a database.

Typically a database domain is either fully specified or comes from
DatabaseDomain::newUnspecified(), but there's no reason not to use
at least the schema/prefix of otherwise unspecified domains. The only
other sensible option would be some sort of exception.

Change-Id: Ib52f4b69e429810e3c5eac92a48aecf7b22e86e6
---

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index fbc3be96be..00b413017b 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -938,10 +938,6 @@ class LoadBalancer implements ILoadBalancer {
 			$server = $this->servers[$i];
 			$server['serverIndex'] = $i;
 			$server['autoCommitOnly'] = $autoCommit;
-			if ( $this->localDomain->getDatabase() !== null ) {
-				// Use the local domain table prefix if the local domain is specified
-				$server['tablePrefix'] = $this->localDomain->getTablePrefix();
-			}
 			$conn = $this->reallyOpenConnection( $server, $this->localDomain );
 			$host = $this->getServerName( $i );
 			if ( $conn->isOpen() ) {
@@ -1037,7 +1033,6 @@ class LoadBalancer implements ILoadBalancer {
 				$this->errorConnection = $conn;
 				$conn = false;
 			} else {
-				$conn->tablePrefix( $prefix ); // as specified
 				// 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" );
@@ -1081,20 +1076,20 @@ class LoadBalancer implements ILoadBalancer {
 	 * Returns a Database object whether or not the connection was successful.
 	 *
 	 * @param array $server
-	 * @param DatabaseDomain $domainOverride Use an unspecified domain to not select any database
+	 * @param DatabaseDomain $domain Domain the connection is for, possibly unspecified
 	 * @return Database
 	 * @throws DBAccessError
 	 * @throws InvalidArgumentException
 	 */
-	protected function reallyOpenConnection( array $server, DatabaseDomain $domainOverride ) {
+	protected function reallyOpenConnection( array $server, DatabaseDomain $domain ) {
 		if ( $this->disabled ) {
 			throw new DBAccessError();
 		}
 
-		// Handle $domainOverride being a specified or an unspecified domain
-		if ( $domainOverride->getDatabase() === null ) {
-			// Normally, an RDBMS requires a DB name specified on connection and the $server
-			// configuration array is assumed to already specify an appropriate DB name.
+		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
@@ -1102,10 +1097,16 @@ class LoadBalancer implements ILoadBalancer {
 				$server['dbname'] = null;
 			}
 		} else {
-			$server['dbname'] = $domainOverride->getDatabase();
-			$server['schema'] = $domainOverride->getSchema();
+			$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;