Aaron Schulz has uploaded a new change for review.
https://gerrit.wikimedia.org/r/309913
Change subject: Various depedency injection cleanups to LoadBalancer
......................................................................
Various depedency injection cleanups to LoadBalancer
* Inject wfWikiID() and MWExceptionHandler into LoadBalancer.
* Factor out LBFactory duplication into baseLoadBalancerParams().
* Remove $wgDBtype hack. Presumably, sites with others DBs would
not have multiple servers configured if does not work anyway.
* Make use of injected TransactionProfiler rather than calling
Profiler::instance()->getTransactionProfiler().
* Avoid use of trivial wfSplitWikiID() function.
* Make DBConnRef enforce its arguments more strongly and
optimize getWiki() to avoid causing a connection attempt.
* Avoid deprecated method call in LBFactory::destroyInstance().
Change-Id: If134b62d4f48cd68cb48ccbe149e72f12aa26819
---
M includes/db/DBConnRef.php
M includes/db/loadbalancer/LBFactory.php
M includes/db/loadbalancer/LBFactoryMulti.php
M includes/db/loadbalancer/LBFactorySimple.php
M includes/db/loadbalancer/LBFactorySingle.php
M includes/db/loadbalancer/LoadBalancer.php
6 files changed, 68 insertions(+), 43 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/13/309913/1
diff --git a/includes/db/DBConnRef.php b/includes/db/DBConnRef.php
index 9997f2c..e826c18 100644
--- a/includes/db/DBConnRef.php
+++ b/includes/db/DBConnRef.php
@@ -17,16 +17,22 @@
/** @var array|null */
private $params;
+ const FLD_INDEX = 0;
+ const FLD_GROUP = 1;
+ const FLD_WIKI = 2;
+
/**
* @param LoadBalancer $lb
- * @param DatabaseBase|array $conn Connection or (server index, group,
wiki ID) array
+ * @param DatabaseBase|array $conn Connection or (server index, group,
wiki ID)
*/
public function __construct( LoadBalancer $lb, $conn ) {
$this->lb = $lb;
if ( $conn instanceof DatabaseBase ) {
$this->conn = $conn;
- } else {
+ } elseif ( count( $conn ) >= 3 && $conn[self::FLD_WIKI] != '' )
{
$this->params = $conn;
+ } else {
+ throw new InvalidArgumentException( "Missing lazy
connection arguments." );
}
}
@@ -136,6 +142,11 @@
}
public function getWikiID() {
+ if ( $this->conn === null ) {
+ // Avoid triggering a connection
+ return $this->params[self::FLD_WIKI];
+ }
+
return $this->__call( __FUNCTION__, func_get_args() );
}
diff --git a/includes/db/loadbalancer/LBFactory.php
b/includes/db/loadbalancer/LBFactory.php
index 62a5286..21e1c64 100644
--- a/includes/db/loadbalancer/LBFactory.php
+++ b/includes/db/loadbalancer/LBFactory.php
@@ -147,7 +147,7 @@
* @deprecated since 1.27, use LBFactory::destroy()
*/
public static function destroyInstance() {
- self::singleton()->destroy();
+
MediaWikiServices::getInstance()->getDBLoadBalancerFactory()->destroy();
}
/**
@@ -612,6 +612,20 @@
}
/**
+ * Base parameters to LoadBalancer::__construct()
+ */
+ final protected function baseLoadBalancerParams() {
+ return [
+ 'readOnlyReason' => $this->readOnlyReason,
+ 'trxProfiler' => $this->trxProfiler,
+ 'srvCache' => $this->srvCache,
+ 'wanCache' => $this->wanCache,
+ 'localDomain' => wfWikiID(),
+ 'errorLogger' => [ MWExceptionHandler::class,
'logException' ]
+ ];
+ }
+
+ /**
* @param LoadBalancer $lb
*/
protected function initLoadBalancer( LoadBalancer $lb ) {
diff --git a/includes/db/loadbalancer/LBFactoryMulti.php
b/includes/db/loadbalancer/LBFactoryMulti.php
index e56631d..48cd09e 100644
--- a/includes/db/loadbalancer/LBFactoryMulti.php
+++ b/includes/db/loadbalancer/LBFactoryMulti.php
@@ -313,15 +313,14 @@
* @return LoadBalancer
*/
private function newLoadBalancer( $template, $loads, $groupLoads,
$readOnlyReason ) {
- $lb = new LoadBalancer( [
- 'servers' => $this->makeServerArray( $template, $loads,
$groupLoads ),
- 'loadMonitor' => $this->loadMonitorClass,
- 'readOnlyReason' => $readOnlyReason,
- 'trxProfiler' => $this->trxProfiler,
- 'srvCache' => $this->srvCache,
- 'wanCache' => $this->wanCache
- ] );
-
+ $lb = new LoadBalancer( array_merge(
+ $this->baseLoadBalancerParams(),
+ [
+ 'servers' => $this->makeServerArray( $template,
$loads, $groupLoads ),
+ 'loadMonitor' => $this->loadMonitorClass,
+ 'readOnlyReason' => $readOnlyReason
+ ]
+ ) );
$this->initLoadBalancer( $lb );
return $lb;
diff --git a/includes/db/loadbalancer/LBFactorySimple.php
b/includes/db/loadbalancer/LBFactorySimple.php
index 4632b0a..5eb71d9 100644
--- a/includes/db/loadbalancer/LBFactorySimple.php
+++ b/includes/db/loadbalancer/LBFactorySimple.php
@@ -133,15 +133,13 @@
}
private function newLoadBalancer( array $servers ) {
- $lb = new LoadBalancer( [
- 'servers' => $servers,
- 'loadMonitor' => $this->loadMonitorClass,
- 'readOnlyReason' => $this->readOnlyReason,
- 'trxProfiler' => $this->trxProfiler,
- 'srvCache' => $this->srvCache,
- 'wanCache' => $this->wanCache
- ] );
-
+ $lb = new LoadBalancer( array_merge(
+ $this->baseLoadBalancerParams(),
+ [
+ 'servers' => $servers,
+ 'loadMonitor' => $this->loadMonitorClass,
+ ]
+ ) );
$this->initLoadBalancer( $lb );
return $lb;
diff --git a/includes/db/loadbalancer/LBFactorySingle.php
b/includes/db/loadbalancer/LBFactorySingle.php
index 14cec0e..7c26483 100644
--- a/includes/db/loadbalancer/LBFactorySingle.php
+++ b/includes/db/loadbalancer/LBFactorySingle.php
@@ -35,12 +35,7 @@
public function __construct( array $conf ) {
parent::__construct( $conf );
- $this->lb = new LoadBalancerSingle( [
- 'readOnlyReason' => $this->readOnlyReason,
- 'trxProfiler' => $this->trxProfiler,
- 'srvCache' => $this->srvCache,
- 'wanCache' => $this->wanCache
- ] + $conf );
+ $this->lb = new LoadBalancerSingle( $conf +
$this->baseLoadBalancerParams() );
}
/**
diff --git a/includes/db/loadbalancer/LoadBalancer.php
b/includes/db/loadbalancer/LoadBalancer.php
index 17b1728..18c323e 100644
--- a/includes/db/loadbalancer/LoadBalancer.php
+++ b/includes/db/loadbalancer/LoadBalancer.php
@@ -74,6 +74,10 @@
private $trxRoundId = false;
/** @var array[] Map of (name => callable) */
private $trxRecurringCallbacks = [];
+ /** @var string Local Wiki ID and default for selectDB() calls */
+ private $localDomain;
+ /** @var callable Exception logger */
+ private $errorLogger;
/** @var integer Warn when this many connection are held */
const CONN_HELD_WARN_THRESHOLD = 10;
@@ -97,6 +101,8 @@
* - waitTimeout : Maximum time to wait for replicas for consistency
[optional]
* - srvCache : BagOStuff object [optional]
* - wanCache : WANObjectCache object [optional]
+ * - localDomain: The wiki ID of the "local"/"current" wiki [optional]
+ * - errorLogger: Callback that takes an Exception and logs it
[optional]
* @throws MWException
*/
public function __construct( array $params ) {
@@ -107,6 +113,7 @@
$this->mWaitTimeout = isset( $params['waitTimeout'] )
? $params['waitTimeout']
: self::POS_WAIT_TIMEOUT;
+ $this->localDomain = isset( $params['localDomain'] ) ?
$params['localDomain'] : '';
$this->mReadIndex = -1;
$this->mWriteIndex = -1;
@@ -161,6 +168,12 @@
} else {
$this->trxProfiler = new TransactionProfiler();
}
+
+ $this->errorLogger = isset( $params['errorLogger'] )
+ ? $params['errorLogger']
+ : function ( Exception $e ) {
+ trigger_error( E_WARNING, $e->getMessage() );
+ };
}
/**
@@ -249,16 +262,9 @@
* @return bool|int|string
*/
public function getReaderIndex( $group = false, $wiki = false ) {
- global $wgDBtype;
-
- # @todo FIXME: For now, only go through all this for mysql
databases
- if ( $wgDBtype != 'mysql' ) {
- return $this->getWriterIndex();
- }
-
if ( count( $this->mServers ) == 1 ) {
# Skip the load balancing if there's only one server
- return 0;
+ return $this->getWriterIndex();
} elseif ( $group === false && $this->mReadIndex >= 0 ) {
# Shortcut if generic reader exists already
return $this->mReadIndex;
@@ -282,7 +288,7 @@
throw new MWException( "Empty server array given to
LoadBalancer" );
}
- # Scale the configured load ratios according to the dynamic
load (if the load monitor supports it)
+ # Scale the configured load ratios according to the dynamic
load if supported
$this->getLoadMonitor()->scaleLoads( $nonErrorLoads, $group,
$wiki );
$laggedReplicaMode = false;
@@ -541,7 +547,7 @@
' with invalid server index' );
}
- if ( $wiki === wfWikiID() ) {
+ if ( $wiki === $this->localDomain ) {
$wiki = false;
}
@@ -590,8 +596,7 @@
if ( $this->connsOpened > $oldConnsOpened ) {
$host = $conn->getServer();
$dbname = $conn->getDBname();
- $trxProf =
Profiler::instance()->getTransactionProfiler();
- $trxProf->recordConnection( $host, $dbname, $masterOnly
);
+ $this->trxProfiler->recordConnection( $host, $dbname,
$masterOnly );
}
if ( $masterOnly ) {
@@ -677,6 +682,8 @@
* @return DBConnRef
*/
public function getLazyConnectionRef( $db, $groups = [], $wiki = false
) {
+ $wiki = ( $wiki !== false ) ? $wiki : $this->localDomain;
+
return new DBConnRef( $this, [ $db, $groups, $wiki ] );
}
@@ -747,7 +754,8 @@
* @return DatabaseBase
*/
private function openForeignConnection( $i, $wiki ) {
- list( $dbName, $prefix ) = wfSplitWikiID( $wiki );
+ list( $dbName, $prefix ) = explode( '-', $wiki, 2 ) + [ '', ''
];
+
if ( isset( $this->mConns['foreignUsed'][$i][$wiki] ) ) {
// Reuse an already-used connection
$conn = $this->mConns['foreignUsed'][$i][$wiki];
@@ -1084,7 +1092,7 @@
try {
$conn->commit( $fname,
$conn::FLUSHING_ALL_PEERS );
} catch ( DBError $e ) {
- MWExceptionHandler::logException( $e );
+ call_user_func( $this->errorLogger, $e
);
$failures[] = "{$conn->getServer()}:
{$e->getMessage()}";
}
if ( $restore && $conn->getLBInfo( 'master' ) )
{
@@ -1184,7 +1192,7 @@
try {
$conn->flushSnapshot( $fname );
} catch ( DBError $e ) {
- MWExceptionHandler::logException( $e );
+ call_user_func( $this->errorLogger, $e
);
$failures[] = "{$conn->getServer()}:
{$e->getMessage()}";
}
$conn->setTrxEndCallbackSuppression( false );
@@ -1219,7 +1227,7 @@
$conn->flushSnapshot( $fname );
}
} catch ( DBError $e ) {
- MWExceptionHandler::logException( $e );
+ call_user_func( $this->errorLogger, $e
);
$failures[] = "{$conn->getServer()}:
{$e->getMessage()}";
}
if ( $restore ) {
--
To view, visit https://gerrit.wikimedia.org/r/309913
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: If134b62d4f48cd68cb48ccbe149e72f12aa26819
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Aaron Schulz <asch...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
