From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 23 Sep 2016 06:10:20 +0000 (-0700)
Subject: Add ILBFactory interface for completeness
X-Git-Tag: 1.31.0-rc.0~5352^2
X-Git-Url: https://git.cyclocoop.org/admin/?a=commitdiff_plain;h=44705ce9dd1023ef68bed8c09e7b98867475ac83;p=lhc%2Fweb%2Fwiklou.git

Add ILBFactory interface for completeness

Also fixed comment indentation.

Change-Id: I535d320b63477302b06fd634b3ab47eef6868ffc
---

diff --git a/autoload.php b/autoload.php
index dd4bb9d1ac..a05c09b474 100644
--- a/autoload.php
+++ b/autoload.php
@@ -584,6 +584,7 @@ $wgAutoloadLocalClasses = [
 	'IEUrlExtension' => __DIR__ . '/includes/libs/IEUrlExtension.php',
 	'IExpiringStore' => __DIR__ . '/includes/libs/objectcache/IExpiringStore.php',
 	'IJobSpecification' => __DIR__ . '/includes/jobqueue/JobSpecification.php',
+	'ILBFactory' => __DIR__ . '/includes/libs/rdbms/lbfactory/ILBFactory.php',
 	'ILoadBalancer' => __DIR__ . '/includes/libs/rdbms/loadbalancer/ILoadBalancer.php',
 	'ILoadMonitor' => __DIR__ . '/includes/libs/rdbms/loadmonitor/ILoadMonitor.php',
 	'IP' => __DIR__ . '/includes/libs/IP.php',
diff --git a/includes/libs/rdbms/lbfactory/ILBFactory.php b/includes/libs/rdbms/lbfactory/ILBFactory.php
new file mode 100644
index 0000000000..d7ca7cd1a9
--- /dev/null
+++ b/includes/libs/rdbms/lbfactory/ILBFactory.php
@@ -0,0 +1,300 @@
+<?php
+/**
+ * Generator and manager of database load balancing objects
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ * http://www.gnu.org/copyleft/gpl.html
+ *
+ * @file
+ * @ingroup Database
+ */
+
+/**
+ * An interface for generating database load balancers
+ * @ingroup Database
+ * @since 1.28
+ */
+interface ILBFactory {
+	const SHUTDOWN_NO_CHRONPROT = 0; // don't save DB positions at all
+	const SHUTDOWN_CHRONPROT_ASYNC = 1; // save DB positions, but don't wait on remote DCs
+	const SHUTDOWN_CHRONPROT_SYNC = 2; // save DB positions, waiting on all DCs
+
+	/**
+	 * Construct a manager of ILoadBalancer objects
+	 *
+	 * Sub-classes will extend the required keys in $conf with additional parameters
+	 *
+	 * @param $conf $params Array with keys:
+	 *  - localDomain: A DatabaseDomain or domain ID string.
+	 *  - readOnlyReason : Reason the master DB is read-only if so [optional]
+	 *  - srvCache : BagOStuff object for server cache [optional]
+	 *  - memCache : BagOStuff object for cluster memory cache [optional]
+	 *  - wanCache : WANObjectCache object [optional]
+	 *  - hostname : The name of the current server [optional]
+	 *  - cliMode: Whether the execution context is a CLI script. [optional]
+	 *  - profiler : Class name or instance with profileIn()/profileOut() methods. [optional]
+	 *  - trxProfiler: TransactionProfiler instance. [optional]
+	 *  - replLogger: PSR-3 logger instance. [optional]
+	 *  - connLogger: PSR-3 logger instance. [optional]
+	 *  - queryLogger: PSR-3 logger instance. [optional]
+	 *  - perfLogger: PSR-3 logger instance. [optional]
+	 *  - errorLogger : Callback that takes an Exception and logs it. [optional]
+	 * @throws InvalidArgumentException
+	 */
+	public function __construct( array $conf );
+
+	/**
+	 * Disables all load balancers. All connections are closed, and any attempt to
+	 * open a new connection will result in a DBAccessError.
+	 * @see ILoadBalancer::disable()
+	 */
+	public function destroy();
+
+	/**
+	 * Create a new load balancer object. The resulting object will be untracked,
+	 * not chronology-protected, and the caller is responsible for cleaning it up.
+	 *
+	 * This method is for only advanced usage and callers should almost always use
+	 * getMainLB() instead. This method can be useful when a table is used as a key/value
+	 * store. In that cases, one might want to query it in autocommit mode (DBO_TRX off)
+	 * but still use DBO_TRX transaction rounds on other tables.
+	 *
+	 * @param bool|string $domain Domain ID, or false for the current domain
+	 * @return ILoadBalancer
+	 */
+	public function newMainLB( $domain = false );
+
+	/**
+	 * Get a cached (tracked) load balancer object.
+	 *
+	 * @param bool|string $domain Domain ID, or false for the current domain
+	 * @return ILoadBalancer
+	 */
+	public function getMainLB( $domain = false );
+
+	/**
+	 * Create a new load balancer for external storage. The resulting object will be
+	 * untracked, not chronology-protected, and the caller is responsible for
+	 * cleaning it up.
+	 *
+	 * This method is for only advanced usage and callers should almost always use
+	 * getExternalLB() instead. This method can be useful when a table is used as a
+	 * key/value store. In that cases, one might want to query it in autocommit mode
+	 * (DBO_TRX off) but still use DBO_TRX transaction rounds on other tables.
+	 *
+	 * @param string $cluster External storage cluster, or false for core
+	 * @param bool|string $domain Domain ID, or false for the current domain
+	 * @return ILoadBalancer
+	 */
+	public function newExternalLB( $cluster, $domain = false );
+
+	/**
+	 * Get a cached (tracked) load balancer for external storage
+	 *
+	 * @param string $cluster External storage cluster, or false for core
+	 * @param bool|string $domain Domain ID, or false for the current domain
+	 * @return ILoadBalancer
+	 */
+	public function getExternalLB( $cluster, $domain = false );
+
+	/**
+	 * Execute a function for each tracked load balancer
+	 * The callback is called with the load balancer as the first parameter,
+	 * and $params passed as the subsequent parameters.
+	 *
+	 * @param callable $callback
+	 * @param array $params
+	 */
+	public function forEachLB( $callback, array $params = [] );
+
+	/**
+	 * Prepare all tracked load balancers for shutdown
+	 * @param integer $mode One of the class SHUTDOWN_* constants
+	 * @param callable|null $workCallback Work to mask ChronologyProtector writes
+	 */
+	public function shutdown(
+		$mode = self::SHUTDOWN_CHRONPROT_SYNC, callable $workCallback = null
+	);
+
+	/**
+	 * Commit all replica DB transactions so as to flush any REPEATABLE-READ or SSI snapshot
+	 *
+	 * @param string $fname Caller name
+	 */
+	public function flushReplicaSnapshots( $fname = __METHOD__ );
+
+	/**
+	 * Commit on all connections. Done for two reasons:
+	 * 1. To commit changes to the masters.
+	 * 2. To release the snapshot on all connections, master and replica DB.
+	 * @param string $fname Caller name
+	 * @param array $options Options map:
+	 *   - maxWriteDuration: abort if more than this much time was spent in write queries
+	 */
+	public function commitAll( $fname = __METHOD__, array $options = [] );
+
+	/**
+	 * Flush any master transaction snapshots and set DBO_TRX (if DBO_DEFAULT is set)
+	 *
+	 * The DBO_TRX setting will be reverted to the default in each of these methods:
+	 *   - commitMasterChanges()
+	 *   - rollbackMasterChanges()
+	 *   - commitAll()
+	 *
+	 * This allows for custom transaction rounds from any outer transaction scope.
+	 *
+	 * @param string $fname
+	 * @throws DBTransactionError
+	 */
+	public function beginMasterChanges( $fname = __METHOD__ );
+
+	/**
+	 * Commit changes on all master connections
+	 * @param string $fname Caller name
+	 * @param array $options Options map:
+	 *   - maxWriteDuration: abort if more than this much time was spent in write queries
+	 * @throws Exception
+	 */
+	public function commitMasterChanges( $fname = __METHOD__, array $options = [] );
+
+	/**
+	 * Rollback changes on all master connections
+	 * @param string $fname Caller name
+	 */
+	public function rollbackMasterChanges( $fname = __METHOD__ );
+
+	/**
+	 * Determine if any master connection has pending changes
+	 * @return bool
+	 */
+	public function hasMasterChanges();
+
+	/**
+	 * Detemine if any lagged replica DB connection was used
+	 * @return bool
+	 */
+	public function laggedReplicaUsed();
+
+	/**
+	 * Determine if any master connection has pending/written changes from this request
+	 * @param float $age How many seconds ago is "recent" [defaults to LB lag wait timeout]
+	 * @return bool
+	 */
+	public function hasOrMadeRecentMasterChanges( $age = null );
+
+	/**
+	 * Waits for the replica DBs to catch up to the current master position
+	 *
+	 * Use this when updating very large numbers of rows, as in maintenance scripts,
+	 * to avoid causing too much lag. Of course, this is a no-op if there are no replica DBs.
+	 *
+	 * By default this waits on all DB clusters actually used in this request.
+	 * This makes sense when lag being waiting on is caused by the code that does this check.
+	 * In that case, setting "ifWritesSince" can avoid the overhead of waiting for clusters
+	 * that were not changed since the last wait check. To forcefully wait on a specific cluster
+	 * for a given domain, use the 'domain' parameter. To forcefully wait on an "external" cluster,
+	 * use the "cluster" parameter.
+	 *
+	 * Never call this function after a large DB write that is *still* in a transaction.
+	 * It only makes sense to call this after the possible lag inducing changes were committed.
+	 *
+	 * @param array $opts Optional fields that include:
+	 *   - domain : wait on the load balancer DBs that handles the given domain ID
+	 *   - cluster : wait on the given external load balancer DBs
+	 *   - timeout : Max wait time. Default: ~60 seconds
+	 *   - ifWritesSince: Only wait if writes were done since this UNIX timestamp
+	 * @throws DBReplicationWaitError If a timeout or error occured waiting on a DB cluster
+	 */
+	public function waitForReplication( array $opts = [] );
+
+	/**
+	 * Add a callback to be run in every call to waitForReplication() before waiting
+	 *
+	 * Callbacks must clear any transactions that they start
+	 *
+	 * @param string $name Callback name
+	 * @param callable|null $callback Use null to unset a callback
+	 */
+	public function setWaitForReplicationListener( $name, callable $callback = null );
+
+	/**
+	 * Get a token asserting that no transaction writes are active
+	 *
+	 * @param string $fname Caller name (e.g. __METHOD__)
+	 * @return mixed A value to pass to commitAndWaitForReplication()
+	 */
+	public function getEmptyTransactionTicket( $fname );
+
+	/**
+	 * Convenience method for safely running commitMasterChanges()/waitForReplication()
+	 *
+	 * This will commit and wait unless $ticket indicates it is unsafe to do so
+	 *
+	 * @param string $fname Caller name (e.g. __METHOD__)
+	 * @param mixed $ticket Result of getEmptyTransactionTicket()
+	 * @param array $opts Options to waitForReplication()
+	 * @throws DBReplicationWaitError
+	 */
+	public function commitAndWaitForReplication( $fname, $ticket, array $opts = [] );
+
+	/**
+	 * @param string $dbName DB master name (e.g. "db1052")
+	 * @return float|bool UNIX timestamp when client last touched the DB or false if not recent
+	 */
+	public function getChronologyProtectorTouched( $dbName );
+
+	/**
+	 * Disable the ChronologyProtector for all load balancers
+	 *
+	 * This can be called at the start of special API entry points
+	 */
+	public function disableChronologyProtection();
+
+	/**
+	 * Set a new table prefix for the existing local domain ID for testing
+	 *
+	 * @param string $prefix
+	 */
+	public function setDomainPrefix( $prefix );
+
+	/**
+	 * Close all open database connections on all open load balancers.
+	 */
+	public function closeAll();
+
+	/**
+	 * @param string $agent Agent name for query profiling
+	 */
+	public function setAgentName( $agent );
+
+	/**
+	 * Append ?cpPosTime parameter to a URL for ChronologyProtector purposes if needed
+	 *
+	 * Note that unlike cookies, this works accross domains
+	 *
+	 * @param string $url
+	 * @param float $time UNIX timestamp just before shutdown() was called
+	 * @return string
+	 */
+	public function appendPreShutdownTimeAsQuery( $url, $time );
+
+	/**
+	 * @param array $info Map of fields, including:
+	 *   - IPAddress : IP address
+	 *   - UserAgent : User-Agent HTTP header
+	 *   - ChronologyProtection : cookie/header value specifying ChronologyProtector usage
+	 */
+	public function setRequestInfo( array $info );
+}
diff --git a/includes/libs/rdbms/lbfactory/LBFactory.php b/includes/libs/rdbms/lbfactory/LBFactory.php
index fd89f336db..85194bc009 100644
--- a/includes/libs/rdbms/lbfactory/LBFactory.php
+++ b/includes/libs/rdbms/lbfactory/LBFactory.php
@@ -27,7 +27,7 @@ use Psr\Log\LoggerInterface;
  * An interface for generating database load balancers
  * @ingroup Database
  */
-abstract class LBFactory {
+abstract class LBFactory implements ILBFactory {
 	/** @var ChronologyProtector */
 	protected $chronProt;
 	/** @var object|string Class name or object With profileIn/profileOut methods */
@@ -72,35 +72,9 @@ abstract class LBFactory {
 	/** @var string Agent name for query profiling */
 	protected $agent;
 
-	const SHUTDOWN_NO_CHRONPROT = 0; // don't save DB positions at all
-	const SHUTDOWN_CHRONPROT_ASYNC = 1; // save DB positions, but don't wait on remote DCs
-	const SHUTDOWN_CHRONPROT_SYNC = 2; // save DB positions, waiting on all DCs
-
 	private static $loggerFields =
 		[ 'replLogger', 'connLogger', 'queryLogger', 'perfLogger' ];
 
-	/**
-	 * Construct a manager of ILoadBalancer objects
-	 *
-	 * Sub-classes will extend the required keys in $conf with additional parameters
-	 *
-	 * @param $conf $params Array with keys:
-	 *  - localDomain: A DatabaseDomain or domain ID string.
-	 *  - readOnlyReason : Reason the master DB is read-only if so [optional]
-	 *  - srvCache : BagOStuff object for server cache [optional]
-	 *  - memCache : BagOStuff object for cluster memory cache [optional]
-	 *  - wanCache : WANObjectCache object [optional]
-	 *  - hostname : The name of the current server [optional]
-	 *  - cliMode: Whether the execution context is a CLI script. [optional]
-	 *  - profiler : Class name or instance with profileIn()/profileOut() methods. [optional]
-	 *  - trxProfiler: TransactionProfiler instance. [optional]
-	 *  - replLogger: PSR-3 logger instance. [optional]
-	 *  - connLogger: PSR-3 logger instance. [optional]
-	 *  - queryLogger: PSR-3 logger instance. [optional]
-	 *  - perfLogger: PSR-3 logger instance. [optional]
-	 *  - errorLogger : Callback that takes an Exception and logs it. [optional]
-	 * @throws InvalidArgumentException
-	 */
 	public function __construct( array $conf ) {
 		$this->localDomain = isset( $conf['localDomain'] )
 			? DatabaseDomain::newFromId( $conf['localDomain'] )
@@ -143,78 +117,11 @@ abstract class LBFactory {
 		$this->ticket = mt_rand();
 	}
 
-	/**
-	 * Disables all load balancers. All connections are closed, and any attempt to
-	 * open a new connection will result in a DBAccessError.
-	 * @see ILoadBalancer::disable()
-	 */
 	public function destroy() {
 		$this->shutdown( self::SHUTDOWN_NO_CHRONPROT );
 		$this->forEachLBCallMethod( 'disable' );
 	}
 
-	/**
-	 * Create a new load balancer object. The resulting object will be untracked,
-	 * not chronology-protected, and the caller is responsible for cleaning it up.
-	 *
-	 * This method is for only advanced usage and callers should almost always use
-	 * getMainLB() instead. This method can be useful when a table is used as a key/value
-	 * store. In that cases, one might want to query it in autocommit mode (DBO_TRX off)
-	 * but still use DBO_TRX transaction rounds on other tables.
-	 *
-	 * @param bool|string $domain Domain ID, or false for the current domain
-	 * @return ILoadBalancer
-	 */
-	abstract public function newMainLB( $domain = false );
-
-	/**
-	 * Get a cached (tracked) load balancer object.
-	 *
-	 * @param bool|string $domain Domain ID, or false for the current domain
-	 * @return ILoadBalancer
-	 */
-	abstract public function getMainLB( $domain = false );
-
-	/**
-	 * Create a new load balancer for external storage. The resulting object will be
-	 * untracked, not chronology-protected, and the caller is responsible for
-	 * cleaning it up.
-	 *
-	 * This method is for only advanced usage and callers should almost always use
-	 * getExternalLB() instead. This method can be useful when a table is used as a
-	 * key/value store. In that cases, one might want to query it in autocommit mode
-	 * (DBO_TRX off) but still use DBO_TRX transaction rounds on other tables.
-	 *
-	 * @param string $cluster External storage cluster, or false for core
-	 * @param bool|string $domain Domain ID, or false for the current domain
-	 * @return ILoadBalancer
-	 */
-	abstract public function newExternalLB( $cluster, $domain = false );
-
-	/**
-	 * Get a cached (tracked) load balancer for external storage
-	 *
-	 * @param string $cluster External storage cluster, or false for core
-	 * @param bool|string $domain Domain ID, or false for the current domain
-	 * @return ILoadBalancer
-	 */
-	abstract public function getExternalLB( $cluster, $domain = false );
-
-	/**
-	 * Execute a function for each tracked load balancer
-	 * The callback is called with the load balancer as the first parameter,
-	 * and $params passed as the subsequent parameters.
-	 *
-	 * @param callable $callback
-	 * @param array $params
-	 */
-	abstract public function forEachLB( $callback, array $params = [] );
-
-	/**
-	 * Prepare all tracked load balancers for shutdown
-	 * @param integer $mode One of the class SHUTDOWN_* constants
-	 * @param callable|null $workCallback Work to mask ChronologyProtector writes
-	 */
 	public function shutdown(
 		$mode = self::SHUTDOWN_CHRONPROT_SYNC, callable $workCallback = null
 	) {
@@ -243,43 +150,15 @@ abstract class LBFactory {
 		);
 	}
 
-	/**
-	 * Commit all replica DB transactions so as to flush any REPEATABLE-READ or SSI snapshot
-	 *
-	 * @param string $fname Caller name
-	 * @since 1.28
-	 */
 	public function flushReplicaSnapshots( $fname = __METHOD__ ) {
 		$this->forEachLBCallMethod( 'flushReplicaSnapshots', [ $fname ] );
 	}
 
-	/**
-	 * Commit on all connections. Done for two reasons:
-	 * 1. To commit changes to the masters.
-	 * 2. To release the snapshot on all connections, master and replica DB.
-	 * @param string $fname Caller name
-	 * @param array $options Options map:
-	 *   - maxWriteDuration: abort if more than this much time was spent in write queries
-	 */
 	public function commitAll( $fname = __METHOD__, array $options = [] ) {
 		$this->commitMasterChanges( $fname, $options );
 		$this->forEachLBCallMethod( 'commitAll', [ $fname ] );
 	}
 
-	/**
-	 * Flush any master transaction snapshots and set DBO_TRX (if DBO_DEFAULT is set)
-	 *
-	 * The DBO_TRX setting will be reverted to the default in each of these methods:
-	 *   - commitMasterChanges()
-	 *   - rollbackMasterChanges()
-	 *   - commitAll()
-	 *
-	 * This allows for custom transaction rounds from any outer transaction scope.
-	 *
-	 * @param string $fname
-	 * @throws DBTransactionError
-	 * @since 1.28
-	 */
 	public function beginMasterChanges( $fname = __METHOD__ ) {
 		if ( $this->trxRoundId !== false ) {
 			throw new DBTransactionError(
@@ -292,13 +171,6 @@ abstract class LBFactory {
 		$this->forEachLBCallMethod( 'beginMasterChanges', [ $fname ] );
 	}
 
-	/**
-	 * Commit changes on all master connections
-	 * @param string $fname Caller name
-	 * @param array $options Options map:
-	 *   - maxWriteDuration: abort if more than this much time was spent in write queries
-	 * @throws Exception
-	 */
 	public function commitMasterChanges( $fname = __METHOD__, array $options = [] ) {
 		if ( $this->trxRoundId !== false && $this->trxRoundId !== $fname ) {
 			throw new DBTransactionError(
@@ -330,11 +202,6 @@ abstract class LBFactory {
 		}
 	}
 
-	/**
-	 * Rollback changes on all master connections
-	 * @param string $fname Caller name
-	 * @since 1.23
-	 */
 	public function rollbackMasterChanges( $fname = __METHOD__ ) {
 		$this->trxRoundId = false;
 		$this->forEachLBCallMethod( 'suppressTransactionEndCallbacks' );
@@ -368,11 +235,6 @@ abstract class LBFactory {
 		}
 	}
 
-	/**
-	 * Determine if any master connection has pending changes
-	 * @return bool
-	 * @since 1.23
-	 */
 	public function hasMasterChanges() {
 		$ret = false;
 		$this->forEachLB( function ( ILoadBalancer $lb ) use ( &$ret ) {
@@ -382,11 +244,6 @@ abstract class LBFactory {
 		return $ret;
 	}
 
-	/**
-	 * Detemine if any lagged replica DB connection was used
-	 * @return bool
-	 * @since 1.28
-	 */
 	public function laggedReplicaUsed() {
 		$ret = false;
 		$this->forEachLB( function ( ILoadBalancer $lb ) use ( &$ret ) {
@@ -396,12 +253,6 @@ abstract class LBFactory {
 		return $ret;
 	}
 
-	/**
-	 * Determine if any master connection has pending/written changes from this request
-	 * @param float $age How many seconds ago is "recent" [defaults to LB lag wait timeout]
-	 * @return bool
-	 * @since 1.27
-	 */
 	public function hasOrMadeRecentMasterChanges( $age = null ) {
 		$ret = false;
 		$this->forEachLB( function ( ILoadBalancer $lb ) use ( $age, &$ret ) {
@@ -410,30 +261,6 @@ abstract class LBFactory {
 		return $ret;
 	}
 
-	/**
-	 * Waits for the replica DBs to catch up to the current master position
-	 *
-	 * Use this when updating very large numbers of rows, as in maintenance scripts,
-	 * to avoid causing too much lag. Of course, this is a no-op if there are no replica DBs.
-	 *
-	 * By default this waits on all DB clusters actually used in this request.
-	 * This makes sense when lag being waiting on is caused by the code that does this check.
-	 * In that case, setting "ifWritesSince" can avoid the overhead of waiting for clusters
-	 * that were not changed since the last wait check. To forcefully wait on a specific cluster
-	 * for a given domain, use the 'domain' parameter. To forcefully wait on an "external" cluster,
-	 * use the "cluster" parameter.
-	 *
-	 * Never call this function after a large DB write that is *still* in a transaction.
-	 * It only makes sense to call this after the possible lag inducing changes were committed.
-	 *
-	 * @param array $opts Optional fields that include:
-	 *   - domain : wait on the load balancer DBs that handles the given domain ID
-	 *   - cluster : wait on the given external load balancer DBs
-	 *   - timeout : Max wait time. Default: ~60 seconds
-	 *   - ifWritesSince: Only wait if writes were done since this UNIX timestamp
-	 * @throws DBReplicationWaitError If a timeout or error occured waiting on a DB cluster
-	 * @since 1.27
-	 */
 	public function waitForReplication( array $opts = [] ) {
 		$opts += [
 			'domain' => false,
@@ -503,15 +330,6 @@ abstract class LBFactory {
 		}
 	}
 
-	/**
-	 * Add a callback to be run in every call to waitForReplication() before waiting
-	 *
-	 * Callbacks must clear any transactions that they start
-	 *
-	 * @param string $name Callback name
-	 * @param callable|null $callback Use null to unset a callback
-	 * @since 1.28
-	 */
 	public function setWaitForReplicationListener( $name, callable $callback = null ) {
 		if ( $callback ) {
 			$this->replicationWaitCallbacks[$name] = $callback;
@@ -520,13 +338,6 @@ abstract class LBFactory {
 		}
 	}
 
-	/**
-	 * Get a token asserting that no transaction writes are active
-	 *
-	 * @param string $fname Caller name (e.g. __METHOD__)
-	 * @return mixed A value to pass to commitAndWaitForReplication()
-	 * @since 1.28
-	 */
 	public function getEmptyTransactionTicket( $fname ) {
 		if ( $this->hasMasterChanges() ) {
 			$this->queryLogger->error( __METHOD__ . ": $fname does not have outer scope.\n" .
@@ -538,17 +349,6 @@ abstract class LBFactory {
 		return $this->ticket;
 	}
 
-	/**
-	 * Convenience method for safely running commitMasterChanges()/waitForReplication()
-	 *
-	 * This will commit and wait unless $ticket indicates it is unsafe to do so
-	 *
-	 * @param string $fname Caller name (e.g. __METHOD__)
-	 * @param mixed $ticket Result of getEmptyTransactionTicket()
-	 * @param array $opts Options to waitForReplication()
-	 * @throws DBReplicationWaitError
-	 * @since 1.28
-	 */
 	public function commitAndWaitForReplication( $fname, $ticket, array $opts = [] ) {
 		if ( $ticket !== $this->ticket ) {
 			$this->perfLogger->error( __METHOD__ . ": $fname does not have outer scope.\n" .
@@ -575,22 +375,10 @@ abstract class LBFactory {
 		}
 	}
 
-	/**
-	 * @param string $dbName DB master name (e.g. "db1052")
-	 * @return float|bool UNIX timestamp when client last touched the DB or false if not recent
-	 * @since 1.28
-	 */
 	public function getChronologyProtectorTouched( $dbName ) {
 		return $this->getChronologyProtector()->getTouched( $dbName );
 	}
 
-	/**
-	 * Disable the ChronologyProtector for all load balancers
-	 *
-	 * This can be called at the start of special API entry points
-	 *
-	 * @since 1.27
-	 */
 	public function disableChronologyProtection() {
 		$this->getChronologyProtector()->setEnabled( false );
 	}
@@ -691,12 +479,6 @@ abstract class LBFactory {
 		}
 	}
 
-	/**
-	 * Set a new table prefix for the existing local domain ID for testing
-	 *
-	 * @param string $prefix
-	 * @since 1.28
-	 */
 	public function setDomainPrefix( $prefix ) {
 		$this->localDomain = new DatabaseDomain(
 			$this->localDomain->getDatabase(),
@@ -709,32 +491,14 @@ abstract class LBFactory {
 		} );
 	}
 
-	/**
-	 * Close all open database connections on all open load balancers.
-	 * @since 1.28
-	 */
 	public function closeAll() {
 		$this->forEachLBCallMethod( 'closeAll', [] );
 	}
 
-	/**
-	 * @param string $agent Agent name for query profiling
-	 * @since 1.28
-	 */
 	public function setAgentName( $agent ) {
 		$this->agent = $agent;
 	}
 
-	/**
-	 * Append ?cpPosTime parameter to a URL for ChronologyProtector purposes if needed
-	 *
-	 * Note that unlike cookies, this works accross domains
-	 *
-	 * @param string $url
-	 * @param float $time UNIX timestamp just before shutdown() was called
-	 * @return string
-	 * @since 1.28
-	 */
 	public function appendPreShutdownTimeAsQuery( $url, $time ) {
 		$usedCluster = 0;
 		$this->forEachLB( function ( ILoadBalancer $lb ) use ( &$usedCluster ) {
@@ -748,13 +512,6 @@ abstract class LBFactory {
 		return strpos( $url, '?' ) === false ? "$url?cpPosTime=$time" : "$url&cpPosTime=$time";
 	}
 
-	/**
-	 * @param array $info Map of fields, including:
-	 *   - IPAddress : IP address
-	 *   - UserAgent : User-Agent HTTP header
-	 *   - ChronologyProtection : cookie/header value specifying ChronologyProtector usage
-	 * @since 1.28
-	 */
 	public function setRequestInfo( array $info ) {
 		$this->requestInfo = $info + $this->requestInfo;
 	}
diff --git a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
index 3b2479f1eb..54d8203300 100644
--- a/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
+++ b/includes/libs/rdbms/loadbalancer/ILoadBalancer.php
@@ -60,9 +60,9 @@
  *   - Read-only archive clones: set 'is static' in the server configuration maps. This will
  *      treat all such DBs as having 0 lag.
  *   - SQL load balancing proxy: any proxy should handle lag checks on its own, so the 'max lag'
- *     parameter should probably be set to INF in the server configuration maps. This will make
- *     the load balancer ignore whatever it detects as the lag of the logical replica is (which
- *     would probably just randomly bounce around).
+ *      parameter should probably be set to INF in the server configuration maps. This will make
+ *      the load balancer ignore whatever it detects as the lag of the logical replica is (which
+ *      would probably just randomly bounce around).
  *
  * If using a SQL proxy service, it would probably be best to have two proxy hosts for the
  * load balancer to talk to. One would be the 'host' of the master server entry and another for