jenkins-bot has submitted this change and it was merged.
Change subject: Make LoadBalancer domain handling more robust
......................................................................
Make LoadBalancer domain handling more robust
* Cleanup setDomainPrefix() methods to handle existing LBs/DBs.
* This also makes it set just the prefix as the prefix rather
than as the whole domain.
* Fix regression from 789a54a5f1 where "read" mode of
tablePrefix()/dbSchema() still set the field.
* Make getConnectionRef() always have the domain set.
* Add a check in openConnection() for explicit local
connections, treating $domain as false and avoiding
openForeignConnection().
Bug: T145840
Change-Id: Idf392bd9992a215c4fa3bddf275562f3916596aa
---
M includes/db/CloneDatabase.php
M includes/db/Database.php
M includes/libs/rdbms/lbfactory/LBFactory.php
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
M tests/phpunit/includes/db/DatabaseTest.php
5 files changed, 53 insertions(+), 21 deletions(-)
Approvals:
Gergő Tisza: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/db/CloneDatabase.php b/includes/db/CloneDatabase.php
index ee82bdf..2af742e 100644
--- a/includes/db/CloneDatabase.php
+++ b/includes/db/CloneDatabase.php
@@ -132,12 +132,6 @@
$lbFactory = wfGetLBFactory();
$lbFactory->setDomainPrefix( $prefix );
- $lbFactory->forEachLB( function( LoadBalancer $lb ) use (
$prefix ) {
- $lb->setDomainPrefix( $prefix );
- $lb->forEachOpenConnection( function ( IDatabase $db )
use ( $prefix ) {
- $db->tablePrefix( $prefix );
- } );
- } );
$wgDBprefix = $prefix;
}
}
diff --git a/includes/db/Database.php b/includes/db/Database.php
index e908824..3d9b41a 100644
--- a/includes/db/Database.php
+++ b/includes/db/Database.php
@@ -93,9 +93,9 @@
protected $mTrxEndCallbacksSuppressed = false;
/** @var string */
- protected $mTablePrefix;
+ protected $mTablePrefix = '';
/** @var string */
- protected $mSchema;
+ protected $mSchema = '';
/** @var integer */
protected $mFlags;
/** @var array */
@@ -367,7 +367,7 @@
$p['variables'] = isset( $p['variables'] ) ?
$p['variables'] : [];
$p['tablePrefix'] = isset( $p['tablePrefix'] ) ?
$p['tablePrefix'] : '';
if ( !isset( $p['schema'] ) ) {
- $p['schema'] = isset( $defaultSchemas[$dbType]
) ? $defaultSchemas[$dbType] : null;
+ $p['schema'] = isset( $defaultSchemas[$dbType]
) ? $defaultSchemas[$dbType] : '';
}
$p['foreign'] = isset( $p['foreign'] ) ? $p['foreign']
: false;
@@ -466,14 +466,18 @@
public function tablePrefix( $prefix = null ) {
$old = $this->mTablePrefix;
- $this->mTablePrefix = $prefix;
+ if ( $prefix !== null ) {
+ $this->mTablePrefix = $prefix;
+ }
return $old;
}
public function dbSchema( $schema = null ) {
$old = $this->mSchema;
- $this->mSchema = $schema;
+ if ( $schema !== null ) {
+ $this->mSchema = $schema;
+ }
return $old;
}
@@ -699,7 +703,7 @@
}
public function getWikiID() {
- if ( $this->mTablePrefix ) {
+ if ( $this->mTablePrefix != '' ) {
return "{$this->mDBname}-{$this->mTablePrefix}";
} else {
return $this->mDBname;
@@ -1884,7 +1888,7 @@
# In dbs that support it, $database may actually be the
schema
# but that doesn't affect any of the functionality here
$prefix = '';
- $schema = null;
+ $schema = '';
} else {
list( $table ) = $dbDetails;
if ( isset( $this->tableAliases[$table] ) ) {
diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php
b/includes/libs/rdbms/lbfactory/LBFactory.php
index b6f3317..e011ff0 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -633,15 +633,18 @@
}
/**
- * Define a new local domain (for testing)
+ * Set a new table prefix for the existing local domain ID for testing
*
- * Caller should make sure no local connection are open to the old
local domain
- *
- * @param string $domain
+ * @param string $prefix
* @since 1.28
*/
- public function setDomainPrefix( $domain ) {
- $this->localDomain = $domain;
+ public function setDomainPrefix( $prefix ) {
+ list( $dbName, ) = explode( '-', $this->localDomain, 2 );
+ $this->localDomain = "{$dbName}-{$prefix}";
+
+ $this->forEachLB( function( LoadBalancer $lb ) use ( $prefix ) {
+ $lb->setDomainPrefix( $prefix );
+ } );
}
/**
diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index db69de1..57c905f 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -515,7 +515,7 @@
}
if ( $domain === $this->localDomain ) {
- $domain = false;
+ $domain = false; // local connection requested
}
$groups = ( $groups === false || $groups === [] )
@@ -627,6 +627,8 @@
* @since 1.22
*/
public function getConnectionRef( $db, $groups = [], $domain = false ) {
+ $domain = ( $domain !== false ) ? $domain : $this->localDomain;
+
return new DBConnRef( $this, $this->getConnection( $db,
$groups, $domain ) );
}
@@ -650,6 +652,10 @@
}
public function openConnection( $i, $domain = false ) {
+ if ( $domain === $this->localDomain ) {
+ $domain = false; // local connection requested
+ }
+
if ( $domain !== false ) {
$conn = $this->openForeignConnection( $i, $domain );
} elseif ( isset( $this->mConns['local'][$i][0] ) ) {
@@ -1607,7 +1613,10 @@
*/
public function setDomainPrefix( $prefix ) {
list( $dbName, ) = explode( '-', $this->localDomain, 2 );
-
$this->localDomain = "{$dbName}-{$prefix}";
+
+ $this->forEachOpenConnection( function ( IDatabase $db ) use (
$prefix ) {
+ $db->tablePrefix( $prefix );
+ } );
}
}
diff --git a/tests/phpunit/includes/db/DatabaseTest.php
b/tests/phpunit/includes/db/DatabaseTest.php
index d4be6e4..134caf4 100644
--- a/tests/phpunit/includes/db/DatabaseTest.php
+++ b/tests/phpunit/includes/db/DatabaseTest.php
@@ -427,4 +427,26 @@
$this->assertEquals( $origSsl, $db->getFlag( DBO_SSL ) );
$this->assertEquals( $origTrx, $db->getFlag( DBO_TRX ) );
}
+
+ /**
+ * @covers DatabaseBase::tablePrefix()
+ * @covers DatabaseBase::dbSchema()
+ */
+ public function testMutators() {
+ $old = $this->db->tablePrefix();
+ $this->assertType( 'string', $old, 'Prefix is string' );
+ $this->assertEquals( $old, $this->db->tablePrefix(), "Prefix
unchanged" );
+ $this->assertEquals( $old, $this->db->tablePrefix( 'xxx' ) );
+ $this->assertEquals( 'xxx', $this->db->tablePrefix(), "Prefix
set" );
+ $this->db->tablePrefix( $old );
+ $this->assertNotEquals( 'xxx', $this->db->tablePrefix() );
+
+ $old = $this->db->dbSchema();
+ $this->assertType( 'string', $old, 'Schema is string' );
+ $this->assertEquals( $old, $this->db->dbSchema(), "Schema
unchanged" );
+ $this->assertEquals( $old, $this->db->dbSchema( 'xxx' ) );
+ $this->assertEquals( 'xxx', $this->db->dbSchema(), "Schema set"
);
+ $this->db->dbSchema( $old );
+ $this->assertNotEquals( 'xxx', $this->db->dbSchema() );
+ }
}
--
To view, visit https://gerrit.wikimedia.org/r/311097
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Idf392bd9992a215c4fa3bddf275562f3916596aa
Gerrit-PatchSet: 5
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
Gerrit-Reviewer: Gergő Tisza <gti...@wikimedia.org>
Gerrit-Reviewer: Parent5446 <tylerro...@gmail.com>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
