[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Add sanity check to LoadBalancer::setDomainPrefix()

2016-09-21 Thread jenkins-bot (Code Review)
jenkins-bot has submitted this change and it was merged.

Change subject: Add sanity check to LoadBalancer::setDomainPrefix()
..


Add sanity check to LoadBalancer::setDomainPrefix()

Fixed some errors that popped up in CI:
* Also cleanup $domain handling in reuseConnection().
* Fix empty string handling in openForeignConnection() where
  the empty string check against $dbName failed since an empty
  string $domain results in $dbName being null.

Change-Id: Ie78fefa1acb401fe4e8bdc96b75053692aa0a925
---
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
M tests/phpunit/includes/db/LBFactoryTest.php
2 files changed, 25 insertions(+), 15 deletions(-)

Approvals:
  Legoktm: Verified; Looks good to me, approved
  jenkins-bot: Verified



diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php 
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 9c07043..c030cb2 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -136,9 +136,10 @@
 
$this->mReadIndex = -1;
$this->mConns = [
-   'local' => [],
+   'local'   => [],
'foreignUsed' => [],
-   'foreignFree' => [] ];
+   'foreignFree' => []
+   ];
$this->mLoads = [];
$this->mWaitForPos = false;
$this->mErrorConnection = false;
@@ -598,21 +599,18 @@
return;
}
 
-   $dbName = $conn->getDBname();
-   $prefix = $conn->tablePrefix();
-   if ( strval( $prefix ) !== '' ) {
-   $domain = "$dbName-$prefix";
-   } else {
-   $domain = $dbName;
-   }
+   $domain = $conn->getDomainID();
if ( $this->mConns['foreignUsed'][$serverIndex][$domain] !== 
$conn ) {
-   throw new InvalidArgumentException( __METHOD__ . ": 
connection not found, has " .
-   "the connection been freed already?" );
+   throw new InvalidArgumentException( __METHOD__ .
+   ": connection not found, has the connection 
been freed already?" );
}
$conn->setLBInfo( 'foreignPoolRefCount', --$refCount );
if ( $refCount <= 0 ) {
$this->mConns['foreignFree'][$serverIndex][$domain] = 
$conn;
unset( 
$this->mConns['foreignUsed'][$serverIndex][$domain] );
+   if ( !$this->mConns['foreignUsed'][$serverIndex] ) {
+   unset( $this->mConns[ 'foreignUsed' 
][$serverIndex] ); // clean up
+   }
$this->connLogger->debug( __METHOD__ . ": freed 
connection $serverIndex/$domain" );
} else {
$this->connLogger->debug( __METHOD__ .
@@ -707,11 +705,10 @@
// Reuse a connection from another domain
$conn = reset( $this->mConns['foreignFree'][$i] );
$oldDomain = key( $this->mConns['foreignFree'][$i] );
-
// The empty string as a DB name means "don't care".
// DatabaseMysqlBase::open() already handle this on 
connection.
-   if ( $dbName !== '' && !$conn->selectDB( $dbName ) ) {
-   $this->mLastError = "Error selecting database 
$dbName on server " .
+   if ( strlen( $dbName ) && !$conn->selectDB( $dbName ) ) 
{
+   $this->mLastError = "Error selecting database 
'$dbName' on server " .
$conn->getServer() . " from client host 
{$this->host}";
$this->mErrorConnection = $conn;
$conn = false;
@@ -770,7 +767,7 @@
 * @access private
 *
 * @param array $server
-* @param bool $dbNameOverride
+* @param string|bool $dbNameOverride Use "" to not select any database
 * @return IDatabase
 * @throws DBAccessError
 * @throws InvalidArgumentException
@@ -1473,6 +1470,17 @@
}
 
public function setDomainPrefix( $prefix ) {
+   if ( $this->mConns['foreignUsed'] ) {
+   // Do not switch connections to explicit foreign 
domains unless marked as free
+   $domains = [];
+   foreach ( $this->mConns['foreignUsed'] as $i => 
$connsByDomain ) {
+   $domains = array_merge( $domains, array_keys( 
$connsByDomain ) );
+   }
+   $domains = implode( ', ', $domains );
+   throw new 

[MediaWiki-commits] [Gerrit] mediawiki/core[master]: Add sanity check to LoadBalancer::setDomainPrefix()

2016-09-21 Thread Aaron Schulz (Code Review)
Aaron Schulz has uploaded a new change for review.

  https://gerrit.wikimedia.org/r/311937

Change subject: Add sanity check to LoadBalancer::setDomainPrefix()
..

Add sanity check to LoadBalancer::setDomainPrefix()

Change-Id: Ie78fefa1acb401fe4e8bdc96b75053692aa0a925
---
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
1 file changed, 5 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core 
refs/changes/37/311937/1

diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php 
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 9c07043..25eb3b3 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -1473,6 +1473,11 @@
}
 
public function setDomainPrefix( $prefix ) {
+   if ( $this->mConns['foreignUsed'] ) {
+   // Do not switch connections to explicit foreign 
domains unless marked as free
+   throw new DBUnexpectedError( null, "Foriegn domain 
connections are still in use." );
+   }
+
$this->localDomain = new DatabaseDomain(
$this->localDomain->getDatabase(),
null,

-- 
To view, visit https://gerrit.wikimedia.org/r/311937
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie78fefa1acb401fe4e8bdc96b75053692aa0a925
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz 

___
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits