jenkins-bot has submitted this change and it was merged. (
https://gerrit.wikimedia.org/r/323883 )
Change subject: Add LoadBalancer::getMaintenanceConnectionRef() method
......................................................................
Add LoadBalancer::getMaintenanceConnectionRef() method
This is useful when IMaintainableDatabase methods are needed
for foreign wiki connections to things like external store.
Also:
* Set visibility for ExternalStoreDB methods.
* Cleaned up various type hints and comments.
Change-Id: Ie35b1ff21032cc4e78912dc499486da23aeba041
---
M autoload.php
M includes/db/CloneDatabase.php
M includes/externalstore/ExternalStoreDB.php
M includes/libs/rdbms/database/DBConnRef.php
M includes/libs/rdbms/database/Database.php
M includes/libs/rdbms/database/IMaintainableDatabase.php
A includes/libs/rdbms/database/MaintainableDBConnRef.php
M includes/libs/rdbms/loadbalancer/ILoadBalancer.php
M includes/libs/rdbms/loadbalancer/LoadBalancer.php
M tests/phpunit/MediaWikiTestCase.php
10 files changed, 161 insertions(+), 67 deletions(-)
Approvals:
Krinkle: Looks good to me, approved
Addshore: Looks good to me, but someone else must approve
jenkins-bot: Verified
diff --git a/autoload.php b/autoload.php
index e079686..66a9e9b 100644
--- a/autoload.php
+++ b/autoload.php
@@ -803,6 +803,7 @@
'MagicWordArray' => __DIR__ . '/includes/MagicWordArray.php',
'MailAddress' => __DIR__ . '/includes/mail/MailAddress.php',
'MainConfigDependency' => __DIR__ .
'/includes/cache/CacheDependency.php',
+ 'MaintainableDBConnRef' => __DIR__ .
'/includes/libs/rdbms/database/MaintainableDBConnRef.php',
'Maintenance' => __DIR__ . '/maintenance/Maintenance.php',
'MaintenanceFormatInstallDoc' => __DIR__ .
'/maintenance/formatInstallDoc.php',
'MakeTestEdits' => __DIR__ . '/maintenance/makeTestEdits.php',
diff --git a/includes/db/CloneDatabase.php b/includes/db/CloneDatabase.php
index f1ccd2a..2b394b6 100644
--- a/includes/db/CloneDatabase.php
+++ b/includes/db/CloneDatabase.php
@@ -41,19 +41,19 @@
/** @var bool Whether to use temporary tables or not */
private $useTemporaryTables = true;
- /** @var Database */
+ /** @var IMaintainableDatabase */
private $db;
/**
* Constructor
*
- * @param Database $db A database subclass
+ * @param IMaintainableDatabase $db A database subclass
* @param array $tablesToClone An array of tables to clone, unprefixed
* @param string $newTablePrefix Prefix to assign to the tables
* @param string $oldTablePrefix Prefix on current tables, if not
$wgDBprefix
* @param bool $dropCurrentTables
*/
- public function __construct( Database $db, array $tablesToClone,
+ public function __construct( IMaintainableDatabase $db, array
$tablesToClone,
$newTablePrefix, $oldTablePrefix = '', $dropCurrentTables = true
) {
$this->db = $db;
@@ -107,7 +107,8 @@
# Create new table
wfDebug( __METHOD__ . " duplicating $oldTableName to
$newTableName\n" );
- $this->db->duplicateTableStructure( $oldTableName,
$newTableName, $this->useTemporaryTables );
+ $this->db->duplicateTableStructure(
+ $oldTableName, $newTableName,
$this->useTemporaryTables );
}
}
diff --git a/includes/externalstore/ExternalStoreDB.php
b/includes/externalstore/ExternalStoreDB.php
index 7e93299..52c1a4c 100644
--- a/includes/externalstore/ExternalStoreDB.php
+++ b/includes/externalstore/ExternalStoreDB.php
@@ -105,7 +105,7 @@
* @param string $cluster Cluster name
* @return LoadBalancer
*/
- function getLoadBalancer( $cluster ) {
+ private function getLoadBalancer( $cluster ) {
$wiki = isset( $this->params['wiki'] ) ? $this->params['wiki']
: false;
return wfGetLBFactory()->getExternalLB( $cluster, $wiki );
@@ -115,9 +115,9 @@
* Get a replica DB connection for the specified cluster
*
* @param string $cluster Cluster name
- * @return IDatabase
+ * @return DBConnRef
*/
- function getSlave( $cluster ) {
+ public function getSlave( $cluster ) {
global $wgDefaultExternalStore;
$wiki = isset( $this->params['wiki'] ) ? $this->params['wiki']
: false;
@@ -140,13 +140,13 @@
* Get a master database connection for the specified cluster
*
* @param string $cluster Cluster name
- * @return IDatabase
+ * @return MaintainableDBConnRef
*/
- function getMaster( $cluster ) {
+ public function getMaster( $cluster ) {
$wiki = isset( $this->params['wiki'] ) ? $this->params['wiki']
: false;
$lb = $this->getLoadBalancer( $cluster );
- $db = $lb->getConnectionRef( DB_MASTER, [], $wiki );
+ $db = $lb->getMaintenanceConnectionRef( DB_MASTER, [], $wiki );
$db->clearFlag( DBO_TRX ); // sanity
return $db;
@@ -158,7 +158,7 @@
* @param IDatabase $db
* @return string Table name ('blobs' by default)
*/
- function getTable( $db ) {
+ public function getTable( $db ) {
$table = $db->getLBInfo( 'blobs table' );
if ( is_null( $table ) ) {
$table = 'blobs';
@@ -175,9 +175,8 @@
* @param string $id
* @param string $itemID
* @return HistoryBlob|bool Returns false if missing
- * @private
*/
- function fetchBlob( $cluster, $id, $itemID ) {
+ private function fetchBlob( $cluster, $id, $itemID ) {
/**
* One-step cache variable to hold base blobs; operations that
* pull multiple revisions may often pull multiple times from
@@ -230,7 +229,7 @@
* @return array A map from the blob_id's requested to their content.
* Unlocated ids are not represented
*/
- function batchFetchBlobs( $cluster, array $ids ) {
+ private function batchFetchBlobs( $cluster, array $ids ) {
$dbr = $this->getSlave( $cluster );
$res = $dbr->select( $this->getTable( $dbr ),
[ 'blob_id', 'blob_text' ], [ 'blob_id' => array_keys(
$ids ) ], __METHOD__ );
diff --git a/includes/libs/rdbms/database/DBConnRef.php
b/includes/libs/rdbms/database/DBConnRef.php
index 20198bf..b268b9f 100644
--- a/includes/libs/rdbms/database/DBConnRef.php
+++ b/includes/libs/rdbms/database/DBConnRef.php
@@ -10,10 +10,8 @@
class DBConnRef implements IDatabase {
/** @var ILoadBalancer */
private $lb;
-
- /** @var IDatabase|null Live connection handle */
+ /** @var Database|null Live connection handle */
private $conn;
-
/** @var array|null N-tuple of (server index, group,
DatabaseDomain|string) */
private $params;
@@ -22,12 +20,12 @@
const FLD_DOMAIN = 2;
/**
- * @param ILoadBalancer $lb
- * @param IDatabase|array $conn Connection or (server index, group,
DatabaseDomain|string)
+ * @param ILoadBalancer $lb Connection manager for $conn
+ * @param Database|array $conn New connection handle or (server index,
query groups, domain)
*/
public function __construct( ILoadBalancer $lb, $conn ) {
$this->lb = $lb;
- if ( $conn instanceof IDatabase ) {
+ if ( $conn instanceof Database ) {
$this->conn = $conn; // live handle
} elseif ( count( $conn ) >= 3 && $conn[self::FLD_DOMAIN] !==
false ) {
$this->params = $conn;
@@ -595,7 +593,7 @@
* Clean up the connection when out of scope
*/
function __destruct() {
- if ( $this->conn !== null ) {
+ if ( $this->conn ) {
$this->lb->reuseConnection( $this->conn );
}
}
diff --git a/includes/libs/rdbms/database/Database.php
b/includes/libs/rdbms/database/Database.php
index 3d35d76..b9beac8 100644
--- a/includes/libs/rdbms/database/Database.php
+++ b/includes/libs/rdbms/database/Database.php
@@ -2862,23 +2862,8 @@
return $this->mTrxLevel && ( $this->mTrxAtomicLevels ||
!$this->mTrxAutomatic );
}
- /**
- * Creates a new table with structure copied from existing table
- * Note that unlike most database abstraction functions, this function
does not
- * automatically append database prefix, because it works at a lower
- * abstraction level.
- * The table names passed to this function shall not be quoted (this
- * function calls addIdentifierQuotes when needed).
- *
- * @param string $oldName Name of table whose structure should be copied
- * @param string $newName Name of table to be created
- * @param bool $temporary Whether the new table should be temporary
- * @param string $fname Calling function name
- * @throws RuntimeException
- * @return bool True if operation was successful
- */
- public function duplicateTableStructure( $oldName, $newName, $temporary
= false,
- $fname = __METHOD__
+ public function duplicateTableStructure(
+ $oldName, $newName, $temporary = false, $fname = __METHOD__
) {
throw new RuntimeException( __METHOD__ . ' is not implemented
in descendant class' );
}
diff --git a/includes/libs/rdbms/database/IMaintainableDatabase.php
b/includes/libs/rdbms/database/IMaintainableDatabase.php
index 8395359..43cec28 100644
--- a/includes/libs/rdbms/database/IMaintainableDatabase.php
+++ b/includes/libs/rdbms/database/IMaintainableDatabase.php
@@ -186,4 +186,23 @@
* @return array
*/
public function listViews( $prefix = null, $fname = __METHOD__ );
+
+ /**
+ * Creates a new table with structure copied from existing table
+ *
+ * Note that unlike most database abstraction functions, this function
does not
+ * automatically append database prefix, because it works at a lower
abstraction level.
+ * The table names passed to this function shall not be quoted (this
function calls
+ * addIdentifierQuotes() when needed).
+ *
+ * @param string $oldName Name of table whose structure should be copied
+ * @param string $newName Name of table to be created
+ * @param bool $temporary Whether the new table should be temporary
+ * @param string $fname Calling function name
+ * @return bool True if operation was successful
+ * @throws RuntimeException
+ */
+ public function duplicateTableStructure(
+ $oldName, $newName, $temporary = false, $fname = __METHOD__
+ );
}
diff --git a/includes/libs/rdbms/database/MaintainableDBConnRef.php
b/includes/libs/rdbms/database/MaintainableDBConnRef.php
new file mode 100644
index 0000000..fa3ddf9
--- /dev/null
+++ b/includes/libs/rdbms/database/MaintainableDBConnRef.php
@@ -0,0 +1,68 @@
+<?php
+/**
+ * Helper class to handle automatically marking connections as reusable (via
RAII pattern)
+ * as well handling deferring the actual network connection until the handle
is used
+ *
+ * @note: proxy methods are defined explicity to avoid interface errors
+ * @ingroup Database
+ * @since 1.29
+ */
+class MaintainableDBConnRef extends DBConnRef implements IMaintainableDatabase
{
+ public function tableName( $name, $format = 'quoted' ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function tableNames() {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function tableNamesN() {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function sourceFile(
+ $filename,
+ callable $lineCallback = null,
+ callable $resultCallback = null,
+ $fname = false,
+ callable $inputCallback = null
+ ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function sourceStream(
+ $fp,
+ callable $lineCallback = null,
+ callable $resultCallback = null,
+ $fname = __METHOD__,
+ callable $inputCallback = null
+ ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function dropTable( $tableName, $fName = __METHOD__ ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function deadlockLoop() {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function listViews( $prefix = null, $fname = __METHOD__ ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function textFieldSize( $table, $field ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function streamStatementEnd( &$sql, &$newLine ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+
+ public function duplicateTableStructure(
+ $oldName, $newName, $temporary = false, $fname = __METHOD__
+ ) {
+ return $this->__call( __FUNCTION__, func_get_args() );
+ }
+}
diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index 8854479..fc306b4 100644
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
@@ -108,8 +108,9 @@
/**
* Get the index of the reader connection, which may be a replica DB
+ *
* This takes into account load ratios and lag times. It should
- * always return a consistent index during a given invocation
+ * always return a consistent index during a given invocation.
*
* Side effect: opens connections to databases
* @param string|bool $group Query group, or false for the generic
reader
@@ -121,8 +122,10 @@
/**
* Set the master wait position
- * If a DB_REPLICA connection has been opened already, waits
- * Otherwise sets a variable telling it to wait if such a connection is
opened
+ *
+ * If a DB_REPLICA connection has been opened already, then wait
immediately.
+ * Otherwise sets a variable telling it to wait if such a connection is
opened.
+ *
* @param DBMasterPos $pos
*/
public function waitFor( $pos );
@@ -140,6 +143,7 @@
/**
* Set the master wait position and wait for ALL replica DBs to catch
up to it
+ *
* @param DBMasterPos $pos
* @param int $timeout Max seconds to wait; default is mWaitTimeout
* @return bool Success (able to connect and no timeouts reached)
@@ -148,30 +152,29 @@
/**
* Get any open connection to a given server index, local or foreign
- * Returns false if there is no connection open
*
- * @param int $i Server index
- * @return IDatabase|bool False on failure
+ * @param int $i Server index or DB_MASTER/DB_REPLICA
+ * @return Database|bool False if no such connection is open
*/
public function getAnyOpenConnection( $i );
/**
* Get a connection by index
- * This is the main entry point for this class.
*
- * @param int $i Server index
+ * @param int $i Server index or DB_MASTER/DB_REPLICA
* @param array|string|bool $groups Query group(s), or false for the
generic reader
* @param string|bool $domain Domain ID, or false for the current domain
*
* @throws DBError
- * @return IDatabase
+ * @return Database
*/
public function getConnection( $i, $groups = [], $domain = false );
/**
- * Mark a foreign connection as being available for reuse under a
different
- * DB name or prefix. This mechanism is reference-counted, and must be
called
- * the same number of times as getConnection() to work.
+ * Mark a foreign connection as being available for reuse under a
different DB domain
+ *
+ * This mechanism is reference-counted, and must be called the same
number of times
+ * as getConnection() to work.
*
* @param IDatabase $conn
* @throws InvalidArgumentException
@@ -181,30 +184,44 @@
/**
* Get a database connection handle reference
*
- * The handle's methods wrap simply wrap those of a IDatabase handle
+ * The handle's methods simply wrap those of a Database handle
*
- * @see LoadBalancer::getConnection() for parameter information
+ * @see ILoadBalancer::getConnection() for parameter information
*
- * @param int $db
+ * @param int $i Server index or DB_MASTER/DB_REPLICA
* @param array|string|bool $groups Query group(s), or false for the
generic reader
* @param string|bool $domain Domain ID, or false for the current domain
* @return DBConnRef
*/
- public function getConnectionRef( $db, $groups = [], $domain = false );
+ public function getConnectionRef( $i, $groups = [], $domain = false );
/**
* Get a database connection handle reference without connecting yet
*
- * The handle's methods wrap simply wrap those of a IDatabase handle
+ * The handle's methods simply wrap those of a Database handle
*
- * @see LoadBalancer::getConnection() for parameter information
+ * @see ILoadBalancer::getConnection() for parameter information
*
- * @param int $db
+ * @param int $i Server index or DB_MASTER/DB_REPLICA
* @param array|string|bool $groups Query group(s), or false for the
generic reader
* @param string|bool $domain Domain ID, or false for the current domain
* @return DBConnRef
*/
- public function getLazyConnectionRef( $db, $groups = [], $domain =
false );
+ public function getLazyConnectionRef( $i, $groups = [], $domain = false
);
+
+ /**
+ * Get a maintenance database connection handle reference for
migrations and schema changes
+ *
+ * The handle's methods simply wrap those of a Database handle
+ *
+ * @see ILoadBalancer::getConnection() for parameter information
+ *
+ * @param int $db Server index or DB_MASTER/DB_REPLICA
+ * @param array|string|bool $groups Query group(s), or false for the
generic reader
+ * @param string|bool $domain Domain ID, or false for the current domain
+ * @return MaintainableDBConnRef
+ */
+ public function getMaintenanceConnectionRef( $db, $groups = [], $domain
= false );
/**
* Open a connection to the server given by the specified index
@@ -216,9 +233,9 @@
*
* @note If disable() was called on this LoadBalancer, this method will
throw a DBAccessError.
*
- * @param int $i Server index
+ * @param int $i Server index or DB_MASTER/DB_REPLICA
* @param string|bool $domain Domain ID, or false for the current domain
- * @return IDatabase|bool Returns false on errors
+ * @return Database|bool Returns false on errors
* @throws DBAccessError
*/
public function openConnection( $i, $domain = false );
diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 634993a..8601785 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -657,6 +657,12 @@
return new DBConnRef( $this, [ $db, $groups, $domain ] );
}
+ public function getMaintenanceConnectionRef( $db, $groups = [], $domain
= false ) {
+ $domain = ( $domain !== false ) ? $domain : $this->localDomain;
+
+ return new MaintainableDBConnRef( $this, $this->getConnection(
$db, $groups, $domain ) );
+ }
+
/**
* @see ILoadBalancer::openConnection()
*
diff --git a/tests/phpunit/MediaWikiTestCase.php
b/tests/phpunit/MediaWikiTestCase.php
index db1df5c..76559af 100644
--- a/tests/phpunit/MediaWikiTestCase.php
+++ b/tests/phpunit/MediaWikiTestCase.php
@@ -1087,11 +1087,11 @@
* Clones all tables in the given database (whatever database that
connection has
* open), to versions with the test prefix.
*
- * @param Database $db Database to use
+ * @param IMaintainableDatabase $db Database to use
* @param string $prefix Prefix to use for test tables
* @return bool True if tables were cloned, false if only the prefix
was changed
*/
- protected static function setupDatabaseWithTestPrefix( Database $db,
$prefix ) {
+ protected static function setupDatabaseWithTestPrefix(
IMaintainableDatabase $db, $prefix ) {
$tablesCloned = self::listTables( $db );
$dbClone = new CloneDatabase( $db, $tablesCloned, $prefix );
$dbClone->useTemporaryTables( self::$useTemporaryTables );
@@ -1210,9 +1210,7 @@
list( $proto, $cluster ) = explode( '://',
$url, 2 );
// Avoid getMaster() because
setupDatabaseWithTestPrefix()
// requires Database instead of plain
DBConnRef/IDatabase
- $lb = $externalStoreDB->getLoadBalancer(
$cluster );
- $dbw = $lb->getConnection( DB_MASTER );
- $dbws[] = $dbw;
+ $dbws[] = $externalStoreDB->getMaster( $cluster
);
}
}
@@ -1326,11 +1324,11 @@
/**
* @since 1.18
*
- * @param Database $db
+ * @param IMaintainableDatabase $db
*
* @return array
*/
- public static function listTables( Database $db ) {
+ public static function listTables( IMaintainableDatabase $db ) {
$prefix = $db->tablePrefix();
$tables = $db->listTables( $prefix, __METHOD__ );
@@ -1378,6 +1376,8 @@
if ( isset( PHPUnitMaintClass::$additionalOptions[$offset] ) ) {
return PHPUnitMaintClass::$additionalOptions[$offset];
}
+
+ return null;
}
/**
--
To view, visit https://gerrit.wikimedia.org/r/323883
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie35b1ff21032cc4e78912dc499486da23aeba041
Gerrit-PatchSet: 6
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <[email protected]>
Gerrit-Reviewer: Addshore <[email protected]>
Gerrit-Reviewer: Krinkle <[email protected]>
Gerrit-Reviewer: jenkins-bot <>
_______________________________________________
MediaWiki-commits mailing list
[email protected]
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
