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 DBUnexpectedError( null,
+ "Foreign domain connections are still in use
($domains)." );
+ }
+
$this->localDomain = new DatabaseDomain(
$this->localDomain->getDatabase(),
null,
diff --git a/tests/phpunit/includes/db/LBFactoryTest.php
b/tests/phpunit/includes/db/LBFactoryTest.php
index adf8a40..d72768d 100644
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -272,6 +272,7 @@
/** @var DatabaseBase $db */
$db = $lb->getConnection( DB_MASTER, [], '' );
+ $lb->reuseConnection( $db ); // don't care
$this->assertEquals(
'',
@@ -323,6 +324,7 @@
$lb = $factory->getMainLB();
/** @var DatabaseBase $db */
$db = $lb->getConnection( DB_MASTER, [], '' );
+ $lb->reuseConnection( $db ); // don't care
$this->assertEquals(
'',
--
To view, visit https://gerrit.wikimedia.org/r/311937
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie78fefa1acb401fe4e8bdc96b75053692aa0a925
Gerrit-PatchSet: 4
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Legoktm <legoktm.wikipe...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
