aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--RELEASE-NOTES-1.394
-rw-r--r--includes/MediaWikiServices.php9
-rw-r--r--includes/ServiceWiring.php49
-rw-r--r--includes/db/MWLBFactory.php125
-rw-r--r--includes/export/WikiExporter.php17
-rw-r--r--includes/libs/rdbms/database/DBConnRef.php47
-rw-r--r--includes/libs/rdbms/lbfactory/LBFactory.php115
-rw-r--r--includes/libs/rdbms/lbfactory/LBFactoryMulti.php3
-rw-r--r--includes/libs/rdbms/loadbalancer/ILoadBalancerForOwner.php14
-rw-r--r--includes/libs/rdbms/loadbalancer/LoadBalancer.php70
-rw-r--r--maintenance/includes/Maintenance.php12
-rw-r--r--tests/phpunit/includes/db/LBFactoryTest.php165
-rw-r--r--tests/phpunit/includes/db/LoadBalancerTest.php32
-rw-r--r--tests/phpunit/unit/includes/db/MWLBFactoryTest.php20
-rw-r--r--tests/phpunit/unit/includes/libs/rdbms/database/DBConnRefTest.php42
15 files changed, 590 insertions, 134 deletions
diff --git a/RELEASE-NOTES-1.39 b/RELEASE-NOTES-1.39
index 2ea74ea4e81e..a38b053979c0 100644
--- a/RELEASE-NOTES-1.39
+++ b/RELEASE-NOTES-1.39
@@ -35,6 +35,10 @@ For notes on 1.38.x and older releases, see HISTORY.
* $wgCopyUploadAllowOnWikiDomainConfig – Configures if administrators can use
the MediaWiki:Copyupload-allowed-domains system message to define which
domains can be used with the upload-by-url tool.
+* $wgLBFactoryConf['configCallback'] can be set to a callback function that
+ returns an array with keys to update in $wgLBFactoryConf. This can be used
+ to update the database configuration on the fly, e.g. to take replica hosts
+ out of rotation.
* …
==== Changed configuration ====
diff --git a/includes/MediaWikiServices.php b/includes/MediaWikiServices.php
index c1301baeeb05..600226242ec0 100644
--- a/includes/MediaWikiServices.php
+++ b/includes/MediaWikiServices.php
@@ -152,6 +152,7 @@ use MediaWiki\Watchlist\WatchlistManager;
use MessageCache;
use MimeAnalyzer;
use MWException;
+use MWLBFactory;
use NamespaceInfo;
use ObjectCache;
use OldRevisionImporter;
@@ -941,6 +942,14 @@ class MediaWikiServices extends ServiceContainer {
}
/**
+ * @since 1.39
+ * @return MWLBFactory
+ */
+ public function getDBLoadBalancerFactoryConfigBuilder(): MWLBFactory {
+ return $this->getService( 'DBLoadBalancerFactoryConfigBuilder' );
+ }
+
+ /**
* @since 1.37
* @return DeletePageFactory
*/
diff --git a/includes/ServiceWiring.php b/includes/ServiceWiring.php
index e81a67d4bf01..94cb7633dece 100644
--- a/includes/ServiceWiring.php
+++ b/includes/ServiceWiring.php
@@ -536,8 +536,32 @@ return [
'DBLoadBalancerFactory' =>
static function ( MediaWikiServices $services ): Wikimedia\Rdbms\LBFactory {
$mainConfig = $services->getMainConfig();
+ $lbFactoryConfigBuilder = $services->getDBLoadBalancerFactoryConfigBuilder();
- $cpStashType = $mainConfig->get( MainConfigNames::ChronologyProtectorStash );
+ $lbConf = $lbFactoryConfigBuilder->applyDefaultConfig(
+ $mainConfig->get( MainConfigNames::LBFactoryConf )
+ );
+
+ $class = $lbFactoryConfigBuilder->getLBFactoryClass( $lbConf );
+ $instance = new $class( $lbConf );
+
+ $lbFactoryConfigBuilder->setDomainAliases( $instance );
+
+ // NOTE: This accesses ProxyLookup from the MediaWikiServices singleton
+ // for non-essential non-nonimal purposes (via WebRequest::getIP).
+ // This state is fine (and meant) to be consistent for a given PHP process,
+ // even if applied to the service container for a different wiki.
+ $lbFactoryConfigBuilder->applyGlobalState(
+ $instance,
+ $mainConfig,
+ $services->getStatsdDataFactory()
+ );
+
+ return $instance;
+ },
+
+ 'DBLoadBalancerFactoryConfigBuilder' => static function ( MediaWikiServices $services ): MWLBFactory {
+ $cpStashType = $services->getMainConfig()->get( MainConfigNames::ChronologyProtectorStash );
if ( is_string( $cpStashType ) ) {
$cpStash = ObjectCache::getInstance( $cpStashType );
} else {
@@ -559,10 +583,8 @@ return [
// Use process cache if no APCU or other local-server cache (e.g. on CLI)
$srvCache = new HashBagOStuff( [ 'maxKeys' => 100 ] );
}
-
- $lbConf = MWLBFactory::applyDefaultConfig(
- $mainConfig->get( MainConfigNames::LBFactoryConf ),
- new ServiceOptions( MWLBFactory::APPLY_DEFAULT_CONFIG_OPTIONS, $mainConfig ),
+ return new MWLBFactory(
+ new ServiceOptions( MWLBFactory::APPLY_DEFAULT_CONFIG_OPTIONS, $services->getMainConfig() ),
$services->getConfiguredReadOnlyMode(),
$cpStash,
$srvCache,
@@ -570,23 +592,6 @@ return [
$services->getCriticalSectionProvider(),
$services->getStatsdDataFactory()
);
-
- $class = MWLBFactory::getLBFactoryClass( $lbConf );
- $instance = new $class( $lbConf );
-
- MWLBFactory::setDomainAliases( $instance );
-
- // NOTE: This accesses ProxyLookup from the MediaWikiServices singleton
- // for non-essential non-nonimal purposes (via WebRequest::getIP).
- // This state is fine (and meant) to be consistent for a given PHP process,
- // even if applied to the service container for a different wiki.
- MWLBFactory::applyGlobalState(
- $instance,
- $mainConfig,
- $services->getStatsdDataFactory()
- );
-
- return $instance;
},
'DeletePageFactory' => static function ( MediaWikiServices $services ): DeletePageFactory {
diff --git a/includes/db/MWLBFactory.php b/includes/db/MWLBFactory.php
index 7fbcd2873b06..81cea3d78e7b 100644
--- a/includes/db/MWLBFactory.php
+++ b/includes/db/MWLBFactory.php
@@ -38,7 +38,7 @@ use Wikimedia\RequestTimeout\CriticalSectionProvider;
* @internal For use by core ServiceWiring only.
* @ingroup Database
*/
-abstract class MWLBFactory {
+class MWLBFactory {
/** @var array Cache of already-logged deprecation messages */
private static $loggedDeprecations = [];
@@ -67,9 +67,36 @@ abstract class MWLBFactory {
MainConfigNames::SQLiteDataDir,
MainConfigNames::SQLMode,
];
+ /**
+ * @var ServiceOptions
+ */
+ private $options;
+ /**
+ * @var ConfiguredReadOnlyMode
+ */
+ private $readOnlyMode;
+ /**
+ * @var BagOStuff
+ */
+ private $cpStash;
+ /**
+ * @var BagOStuff
+ */
+ private $srvCache;
+ /**
+ * @var WANObjectCache
+ */
+ private $wanCache;
+ /**
+ * @var CriticalSectionProvider
+ */
+ private $csProvider;
+ /**
+ * @var StatsdDataFactoryInterface
+ */
+ private $statsdDataFactory;
/**
- * @param array $lbConf Config for LBFactory::__construct()
* @param ServiceOptions $options
* @param ConfiguredReadOnlyMode $readOnlyMode
* @param BagOStuff $cpStash
@@ -77,11 +104,8 @@ abstract class MWLBFactory {
* @param WANObjectCache $wanCache
* @param CriticalSectionProvider $csProvider
* @param StatsdDataFactoryInterface $statsdDataFactory
- * @return array
- * @internal For use with service wiring
*/
- public static function applyDefaultConfig(
- array $lbConf,
+ public function __construct(
ServiceOptions $options,
ConfiguredReadOnlyMode $readOnlyMode,
BagOStuff $cpStash,
@@ -90,15 +114,30 @@ abstract class MWLBFactory {
CriticalSectionProvider $csProvider,
StatsdDataFactoryInterface $statsdDataFactory
) {
- $options->assertRequiredOptions( self::APPLY_DEFAULT_CONFIG_OPTIONS );
+ $this->options = $options;
+ $this->readOnlyMode = $readOnlyMode;
+ $this->cpStash = $cpStash;
+ $this->srvCache = $srvCache;
+ $this->wanCache = $wanCache;
+ $this->csProvider = $csProvider;
+ $this->statsdDataFactory = $statsdDataFactory;
+ }
+
+ /**
+ * @param array $lbConf Config for LBFactory::__construct()
+ * @return array
+ * @internal For use with service wiring
+ */
+ public function applyDefaultConfig( array $lbConf ) {
+ $this->options->assertRequiredOptions( self::APPLY_DEFAULT_CONFIG_OPTIONS );
$typesWithSchema = self::getDbTypesWithSchemas();
$lbConf += [
'localDomain' => new DatabaseDomain(
- $options->get( MainConfigNames::DBname ),
- $options->get( MainConfigNames::DBmwschema ),
- $options->get( MainConfigNames::DBprefix )
+ $this->options->get( MainConfigNames::DBname ),
+ $this->options->get( MainConfigNames::DBmwschema ),
+ $this->options->get( MainConfigNames::DBprefix )
),
'profiler' => static function ( $section ) {
return Profiler::instance()->scopedProfileIn( $section );
@@ -110,11 +149,11 @@ abstract class MWLBFactory {
'perfLogger' => LoggerFactory::getInstance( 'DBPerformance' ),
'errorLogger' => [ MWExceptionHandler::class, 'logException' ],
'deprecationLogger' => [ static::class, 'logDeprecation' ],
- 'statsdDataFactory' => $statsdDataFactory,
- 'cliMode' => $options->get( 'CommandLineMode' ),
- 'readOnlyReason' => $readOnlyMode->getReason(),
- 'defaultGroup' => $options->get( MainConfigNames::DBDefaultGroup ),
- 'criticalSectionProvider' => $csProvider
+ 'statsdDataFactory' => $this->statsdDataFactory,
+ 'cliMode' => $this->options->get( 'CommandLineMode' ),
+ 'readOnlyReason' => $this->readOnlyMode->getReason(),
+ 'defaultGroup' => $this->options->get( MainConfigNames::DBDefaultGroup ),
+ 'criticalSectionProvider' => $this->csProvider
];
$serversCheck = [];
@@ -124,55 +163,55 @@ abstract class MWLBFactory {
if ( $lbConf['class'] === Wikimedia\Rdbms\LBFactorySimple::class ) {
if ( isset( $lbConf['servers'] ) ) {
// Server array is already explicitly configured
- } elseif ( is_array( $options->get( MainConfigNames::DBservers ) ) ) {
+ } elseif ( is_array( $this->options->get( MainConfigNames::DBservers ) ) ) {
$lbConf['servers'] = [];
- foreach ( $options->get( MainConfigNames::DBservers ) as $i => $server ) {
- $lbConf['servers'][$i] = self::initServerInfo( $server, $options );
+ foreach ( $this->options->get( MainConfigNames::DBservers ) as $i => $server ) {
+ $lbConf['servers'][$i] = self::initServerInfo( $server, $this->options );
}
} else {
$server = self::initServerInfo(
[
- 'host' => $options->get( MainConfigNames::DBserver ),
- 'user' => $options->get( MainConfigNames::DBuser ),
- 'password' => $options->get( MainConfigNames::DBpassword ),
- 'dbname' => $options->get( MainConfigNames::DBname ),
- 'type' => $options->get( MainConfigNames::DBtype ),
+ 'host' => $this->options->get( MainConfigNames::DBserver ),
+ 'user' => $this->options->get( MainConfigNames::DBuser ),
+ 'password' => $this->options->get( MainConfigNames::DBpassword ),
+ 'dbname' => $this->options->get( MainConfigNames::DBname ),
+ 'type' => $this->options->get( MainConfigNames::DBtype ),
'load' => 1
],
- $options
+ $this->options
);
- if ( $options->get( MainConfigNames::DBssl ) ) {
+ if ( $this->options->get( MainConfigNames::DBssl ) ) {
$server['ssl'] = true;
}
- $server['flags'] |= $options->get( MainConfigNames::DBcompress ) ? DBO_COMPRESS : 0;
+ $server['flags'] |= $this->options->get( MainConfigNames::DBcompress ) ? DBO_COMPRESS : 0;
$lbConf['servers'] = [ $server ];
}
if ( !isset( $lbConf['externalClusters'] ) ) {
- $lbConf['externalClusters'] = $options->get( MainConfigNames::ExternalServers );
+ $lbConf['externalClusters'] = $this->options->get( MainConfigNames::ExternalServers );
}
$serversCheck = $lbConf['servers'];
} elseif ( $lbConf['class'] === Wikimedia\Rdbms\LBFactoryMulti::class ) {
if ( isset( $lbConf['serverTemplate'] ) ) {
if ( in_array( $lbConf['serverTemplate']['type'], $typesWithSchema, true ) ) {
- $lbConf['serverTemplate']['schema'] = $options->get( MainConfigNames::DBmwschema );
+ $lbConf['serverTemplate']['schema'] = $this->options->get( MainConfigNames::DBmwschema );
}
- $lbConf['serverTemplate']['sqlMode'] = $options->get( MainConfigNames::SQLMode );
+ $lbConf['serverTemplate']['sqlMode'] = $this->options->get( MainConfigNames::SQLMode );
$serversCheck = [ $lbConf['serverTemplate'] ];
}
}
self::assertValidServerConfigs(
$serversCheck,
- $options->get( MainConfigNames::DBname ),
- $options->get( MainConfigNames::DBprefix )
+ $this->options->get( MainConfigNames::DBname ),
+ $this->options->get( MainConfigNames::DBprefix )
);
- $lbConf['cpStash'] = $cpStash;
- $lbConf['srvCache'] = $srvCache;
- $lbConf['wanCache'] = $wanCache;
+ $lbConf['cpStash'] = $this->cpStash;
+ $lbConf['srvCache'] = $this->srvCache;
+ $lbConf['wanCache'] = $this->wanCache;
return $lbConf;
}
@@ -180,7 +219,7 @@ abstract class MWLBFactory {
/**
* @return array
*/
- private static function getDbTypesWithSchemas() {
+ private function getDbTypesWithSchemas() {
return [ 'postgres' ];
}
@@ -189,7 +228,7 @@ abstract class MWLBFactory {
* @param ServiceOptions $options
* @return array
*/
- private static function initServerInfo( array $server, ServiceOptions $options ) {
+ private function initServerInfo( array $server, ServiceOptions $options ) {
if ( $server['type'] === 'sqlite' ) {
$httpMethod = $_SERVER['REQUEST_METHOD'] ?? null;
// T93097: hint for how file-based databases (e.g. sqlite) should go about locking.
@@ -238,7 +277,7 @@ abstract class MWLBFactory {
* @param string $ldDB Local domain database name
* @param string $ldTP Local domain prefix
*/
- private static function assertValidServerConfigs( array $servers, $ldDB, $ldTP ) {
+ private function assertValidServerConfigs( array $servers, $ldDB, $ldTP ) {
foreach ( $servers as $server ) {
$type = $server['type'] ?? null;
$srvDB = $server['dbname'] ?? null; // server DB
@@ -267,7 +306,7 @@ abstract class MWLBFactory {
* @param string $dbType Database type
* @return never
*/
- private static function reportIfPrefixSet( $prefix, $dbType ) {
+ private function reportIfPrefixSet( $prefix, $dbType ) {
$e = new UnexpectedValueException(
"\$wgDBprefix is set to '$prefix' but the database type is '$dbType'. " .
"MediaWiki does not support using a table prefix with this RDBMS type."
@@ -281,7 +320,7 @@ abstract class MWLBFactory {
* @param string $ldDB Local DB domain database
* @return never
*/
- private static function reportMismatchedDBs( $srvDB, $ldDB ) {
+ private function reportMismatchedDBs( $srvDB, $ldDB ) {
$e = new UnexpectedValueException(
"\$wgDBservers has dbname='$srvDB' but \$wgDBname='$ldDB'. " .
"Set \$wgDBname to the database used by this wiki project. " .
@@ -299,7 +338,7 @@ abstract class MWLBFactory {
* @param string $ldTP Local DB domain database
* @return never
*/
- private static function reportMismatchedPrefixes( $srvTP, $ldTP ) {
+ private function reportMismatchedPrefixes( $srvTP, $ldTP ) {
$e = new UnexpectedValueException(
"\$wgDBservers has tablePrefix='$srvTP' but \$wgDBprefix='$ldTP'. " .
"Set \$wgDBprefix to the table prefix used by this wiki project. " .
@@ -319,7 +358,7 @@ abstract class MWLBFactory {
* @param array $config (e.g. $wgLBFactoryConf)
* @return string Class name
*/
- public static function getLBFactoryClass( array $config ) {
+ public function getLBFactoryClass( array $config ) {
$compat = [
// For LocalSettings.php compat after removing underscores (since 1.23).
'LBFactory_Single' => Wikimedia\Rdbms\LBFactorySingle::class,
@@ -338,7 +377,7 @@ abstract class MWLBFactory {
/**
* @param ILBFactory $lbFactory
*/
- public static function setDomainAliases( ILBFactory $lbFactory ) {
+ public function setDomainAliases( ILBFactory $lbFactory ) {
$domain = DatabaseDomain::newFromId( $lbFactory->getLocalDomainID() );
// For compatibility with hyphenated $wgDBname values on older wikis, handle callers
// that assume corresponding database domain IDs and wiki IDs have identical values
@@ -372,7 +411,7 @@ abstract class MWLBFactory {
* @param Config $config
* @param IBufferingStatsdDataFactory $stats
*/
- public static function applyGlobalState(
+ public function applyGlobalState(
ILBFactory $lbFactory,
Config $config,
IBufferingStatsdDataFactory $stats
diff --git a/includes/export/WikiExporter.php b/includes/export/WikiExporter.php
index 9ee4a89a95cf..1e77eb05f84c 100644
--- a/includes/export/WikiExporter.php
+++ b/includes/export/WikiExporter.php
@@ -367,6 +367,7 @@ class WikiExporter {
}
$lastLogId = $this->outputLogStream( $result );
+ $this->reloadDBConfig();
}
}
@@ -501,6 +502,10 @@ class WikiExporter {
if ( $done && $lastRow ) {
$this->finishPageStreamOutput( $lastRow );
}
+
+ if ( !$done ) {
+ $this->reloadDBConfig();
+ }
}
}
@@ -554,6 +559,7 @@ class WikiExporter {
. ' revision ' . $revRow->rev_id . ': ' . $ex->getMessage() );
}
$lastRow = $revRow;
+ $this->reloadDBConfig();
}
if ( $rowCarry ) {
@@ -621,4 +627,15 @@ class WikiExporter {
}
return $row->log_id ?? null;
}
+
+ /**
+ * Attempt to reload the database configuration, so any changes can take effect.
+ * Dynamic reloading can be enabled by setting $wgLBFactoryConf['configCallback']
+ * to a function that returns an array of any keys that should be updated
+ * in LBFactoryConf.
+ */
+ private function reloadDBConfig() {
+ MediaWikiServices::getInstance()->getDBLoadBalancerFactory()
+ ->autoReconfigure();
+ }
}
diff --git a/includes/libs/rdbms/database/DBConnRef.php b/includes/libs/rdbms/database/DBConnRef.php
index 0c0ffee86272..95982fbc22d8 100644
--- a/includes/libs/rdbms/database/DBConnRef.php
+++ b/includes/libs/rdbms/database/DBConnRef.php
@@ -36,6 +36,18 @@ class DBConnRef implements IMaintainableDatabase {
/** @var int One of DB_PRIMARY/DB_REPLICA */
private $role;
+ /**
+ * @var int Reference to the $modcount passed to the constructor.
+ * $conn is valid if $modCountRef and $modCountFix are the same.
+ */
+ private $modCountRef;
+
+ /**
+ * @var int Last known good value of $modCountRef
+ * $conn is valid if $modCountRef and $modCountFix are the same.
+ */
+ private $modCountFix;
+
private const FLD_INDEX = 0;
private const FLD_GROUP = 1;
private const FLD_DOMAIN = 2;
@@ -45,11 +57,19 @@ class DBConnRef implements IMaintainableDatabase {
* @param ILoadBalancer $lb Connection manager for $conn
* @param IDatabase|array $conn Database or (server index, query groups, domain, flags)
* @param int $role The type of connection asked for; one of DB_PRIMARY/DB_REPLICA
+ * @param null|int &$modcount Reference to a modification counter for invalidating
+ * any wrapped connection and forcing a new connection to be acquired.
+ *
* @internal This method should not be called outside of LoadBalancer
*/
- public function __construct( ILoadBalancer $lb, $conn, $role ) {
+ public function __construct( ILoadBalancer $lb, $conn, $role, &$modcount = 0 ) {
$this->lb = $lb;
$this->role = $role;
+
+ // $this->conn is valid as long as $this->modCountRef and $this->modCountFix are the same.
+ $this->modCountRef = &$modcount; // remember reference
+ $this->modCountFix = $modcount; // remember current value
+
if ( $conn instanceof IDatabase && !( $conn instanceof DBConnRef ) ) {
$this->conn = $conn; // live handle
} elseif ( is_array( $conn ) && count( $conn ) >= 4 && $conn[self::FLD_DOMAIN] !== false ) {
@@ -59,11 +79,32 @@ class DBConnRef implements IMaintainableDatabase {
}
}
- public function __call( $name, array $arguments ) {
+ /**
+ * Make sure we are using the correct connection.
+ */
+ private function ensureConnection() {
+ if ( $this->modCountFix !== $this->modCountRef ) {
+ // Mod counter changed, discard existing connection,
+ // Unless we are in an ongoing transaction.
+ // This is triggered by LoadBalancer::reconfigure(), to allow changed settings
+ // to take effect. The primary use case are replica servers being taken out of
+ // rotation, or the primary database changing.
+
+ if ( !$this->conn->trxLevel() ) {
+ $this->lb->closeConnection( $this->conn );
+ $this->conn = null;
+ }
+ }
+
if ( $this->conn === null ) {
- list( $index, $groups, $wiki, $flags ) = $this->params;
+ [ $index, $groups, $wiki, $flags ] = $this->params;
$this->conn = $this->lb->getConnectionInternal( $index, $groups, $wiki, $flags );
+ $this->modCountFix = $this->modCountRef;
}
+ }
+
+ public function __call( $name, array $arguments ) {
+ $this->ensureConnection();
return $this->conn->$name( ...$arguments );
}
diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php
index d6a56a287ffd..dad2e59f18aa 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -128,7 +128,39 @@ abstract class LBFactory implements ILBFactory {
private static $loggerFields =
[ 'replLogger', 'connLogger', 'queryLogger', 'perfLogger' ];
+ /**
+ * @var callable
+ */
+ private $configCallback = null;
+
+ /**
+ * @var array
+ */
+ private $currentConfig;
+
public function __construct( array $conf ) {
+ $this->configure( $conf );
+
+ if ( isset( $conf['configCallback'] ) ) {
+ $this->configCallback = $conf['configCallback'];
+ }
+
+ $this->requestInfo = [
+ 'IPAddress' => $_SERVER['REMOTE_ADDR'] ?? '',
+ 'UserAgent' => $_SERVER['HTTP_USER_AGENT'] ?? '',
+ // Headers application can inject via LBFactory::setRequestInfo()
+ 'ChronologyProtection' => null,
+ 'ChronologyClientId' => null, // prior $cpClientId value from LBFactory::shutdown()
+ 'ChronologyPositionIndex' => null // prior $cpIndex value from LBFactory::shutdown()
+ ];
+ }
+
+ /**
+ * @param array $conf
+ *
+ * @return void
+ */
+ protected function configure( array $conf ): void {
$this->localDomain = isset( $conf['localDomain'] )
? DatabaseDomain::newFromId( $conf['localDomain'] )
: DatabaseDomain::newUnspecified();
@@ -143,13 +175,13 @@ abstract class LBFactory implements ILBFactory {
$this->wanCache = $conf['wanCache'] ?? WANObjectCache::newEmpty();
foreach ( self::$loggerFields as $key ) {
- $this->$key = $conf[$key] ?? new NullLogger();
+ $this->$key = $conf[ $key ] ?? new NullLogger();
}
$this->errorLogger = $conf['errorLogger'] ?? static function ( Throwable $e ) {
- trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
+ trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
};
$this->deprecationLogger = $conf['deprecationLogger'] ?? static function ( $msg ) {
- trigger_error( $msg, E_USER_DEPRECATED );
+ trigger_error( $msg, E_USER_DEPRECATED );
};
$this->profiler = $conf['profiler'] ?? null;
@@ -158,15 +190,6 @@ abstract class LBFactory implements ILBFactory {
$this->csProvider = $conf['criticalSectionProvider'] ?? null;
- $this->requestInfo = [
- 'IPAddress' => $_SERVER[ 'REMOTE_ADDR' ] ?? '',
- 'UserAgent' => $_SERVER['HTTP_USER_AGENT'] ?? '',
- // Headers application can inject via LBFactory::setRequestInfo()
- 'ChronologyProtection' => null,
- 'ChronologyClientId' => null, // prior $cpClientId value from LBFactory::shutdown()
- 'ChronologyPositionIndex' => null // prior $cpIndex value from LBFactory::shutdown()
- ];
-
$this->cliMode = $conf['cliMode'] ?? ( PHP_SAPI === 'cli' || PHP_SAPI === 'phpdbg' );
$this->agent = $conf['agent'] ?? '';
$this->defaultGroup = $conf['defaultGroup'] ?? null;
@@ -175,6 +198,8 @@ abstract class LBFactory implements ILBFactory {
static $nextTicket;
$this->ticket = $nextTicket = ( is_int( $nextTicket ) ? $nextTicket++ : mt_rand() );
+
+ $this->currentConfig = $conf;
}
public function destroy() {
@@ -186,6 +211,72 @@ abstract class LBFactory implements ILBFactory {
}
}
+ /**
+ * Reload config using the callback passed defined $config['configCallback'].
+ *
+ * If the config returned by the callback is different from the existing config,
+ * this calls reconfigure() on all load balancers, which causes them to invalidate
+ * any existing connections and re-connect using the new configuration.
+ *
+ * @note This invalidates the current transaction ticket.
+ *
+ * @warning This must only be called in top level code such as the execute()
+ * method of a maintenance script. Any database connection in use when this
+ * method is called will become defunct.
+ *
+ * @return bool true if the config changed.
+ */
+ public function autoReconfigure(): bool {
+ if ( !$this->configCallback ) {
+ return false;
+ }
+
+ $conf = ( $this->configCallback )();
+ if ( !$conf ) {
+ return false;
+ }
+
+ return $this->reconfigure( $conf );
+ }
+
+ /**
+ * Reconfigure using the given config array.
+ * Any fields omitted from $conf will be taken from the current config.
+ *
+ * If the config changed, this calls reconfigure() on all load balancers,
+ * which causes them to close all existing connections.
+ *
+ * @note This invalidates the current transaction ticket.
+ *
+ * @warning This must only be called in top level code such as the execute()
+ * method of a maintenance script. Any database connection in use when this
+ * method is called will become defunct.
+ *
+ * @since 1.39
+ *
+ * @param array $conf A configuration array, using the same structure as
+ * the one passed to the constructor (see also $wgLBFactoryConf).
+ *
+ * @return bool true if the config changed.
+ */
+ public function reconfigure( array $conf ): bool {
+ if ( !$conf ) {
+ return false;
+ }
+ if ( $conf['servers'] == $this->currentConfig['servers'] ) {
+ return false;
+ }
+
+ $this->connLogger->notice( 'Reconfiguring LBFactory!' );
+ $this->configure( $conf );
+
+ foreach ( $this->getLBsForOwner() as $lb ) {
+ $lb->reconfigure( $conf );
+ }
+
+ return true;
+ }
+
public function getLocalDomainID() {
return $this->localDomain->getId();
}
diff --git a/includes/libs/rdbms/lbfactory/LBFactoryMulti.php b/includes/libs/rdbms/lbfactory/LBFactoryMulti.php
index 9b475bcdde15..99486c6e284e 100644
--- a/includes/libs/rdbms/lbfactory/LBFactoryMulti.php
+++ b/includes/libs/rdbms/lbfactory/LBFactoryMulti.php
@@ -106,6 +106,9 @@ class LBFactoryMulti extends LBFactory {
* - readOnlyBySection: map of (main section => message text or false).
* String values make sections read only, whereas anything else does not
* restrict read/write mode. [optional]
+ * - configCallback: A callback that returns a conf array that can be passed to
+ * the reconfigure() method. This will be used to autoReconfigure() to load
+ * any updated configuration.
*/
public function __construct( array $conf ) {
parent::__construct( $conf );
diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancerForOwner.php b/includes/libs/rdbms/loadbalancer/ILoadBalancerForOwner.php
index d99e1de8654a..2f9006dcbeb3 100644
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancerForOwner.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancerForOwner.php
@@ -232,4 +232,18 @@ interface ILoadBalancerForOwner extends ILoadBalancer {
* @since 1.33
*/
public function setLocalDomainPrefix( $prefix );
+
+ /**
+ * Reconfigure using the given config array.
+ * If the config changed, this invalidates all existing connections.
+ *
+ * @warning This must only be called in top level code, typically via
+ * LBFactory::reconfigure.
+ *
+ * @since 1.39
+ *
+ * @param array $conf A configuration array, using the same structure as
+ * the one passed to the LoadBalancer's constructor.
+ */
+ public function reconfigure( array $conf );
}
diff --git a/includes/libs/rdbms/loadbalancer/LoadBalancer.php b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
index 67277db1e473..a2c9ce250b3d 100644
--- a/includes/libs/rdbms/loadbalancer/LoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/LoadBalancer.php
@@ -138,6 +138,12 @@ class LoadBalancer implements ILoadBalancerForOwner {
/** @var DatabaseDomain[] Map of (domain ID => domain instance) */
private $nonLocalDomainCache = [];
+ /**
+ * @var int Modification counter for invalidating connections held by
+ * DBConnRef instances. This is bumped by reconfigure().
+ */
+ private $modcount = 0;
+
private const INFO_SERVER_INDEX = 'serverIndex';
private const INFO_AUTOCOMMIT_ONLY = 'autoCommitOnly';
private const INFO_FORIEGN = 'foreign';
@@ -184,6 +190,17 @@ class LoadBalancer implements ILoadBalancerForOwner {
private const READER_INDEX_NONE = -1;
public function __construct( array $params ) {
+ $this->configure( $params );
+
+ $this->conns = self::newTrackedConnectionsArray();
+ }
+
+ /**
+ * @param array $params A database configuration array, see $wgLBFactoryConf.
+ *
+ * @return void
+ */
+ protected function configure( array $params ): void {
if ( !isset( $params['servers'] ) || !count( $params['servers'] ) ) {
throw new InvalidArgumentException( 'Missing or empty "servers" parameter' );
}
@@ -202,17 +219,15 @@ class LoadBalancer implements ILoadBalancerForOwner {
if ( ++$listKey !== $i ) {
throw new UnexpectedValueException( 'List expected for "servers" parameter' );
}
- $this->servers[$i] = $server;
+ $this->servers[ $i ] = $server;
foreach ( ( $server['groupLoads'] ?? [] ) as $group => $ratio ) {
- $this->groupLoads[$group][$i] = $ratio;
+ $this->groupLoads[ $group ][ $i ] = $ratio;
}
- $this->groupLoads[self::GROUP_GENERIC][$i] = $server['load'];
+ $this->groupLoads[ self::GROUP_GENERIC ][ $i ] = $server['load'];
}
$this->waitTimeout = $params['waitTimeout'] ?? self::MAX_WAIT_DEFAULT;
- $this->conns = self::newTrackedConnectionsArray();
-
if ( isset( $params['readOnlyReason'] ) && is_string( $params['readOnlyReason'] ) ) {
$this->readOnlyReason = $params['readOnlyReason'];
}
@@ -223,10 +238,10 @@ class LoadBalancer implements ILoadBalancerForOwner {
$this->srvCache = $params['srvCache'] ?? new EmptyBagOStuff();
$this->wanCache = $params['wanCache'] ?? WANObjectCache::newEmpty();
$this->errorLogger = $params['errorLogger'] ?? static function ( Throwable $e ) {
- trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
+ trigger_error( get_class( $e ) . ': ' . $e->getMessage(), E_USER_WARNING );
};
$this->deprecationLogger = $params['deprecationLogger'] ?? static function ( $msg ) {
- trigger_error( $msg, E_USER_DEPRECATED );
+ trigger_error( $msg, E_USER_DEPRECATED );
};
$this->replLogger = $params['replLogger'] ?? new NullLogger();
$this->connLogger = $params['connLogger'] ?? new NullLogger();
@@ -256,7 +271,7 @@ class LoadBalancer implements ILoadBalancerForOwner {
}
$group = $params['defaultGroup'] ?? self::GROUP_GENERIC;
- $this->defaultGroup = isset( $this->groupLoads[$group] ) ? $group : self::GROUP_GENERIC;
+ $this->defaultGroup = isset( $this->groupLoads[ $group ] ) ? $group : self::GROUP_GENERIC;
}
private static function newTrackedConnectionsArray() {
@@ -547,7 +562,7 @@ class LoadBalancer implements ILoadBalancerForOwner {
// Pick a server to use, accounting for weights, load, lag, and "waitForPos"
$this->lazyLoadReplicationPositions(); // optimizes server candidate selection
- list( $i, $laggedReplicaMode ) = $this->pickReaderIndex( $loads, $domain );
+ [ $i, $laggedReplicaMode ] = $this->pickReaderIndex( $loads, $domain );
if ( $i === false ) {
// DB connection unsuccessful
return false;
@@ -1073,7 +1088,7 @@ class LoadBalancer implements ILoadBalancerForOwner {
$domain = $this->resolveDomainID( $domain );
$role = $this->getRoleFromIndex( $i );
- return new DBConnRef( $this, [ $i, $groups, $domain, $flags ], $role );
+ return new DBConnRef( $this, [ $i, $groups, $domain, $flags ], $role, $this->modcount );
}
public function getLazyConnectionRef( $i, $groups = [], $domain = false, $flags = 0 ): IDatabase {
@@ -1095,9 +1110,8 @@ class LoadBalancer implements ILoadBalancerForOwner {
$domain = $this->resolveDomainID( $domain );
$role = $this->getRoleFromIndex( $i );
- $conn = $this->getConnectionInternal( $i, $groups, $domain, $flags );
- return new DBConnRef( $this, $conn, $role );
+ return new DBConnRef( $this, [ $i, $groups, $domain, $flags ], $role, $this->modcount );
}
/**
@@ -1593,6 +1607,37 @@ class LoadBalancer implements ILoadBalancerForOwner {
return $highestPos;
}
+ /**
+ * Apply updated configuration.
+ *
+ * This invalidates any open connections. However, existing connections may continue to be
+ * used while they are in an active transaction. In that case, the old connection will be
+ * discarded on the first operation after the transaction is complete. The next operation
+ * will use a new connection based on the new configuration.
+ *
+ * @internal for use by LBFactory::reconfigure()
+ *
+ * @see DBConnRef::ensureConnection()
+ * @see LBFactory::reconfigure()
+ *
+ * @param array $params A database configuration array, see $wgLBFactoryConf.
+ *
+ * @return void
+ */
+ public function reconfigure( array $params ) {
+ // NOTE: We could close all connection here, but some may be in the middle of
+ // a transaction. So instead, we leave it to DBConnRef to close the
+ // connection when it detects that the modcount has changed and no
+ // transaction is open.
+
+ $this->configure( $params );
+
+ // Bump modification counter to invalidate the connections held by DBConnRef
+ // instances. This will cause the next call to a method on the DBConnRef
+ // to get a new connection from getConnectionInternal()
+ $this->modcount++;
+ }
+
public function disable( $fname = __METHOD__ ) {
$this->closeAll( $fname );
$this->disabled = true;
@@ -2482,6 +2527,7 @@ class LoadBalancer implements ILoadBalancerForOwner {
$extras
);
}
+
}
/**
diff --git a/maintenance/includes/Maintenance.php b/maintenance/includes/Maintenance.php
index dc15a1881b4f..8349e8530457 100644
--- a/maintenance/includes/Maintenance.php
+++ b/maintenance/includes/Maintenance.php
@@ -1147,7 +1147,9 @@ abstract class Maintenance {
}
/**
- * Wait for replica DBs to catch up
+ * Wait for replica DBs to catch up.
+ *
+ * @note Since 1.39, this also calls LBFactory::autoReconfigure().
*
* @return bool Whether the replica DB wait succeeded
* @since 1.36
@@ -1158,6 +1160,14 @@ abstract class Maintenance {
[ 'timeout' => 30, 'ifWritesSince' => $this->lastReplicationWait ]
);
$this->lastReplicationWait = microtime( true );
+
+ // If possible, apply changes to the database configuration.
+ // The primary use case for this is taking replicas out of rotation.
+ // Long-running scripts may otherwise keep connections to
+ // de-pooled database hosts, and may even re-connect to them.
+ // If no config callback was configured, this has no effect.
+ $lbFactory->autoReconfigure();
+
return $waitSucceeded;
}
diff --git a/tests/phpunit/includes/db/LBFactoryTest.php b/tests/phpunit/includes/db/LBFactoryTest.php
index 68d8e3eb8212..1ab5d9cd339a 100644
--- a/tests/phpunit/includes/db/LBFactoryTest.php
+++ b/tests/phpunit/includes/db/LBFactoryTest.php
@@ -43,26 +43,27 @@ use Wikimedia\TestingAccessWrapper;
* @covers \Wikimedia\Rdbms\LBFactoryMulti
*/
class LBFactoryTest extends MediaWikiIntegrationTestCase {
+
+ private function getPrimaryServerConfig() {
+ global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword, $wgDBtype, $wgSQLiteDataDir;
+ return [
+ 'host' => $wgDBserver,
+ 'dbname' => $wgDBname,
+ 'user' => $wgDBuser,
+ 'password' => $wgDBpassword,
+ 'type' => $wgDBtype,
+ 'dbDirectory' => $wgSQLiteDataDir,
+ 'load' => 0,
+ 'flags' => DBO_TRX // REPEATABLE-READ for consistency
+ ];
+ }
+
/**
* @covers \Wikimedia\Rdbms\LBFactory::getLocalDomainID()
* @covers \Wikimedia\Rdbms\LBFactory::resolveDomainID()
*/
public function testLBFactorySimpleServer() {
- global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword, $wgDBtype, $wgSQLiteDataDir;
-
- $servers = [
- [
- 'host' => $wgDBserver,
- 'dbname' => $wgDBname,
- 'user' => $wgDBuser,
- 'password' => $wgDBpassword,
- 'type' => $wgDBtype,
- 'dbDirectory' => $wgSQLiteDataDir,
- 'load' => 0,
- 'flags' => DBO_TRX // REPEATABLE-READ for consistency
- ],
- ];
-
+ $servers = [ $this->getPrimaryServerConfig() ];
$factory = new LBFactorySimple( [ 'servers' => $servers ] );
$lb = $factory->getMainLB();
@@ -81,29 +82,14 @@ class LBFactoryTest extends MediaWikiIntegrationTestCase {
}
public function testLBFactorySimpleServers() {
- global $wgDBserver, $wgDBname, $wgDBuser, $wgDBpassword, $wgDBtype, $wgSQLiteDataDir;
+ global $wgDBserver;
+
+ $primaryConfig = $this->getPrimaryServerConfig();
+ $fakeReplica = [ 'load' => 100, ] + $primaryConfig;
$servers = [
- [ // master
- 'host' => $wgDBserver,
- 'dbname' => $wgDBname,
- 'user' => $wgDBuser,
- 'password' => $wgDBpassword,
- 'type' => $wgDBtype,
- 'dbDirectory' => $wgSQLiteDataDir,
- 'load' => 0,
- 'flags' => DBO_TRX // REPEATABLE-READ for consistency
- ],
- [ // emulated replica
- 'host' => $wgDBserver,
- 'dbname' => $wgDBname,
- 'user' => $wgDBuser,
- 'password' => $wgDBpassword,
- 'type' => $wgDBtype,
- 'dbDirectory' => $wgSQLiteDataDir,
- 'load' => 100,
- 'flags' => DBO_TRX // REPEATABLE-READ for consistency
- ]
+ $primaryConfig,
+ $fakeReplica
];
$factory = new LBFactorySimple( [
@@ -813,4 +799,111 @@ class LBFactoryTest extends MediaWikiIntegrationTestCase {
$touched = $lbFactory->getChronologyProtectorTouched();
$this->assertEquals( $priorTime, $touched );
}
+
+ public function testReconfigure() {
+ $primaryConfig = $this->getPrimaryServerConfig();
+ $fakeReplica = [ 'load' => 100, ] + $primaryConfig;
+
+ $conf = [ 'servers' => [
+ $primaryConfig,
+ $fakeReplica
+ ] ];
+
+ // Configure an LBFactory with one replica
+ $factory = new LBFactorySimple( $conf );
+ $lb = $factory->getMainLB();
+ $this->assertSame( 2, $lb->getServerCount() );
+
+ $con = $lb->getConnectionInternal( DB_REPLICA );
+ $ref = $lb->getConnection( DB_REPLICA );
+
+ // Call reconfigure with the same config, should have no effect
+ $changed = $factory->reconfigure( $conf );
+ $this->assertFalse( $changed );
+ $this->assertSame( 2, $lb->getServerCount() );
+ $this->assertTrue( $con->isOpen() );
+ $this->assertTrue( $ref->isOpen() );
+
+ // Call reconfigure with empty config, should have no effect
+ $changed = $factory->reconfigure( [] );
+ $this->assertFalse( $changed );
+ $this->assertSame( 2, $lb->getServerCount() );
+ $this->assertTrue( $con->isOpen() );
+ $this->assertTrue( $ref->isOpen() );
+
+ // Reconfigure the LBFactory to only have a single server.
+ $conf['servers'] = [ $this->getPrimaryServerConfig() ];
+ $changed = $factory->reconfigure( $conf );
+ $this->assertTrue( $changed );
+
+ // The LoadBalancer should have been reconfigured automatically.
+ $this->assertSame( 1, $lb->getServerCount() );
+
+ // Reconfiguring should not close connections immediately.
+ $this->assertTrue( $con->isOpen() );
+
+ // Connection refs should detect the config change, close the old connection,
+ // and get a new connection.
+ $this->assertTrue( $ref->isOpen() );
+ $this->assertSame( IDatabase::ROLE_STREAMING_MASTER, $ref->getTopologyRole() );
+
+ // The old connection should have been called by DBConnRef.
+ $this->assertFalse( $con->isOpen() );
+ }
+
+ public function testAutoReconfigure() {
+ $primaryConfig = $this->getPrimaryServerConfig();
+ $fakeReplica = [ 'load' => 100, ] + $primaryConfig;
+
+ $conf = [
+ 'servers' => [
+ $primaryConfig,
+ $fakeReplica
+ ],
+ ];
+
+ // The config callback should return $conf, reflecting changes
+ // made to the local variable.
+ $conf['configCallback'] = static function () use ( &$conf ) {
+ return $conf;
+ };
+
+ // Configure an LBFactory with one replica
+ $factory = new LBFactorySimple( $conf );
+
+ $lb = $factory->getMainLB();
+ $this->assertSame( 2, $lb->getServerCount() );
+
+ $con = $lb->getConnectionInternal( DB_REPLICA );
+ $ref = $lb->getConnection( DB_REPLICA );
+
+ // Nothing changed, autoReconfigure() should do nothing.
+ $changed = $factory->autoReconfigure();
+ $this->assertFalse( $changed );
+
+ $this->assertSame( 2, $lb->getServerCount() );
+ $this->assertTrue( $con->isOpen() );
+ $this->assertTrue( $ref->isOpen() );
+
+ // Change config to only have a single server.
+ $conf['servers'] = [ $this->getPrimaryServerConfig() ];
+
+ // Now autoReconfigure() should detect the change and reconfigure all LoadBalancers.
+ $changed = $factory->autoReconfigure();
+ $this->assertTrue( $changed );
+
+ // The LoadBalancer should have been reconfigured now.
+ $this->assertSame( 1, $lb->getServerCount() );
+
+ // Reconfiguring should not close connections immediately.
+ $this->assertTrue( $con->isOpen() );
+
+ // Connection refs should detect the config change, close the old connection,
+ // and get a new connection.
+ $this->assertTrue( $ref->isOpen() );
+ $this->assertSame( IDatabase::ROLE_STREAMING_MASTER, $ref->getTopologyRole() );
+
+ // The old connection should have been called by DBConnRef.
+ $this->assertFalse( $con->isOpen() );
+ }
}
diff --git a/tests/phpunit/includes/db/LoadBalancerTest.php b/tests/phpunit/includes/db/LoadBalancerTest.php
index 8a3e27eace21..4036f14d41be 100644
--- a/tests/phpunit/includes/db/LoadBalancerTest.php
+++ b/tests/phpunit/includes/db/LoadBalancerTest.php
@@ -477,6 +477,38 @@ class LoadBalancerTest extends MediaWikiIntegrationTestCase {
$lb->closeAll( __METHOD__ );
}
+ public function testReconfigure() {
+ $conf = [
+ 'servers' => [ $this->makeServerConfig() ],
+ 'clusterName' => 'A',
+ 'localDomain' => $this->db->getDomainID()
+ ];
+
+ $lb = new LoadBalancer( $conf );
+ $this->assertSame( 'A', $lb->getClusterName() );
+
+ $con = $lb->getConnectionInternal( DB_PRIMARY );
+ $ref = $lb->getConnection( DB_PRIMARY );
+
+ $this->assertTrue( $con->isOpen() );
+ $this->assertTrue( $ref->isOpen() );
+
+ // Reconfigure the LoadBalancer
+ $conf['clusterName'] = 'X';
+ $lb->reconfigure( $conf );
+ $this->assertSame( 'X', $lb->getClusterName() );
+
+ // Reconfiguring should not close connections immediately.
+ $this->assertTrue( $con->isOpen() );
+
+ // Connection refs should detect the config change, close the old connection,
+ // and get a new connection.
+ $this->assertTrue( $ref->isOpen() );
+
+ // The old connection should have been called by DBConnRef.
+ $this->assertFalse( $con->isOpen() );
+ }
+
/**
* @covers \Wikimedia\Rdbms\LoadBalancer::getWriterIndex()
* @covers \Wikimedia\Rdbms\LoadBalancer::getOpenPrimaryConnections()
diff --git a/tests/phpunit/unit/includes/db/MWLBFactoryTest.php b/tests/phpunit/unit/includes/db/MWLBFactoryTest.php
index 071dd1bfe910..dd5a04b8b272 100644
--- a/tests/phpunit/unit/includes/db/MWLBFactoryTest.php
+++ b/tests/phpunit/unit/includes/db/MWLBFactoryTest.php
@@ -18,8 +18,11 @@
* @file
*/
+use MediaWiki\Config\ServiceOptions;
use Wikimedia\Rdbms\DatabaseDomain;
use Wikimedia\Rdbms\LBFactorySimple;
+use Wikimedia\RequestTimeout\CriticalSectionProvider;
+use Wikimedia\RequestTimeout\RequestTimeout;
/**
* @covers \Wikimedia\Rdbms\LBFactory
@@ -27,6 +30,19 @@ use Wikimedia\Rdbms\LBFactorySimple;
* @covers \Wikimedia\Rdbms\LBFactoryMulti
*/
class MWLBFactoryTest extends MediaWikiUnitTestCase {
+
+ private function newMWLBFactory() {
+ return new MWLBFactory(
+ new ServiceOptions( [], [] ),
+ new ConfiguredReadOnlyMode( 'Test' ),
+ new EmptyBagOStuff(),
+ new EmptyBagOStuff(),
+ new WANObjectCache( [ 'cache' => new EmptyBagOStuff() ] ),
+ new CriticalSectionProvider( RequestTimeout::singleton(), 1, null, null ),
+ new NullStatsdDataFactory()
+ );
+ }
+
/**
* @covers MWLBFactory::getLBFactoryClass
* @dataProvider getLBFactoryClassProvider
@@ -34,7 +50,7 @@ class MWLBFactoryTest extends MediaWikiUnitTestCase {
public function testGetLBFactoryClass( $config, $expected ) {
$this->assertEquals(
$expected,
- MWLBFactory::getLBFactoryClass( $config )
+ $this->newMWLBFactory()->getLBFactoryClass( $config )
);
}
@@ -65,7 +81,7 @@ class MWLBFactoryTest extends MediaWikiUnitTestCase {
'servers' => $servers,
'localDomain' => new DatabaseDomain( $dbname, null, $prefix )
] );
- MWLBFactory::setDomainAliases( $lbFactory );
+ $this->newMWLBFactory()->setDomainAliases( $lbFactory );
$rawDomain = rtrim( "$dbname-$prefix", '-' );
$this->assertEquals(
diff --git a/tests/phpunit/unit/includes/libs/rdbms/database/DBConnRefTest.php b/tests/phpunit/unit/includes/libs/rdbms/database/DBConnRefTest.php
index 96d3311aaf78..427ae659a79c 100644
--- a/tests/phpunit/unit/includes/libs/rdbms/database/DBConnRefTest.php
+++ b/tests/phpunit/unit/includes/libs/rdbms/database/DBConnRefTest.php
@@ -1,5 +1,6 @@
<?php
+use PHPUnit\Framework\MockObject\MockObject;
use Wikimedia\Rdbms\DBConnRef;
use Wikimedia\Rdbms\FakeResultWrapper;
use Wikimedia\Rdbms\IDatabase;
@@ -14,12 +15,23 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
use MediaWikiCoversValidator;
/**
- * @return ILoadBalancer
+ * @return ILoadBalancer|MockObject
*/
private function getLoadBalancerMock() {
+ // getConnection() and getConnectionInternal() should keep returning the same connection
+ // on every call, unless that connection was closed. Then they should return a new
+ // connection.
+ $conn = $this->getDatabaseMock();
+ $getDatabaseMock = function () use ( &$conn ) {
+ if ( !$conn->isOpen() ) {
+ $conn = $this->getDatabaseMock();
+ }
+ return $conn;
+ };
+
$lb = $this->createMock( ILoadBalancer::class );
- $lb->method( 'getConnection' )->willReturn( $this->getDatabaseMock() );
- $lb->method( 'getConnectionInternal' )->willReturn( $this->getDatabaseMock() );
+ $lb->method( 'getConnection' )->willReturnCallback( $getDatabaseMock );
+ $lb->method( 'getConnectionInternal' )->willReturnCallback( $getDatabaseMock );
$lb->method( 'getConnectionRef' )->willReturnCallback(
function () use ( $lb ) {
@@ -65,6 +77,30 @@ class DBConnRefTest extends PHPUnit\Framework\TestCase {
return new DBConnRef( $lb, $this->getDatabaseMock(), DB_PRIMARY );
}
+ /**
+ * Test that bumping the modification counter causes the wrapped connection
+ * to be discarded and re-aquired.
+ */
+ public function testModCount() {
+ $lb = $this->getLoadBalancerMock();
+ $lb->expects( $this->exactly( 3 ) )->method( 'getConnectionInternal' );
+
+ $params = [ DB_PRIMARY, [], 'mywiki', 0 ];
+ $modcount = 0;
+ $ref = new DBConnRef( $lb, $params, DB_PRIMARY, $modcount );
+
+ $ref->select( 'test', '*' );
+ $ref->select( 'test', '*' );
+
+ $modcount++; // cause second call to getConnectionInternal
+ $ref->select( 'test', '*' );
+ $ref->select( 'test', '*' );
+
+ $modcount++; // cause third call to getConnectionInternal
+ $ref->select( 'test', '*' );
+ $ref->select( 'test', '*' );
+ }
+
public function testConstruct() {
$lb = $this->getLoadBalancerMock();
$ref = new DBConnRef( $lb, $this->getDatabaseMock(), DB_PRIMARY );