From: Aaron Schulz <aschulz@wikimedia.org>
Date: Fri, 28 Jun 2019 16:40:05 +0000 (-0700)
Subject: Switch various LoadBalancer::getConnection() callers to getConnectionRef()
X-Git-Tag: 1.34.0-rc.0~1019^2
X-Git-Url: https://git.cyclocoop.org/admin/%7B%24admin_url%7Dmembres/import.php?a=commitdiff_plain;h=f72ae0f6e6ae71e12dd18e5c2d6e1712e5becfbd;p=lhc%2Fweb%2Fwiklou.git

Switch various LoadBalancer::getConnection() callers to getConnectionRef()

This is the preferred method as it enforces read-only mode for DB_REPLICA
and handles LoadBalancer::reuseConnection() calls automatically.

Change-Id: Iab9439ba8e0810fa14c302661ed7a3534f6bfc0d
---

diff --git a/includes/SiteStats.php b/includes/SiteStats.php
index e3cb617e3f..cf3a1ebdf7 100644
--- a/includes/SiteStats.php
+++ b/includes/SiteStats.php
@@ -52,14 +52,14 @@ class SiteStats {
 		$config = MediaWikiServices::getInstance()->getMainConfig();
 
 		$lb = self::getLB();
-		$dbr = $lb->getConnection( DB_REPLICA );
+		$dbr = $lb->getConnectionRef( DB_REPLICA );
 		wfDebug( __METHOD__ . ": reading site_stats from replica DB\n" );
 		$row = self::doLoadFromDB( $dbr );
 
 		if ( !self::isRowSane( $row ) && $lb->hasOrMadeRecentMasterChanges() ) {
 			// Might have just been initialized during this request? Underflow?
 			wfDebug( __METHOD__ . ": site_stats damaged or missing on replica DB\n" );
-			$row = self::doLoadFromDB( $lb->getConnection( DB_MASTER ) );
+			$row = self::doLoadFromDB( $lb->getConnectionRef( DB_MASTER ) );
 		}
 
 		if ( !self::isRowSane( $row ) ) {
@@ -76,7 +76,7 @@ class SiteStats {
 				SiteStatsInit::doAllAndCommit( $dbr );
 			}
 
-			$row = self::doLoadFromDB( $lb->getConnection( DB_MASTER ) );
+			$row = self::doLoadFromDB( $lb->getConnectionRef( DB_MASTER ) );
 		}
 
 		if ( !self::isRowSane( $row ) ) {
@@ -155,7 +155,7 @@ class SiteStats {
 			$cache->makeKey( 'SiteStats', 'groupcounts', $group ),
 			$cache::TTL_HOUR,
 			function ( $oldValue, &$ttl, array &$setOpts ) use ( $group, $fname ) {
-				$dbr = self::getLB()->getConnection( DB_REPLICA );
+				$dbr = self::getLB()->getConnectionRef( DB_REPLICA );
 				$setOpts += Database::getCacheSetOptions( $dbr );
 
 				return (int)$dbr->selectField(
@@ -206,7 +206,7 @@ class SiteStats {
 			$cache->makeKey( 'SiteStats', 'page-in-namespace', $ns ),
 			$cache::TTL_HOUR,
 			function ( $oldValue, &$ttl, array &$setOpts ) use ( $ns, $fname ) {
-				$dbr = self::getLB()->getConnection( DB_REPLICA );
+				$dbr = self::getLB()->getConnectionRef( DB_REPLICA );
 				$setOpts += Database::getCacheSetOptions( $dbr );
 
 				return (int)$dbr->selectField(
diff --git a/includes/Storage/PageEditStash.php b/includes/Storage/PageEditStash.php
index 2285f4a953..6caca29ba1 100644
--- a/includes/Storage/PageEditStash.php
+++ b/includes/Storage/PageEditStash.php
@@ -109,7 +109,7 @@ class PageEditStash {
 		// the stash request finishes parsing. For the lock acquisition below, there is not much
 		// need to duplicate parsing of the same content/user/summary bundle, so try to avoid
 		// blocking at all here.
-		$dbw = $this->lb->getConnection( DB_MASTER );
+		$dbw = $this->lb->getConnectionRef( DB_MASTER );
 		if ( !$dbw->lock( $key, $fname, 0 ) ) {
 			// De-duplicate requests on the same key
 			return self::ERROR_BUSY;
@@ -357,7 +357,8 @@ class PageEditStash {
 	 * @return string|null TS_MW timestamp or null
 	 */
 	private function lastEditTime( User $user ) {
-		$db = $this->lb->getConnection( DB_REPLICA );
+		$db = $this->lb->getConnectionRef( DB_REPLICA );
+
 		$actorQuery = ActorMigration::newMigration()->getWhere( $db, 'rc_user', $user, false );
 		$time = $db->selectField(
 			[ 'recentchanges' ] + $actorQuery['tables'],
diff --git a/includes/block/BlockRestrictionStore.php b/includes/block/BlockRestrictionStore.php
index df09eadc8f..4fa4cfe782 100644
--- a/includes/block/BlockRestrictionStore.php
+++ b/includes/block/BlockRestrictionStore.php
@@ -66,7 +66,7 @@ class BlockRestrictionStore {
 			return [];
 		}
 
-		$db = $db ?: $this->loadBalancer->getConnection( DB_REPLICA );
+		$db = $db ?: $this->loadBalancer->getConnectionRef( DB_REPLICA );
 
 		$result = $db->select(
 			[ 'ipblocks_restrictions', 'page' ],
@@ -104,7 +104,7 @@ class BlockRestrictionStore {
 			return false;
 		}
 
-		$dbw = $this->loadBalancer->getConnection( DB_MASTER );
+		$dbw = $this->loadBalancer->getConnectionRef( DB_MASTER );
 
 		$dbw->insert(
 			'ipblocks_restrictions',
@@ -125,7 +125,7 @@ class BlockRestrictionStore {
 	 * @return bool
 	 */
 	public function update( array $restrictions ) {
-		$dbw = $this->loadBalancer->getConnection( DB_MASTER );
+		$dbw = $this->loadBalancer->getConnectionRef( DB_MASTER );
 
 		$dbw->startAtomic( __METHOD__ );
 
@@ -197,7 +197,7 @@ class BlockRestrictionStore {
 
 		$parentBlockId = (int)$parentBlockId;
 
-		$db = $this->loadBalancer->getConnection( DB_MASTER );
+		$db = $this->loadBalancer->getConnectionRef( DB_MASTER );
 
 		$db->startAtomic( __METHOD__ );
 
@@ -230,7 +230,7 @@ class BlockRestrictionStore {
 	 * @return bool
 	 */
 	public function delete( array $restrictions ) {
-		$dbw = $this->loadBalancer->getConnection( DB_MASTER );
+		$dbw = $this->loadBalancer->getConnectionRef( DB_MASTER );
 		$result = true;
 		foreach ( $restrictions as $restriction ) {
 			if ( !$restriction instanceof Restriction ) {
@@ -260,7 +260,7 @@ class BlockRestrictionStore {
 	 * @return bool
 	 */
 	public function deleteByBlockId( $blockId ) {
-		$dbw = $this->loadBalancer->getConnection( DB_MASTER );
+		$dbw = $this->loadBalancer->getConnectionRef( DB_MASTER );
 		return $dbw->delete(
 			'ipblocks_restrictions',
 			[ 'ir_ipb_id' => $blockId ],
@@ -277,7 +277,7 @@ class BlockRestrictionStore {
 	 * @return bool
 	 */
 	public function deleteByParentBlockId( $parentBlockId ) {
-		$dbw = $this->loadBalancer->getConnection( DB_MASTER );
+		$dbw = $this->loadBalancer->getConnectionRef( DB_MASTER );
 		return $dbw->deleteJoin(
 			'ipblocks_restrictions',
 			'ipblocks',
diff --git a/includes/deferred/SiteStatsUpdate.php b/includes/deferred/SiteStatsUpdate.php
index 7cb2950942..007cf0e5e7 100644
--- a/includes/deferred/SiteStatsUpdate.php
+++ b/includes/deferred/SiteStatsUpdate.php
@@ -102,7 +102,7 @@ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate {
 		$services = MediaWikiServices::getInstance();
 		$config = $services->getMainConfig();
 
-		$dbw = $services->getDBLoadBalancer()->getConnection( DB_MASTER );
+		$dbw = $services->getDBLoadBalancer()->getConnectionRef( DB_MASTER );
 		$lockKey = $dbw->getDomainID() . ':site_stats'; // prepend wiki ID
 		$pd = [];
 		if ( $config->get( 'SiteStatsAsyncFactor' ) ) {
@@ -151,7 +151,7 @@ class SiteStatsUpdate implements DeferrableUpdate, MergeableUpdate {
 		$services = MediaWikiServices::getInstance();
 		$config = $services->getMainConfig();
 
-		$dbr = $services->getDBLoadBalancer()->getConnection( DB_REPLICA, 'vslow' );
+		$dbr = $services->getDBLoadBalancer()->getConnectionRef( DB_REPLICA, 'vslow' );
 		# Get non-bot users than did some recent action other than making accounts.
 		# If account creation is included, the number gets inflated ~20+ fold on enwiki.
 		$rcQuery = RecentChange::getQueryInfo();
diff --git a/includes/deferred/UserEditCountUpdate.php b/includes/deferred/UserEditCountUpdate.php
index ed7e00cfba..687dfbe907 100644
--- a/includes/deferred/UserEditCountUpdate.php
+++ b/includes/deferred/UserEditCountUpdate.php
@@ -67,7 +67,7 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate {
 	 */
 	public function doUpdate() {
 		$lb = MediaWikiServices::getInstance()->getDBLoadBalancer();
-		$dbw = $lb->getConnection( DB_MASTER );
+		$dbw = $lb->getConnectionRef( DB_MASTER );
 		$fname = __METHOD__;
 
 		( new AutoCommitUpdate( $dbw, __METHOD__, function () use ( $lb, $dbw, $fname ) {
@@ -85,8 +85,8 @@ class UserEditCountUpdate implements DeferrableUpdate, MergeableUpdate {
 					// The user_editcount is probably NULL (e.g. not initialized).
 					// Since this update runs after the new revisions were committed,
 					// wait for the replica DB to catch up so they will be counted.
-					$dbr = $lb->getConnection( DB_REPLICA );
-					// If $dbr is actually the master DB, then clearing the snapshot is
+					$dbr = $lb->getConnectionRef( DB_REPLICA );
+					// If $dbr is actually the master DB, then clearing the snapshot
 					// is harmless and waitForMasterPos() will just no-op.
 					$dbr->flushSnapshot( $fname );
 					$lb->waitForMasterPos( $dbr );
diff --git a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
index 882ae6430b..cb4b051978 100644
--- a/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
+++ b/includes/jobqueue/jobs/CategoryMembershipChangeJob.php
@@ -81,7 +81,7 @@ class CategoryMembershipChangeJob extends Job {
 	public function run() {
 		$lbFactory = MediaWikiServices::getInstance()->getDBLoadBalancerFactory();
 		$lb = $lbFactory->getMainLB();
-		$dbw = $lb->getConnection( DB_MASTER );
+		$dbw = $lb->getConnectionRef( DB_MASTER );
 
 		$this->ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
 
@@ -92,7 +92,7 @@ class CategoryMembershipChangeJob extends Job {
 		}
 
 		// Cut down on the time spent in waitForMasterPos() in the critical section
-		$dbr = $lb->getConnection( DB_REPLICA, [ 'recentchanges' ] );
+		$dbr = $lb->getConnectionRef( DB_REPLICA, [ 'recentchanges' ] );
 		if ( !$lb->waitForMasterPos( $dbr ) ) {
 			$this->setLastError( "Timed out while pre-waiting for replica DB to catch up" );
 			return false;
diff --git a/includes/jobqueue/jobs/ClearUserWatchlistJob.php b/includes/jobqueue/jobs/ClearUserWatchlistJob.php
index 74b90ed6d7..e373605281 100644
--- a/includes/jobqueue/jobs/ClearUserWatchlistJob.php
+++ b/includes/jobqueue/jobs/ClearUserWatchlistJob.php
@@ -40,8 +40,8 @@ class ClearUserWatchlistJob extends Job implements GenericParameterJob {
 		$batchSize = $wgUpdateRowsPerQuery;
 
 		$loadBalancer = MediaWikiServices::getInstance()->getDBLoadBalancer();
-		$dbw = $loadBalancer->getConnection( DB_MASTER );
-		$dbr = $loadBalancer->getConnection( DB_REPLICA, [ 'watchlist' ] );
+		$dbw = $loadBalancer->getConnectionRef( DB_MASTER );
+		$dbr = $loadBalancer->getConnectionRef( DB_REPLICA, [ 'watchlist' ] );
 
 		// Wait before lock to try to reduce time waiting in the lock.
 		if ( !$loadBalancer->waitForMasterPos( $dbr ) ) {
diff --git a/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php b/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php
index f53174a573..054718d900 100644
--- a/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php
+++ b/includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php
@@ -51,7 +51,7 @@ class ClearWatchlistNotificationsJob extends Job implements GenericParameterJob
 		$lbFactory = $services->getDBLoadBalancerFactory();
 		$rowsPerQuery = $services->getMainConfig()->get( 'UpdateRowsPerQuery' );
 
-		$dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER );
+		$dbw = $lbFactory->getMainLB()->getConnectionRef( DB_MASTER );
 		$ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
 		$timestamp = $this->params['timestamp'] ?? null;
 		if ( $timestamp === null ) {
diff --git a/includes/jobqueue/jobs/RefreshLinksJob.php b/includes/jobqueue/jobs/RefreshLinksJob.php
index 3179a2fbc3..b4046a61bc 100644
--- a/includes/jobqueue/jobs/RefreshLinksJob.php
+++ b/includes/jobqueue/jobs/RefreshLinksJob.php
@@ -156,7 +156,9 @@ class RefreshLinksJob extends Job {
 		// Serialize link update job by page ID so they see each others' changes.
 		// The page ID and latest revision ID will be queried again after the lock
 		// is acquired to bail if they are changed from that of loadPageData() above.
-		$dbw = $lbFactory->getMainLB()->getConnection( DB_MASTER );
+		// Serialize links updates by page ID so they see each others' changes
+		$dbw = $lbFactory->getMainLB()->getConnectionRef( DB_MASTER );
+		/** @noinspection PhpUnusedLocalVariableInspection */
 		$scopedLock = LinksUpdate::acquirePageLock( $dbw, $page->getId(), 'job' );
 		if ( $scopedLock === null ) {
 			// Another job is already updating the page, likely for a prior revision (T170596)
diff --git a/includes/page/WikiPage.php b/includes/page/WikiPage.php
index fa01ce49a6..6f94ccf018 100644
--- a/includes/page/WikiPage.php
+++ b/includes/page/WikiPage.php
@@ -1588,7 +1588,7 @@ class WikiPage implements Page, IDBAccessObject {
 		$baseRevId = null;
 		if ( $edittime && $sectionId !== 'new' ) {
 			$lb = $this->getDBLoadBalancer();
-			$dbr = $lb->getConnection( DB_REPLICA );
+			$dbr = $lb->getConnectionRef( DB_REPLICA );
 			$rev = Revision::loadFromTimestamp( $dbr, $this->mTitle, $edittime );
 			// Try the master if this thread may have just added it.
 			// This could be abstracted into a Revision method, but we don't want
@@ -1597,7 +1597,7 @@ class WikiPage implements Page, IDBAccessObject {
 				&& $lb->getServerCount() > 1
 				&& $lb->hasOrMadeRecentMasterChanges()
 			) {
-				$dbw = $lb->getConnection( DB_MASTER );
+				$dbw = $lb->getConnectionRef( DB_MASTER );
 				$rev = Revision::loadFromTimestamp( $dbw, $this->mTitle, $edittime );
 			}
 			if ( $rev ) {
diff --git a/includes/search/SearchOracle.php b/includes/search/SearchOracle.php
index a5d351bcd7..7240e819ad 100644
--- a/includes/search/SearchOracle.php
+++ b/includes/search/SearchOracle.php
@@ -240,7 +240,7 @@ class SearchOracle extends SearchDatabase {
 	 * @param string $text
 	 */
 	function update( $id, $title, $text ) {
-		$dbw = $this->lb->getConnection( DB_MASTER );
+		$dbw = $this->lb->getMaintenanceConnectionRef( DB_MASTER );
 		$dbw->replace( 'searchindex',
 			[ 'si_page' ],
 			[
diff --git a/includes/search/SearchSqlite.php b/includes/search/SearchSqlite.php
index 3646b274ed..dedcdff43c 100644
--- a/includes/search/SearchSqlite.php
+++ b/includes/search/SearchSqlite.php
@@ -33,11 +33,15 @@ class SearchSqlite extends SearchDatabase {
 	 * Whether fulltext search is supported by current schema
 	 * @return bool
 	 */
-	function fulltextSearchSupported() {
+	private function fulltextSearchSupported() {
+		// Avoid getConnectionRef() in order to get DatabaseSqlite specifically
 		/** @var DatabaseSqlite $dbr */
 		$dbr = $this->lb->getConnection( DB_REPLICA );
-
-		return $dbr->checkForEnabledSearch();
+		try {
+			return $dbr->checkForEnabledSearch();
+		} finally {
+			$this->lb->reuseConnection( $dbr );
+		}
 	}
 
 	/**
@@ -285,7 +289,7 @@ class SearchSqlite extends SearchDatabase {
 	 * @param string $title
 	 * @param string $text
 	 */
-	function update( $id, $title, $text ) {
+	public function update( $id, $title, $text ) {
 		if ( !$this->fulltextSearchSupported() ) {
 			return;
 		}
@@ -308,7 +312,7 @@ class SearchSqlite extends SearchDatabase {
 	 * @param int $id
 	 * @param string $title
 	 */
-	function updateTitle( $id, $title ) {
+	public function updateTitle( $id, $title ) {
 		if ( !$this->fulltextSearchSupported() ) {
 			return;
 		}
diff --git a/includes/site/DBSiteStore.php b/includes/site/DBSiteStore.php
index bb6a6b3cb0..6076aba866 100644
--- a/includes/site/DBSiteStore.php
+++ b/includes/site/DBSiteStore.php
@@ -75,7 +75,7 @@ class DBSiteStore implements SiteStore {
 	protected function loadSites() {
 		$this->sites = new SiteList();
 
-		$dbr = $this->dbLoadBalancer->getConnection( DB_REPLICA );
+		$dbr = $this->dbLoadBalancer->getConnectionRef( DB_REPLICA );
 
 		$res = $dbr->select(
 			'sites',
@@ -178,7 +178,7 @@ class DBSiteStore implements SiteStore {
 			return true;
 		}
 
-		$dbw = $this->dbLoadBalancer->getConnection( DB_MASTER );
+		$dbw = $this->dbLoadBalancer->getConnectionRef( DB_MASTER );
 
 		$dbw->startAtomic( __METHOD__ );
 
@@ -269,7 +269,7 @@ class DBSiteStore implements SiteStore {
 	 * @return bool Success
 	 */
 	public function clear() {
-		$dbw = $this->dbLoadBalancer->getConnection( DB_MASTER );
+		$dbw = $this->dbLoadBalancer->getConnectionRef( DB_MASTER );
 
 		$dbw->startAtomic( __METHOD__ );
 		$ok = $dbw->delete( 'sites', '*', __METHOD__ );
diff --git a/includes/user/User.php b/includes/user/User.php
index 1f61cb900f..7c2f0380fa 100644
--- a/includes/user/User.php
+++ b/includes/user/User.php
@@ -2584,7 +2584,7 @@ class User implements IDBAccessObject, UserIdentity {
 		if ( $mode === 'refresh' ) {
 			$cache->delete( $key, 1 ); // low tombstone/"hold-off" TTL
 		} else {
-			$lb->getConnection( DB_MASTER )->onTransactionPreCommitOrIdle(
+			$lb->getConnectionRef( DB_MASTER )->onTransactionPreCommitOrIdle(
 				function () use ( $cache, $key ) {
 					$cache->delete( $key );
 				},
diff --git a/includes/user/UserGroupMembership.php b/includes/user/UserGroupMembership.php
index dff19fff82..fdac4a237b 100644
--- a/includes/user/UserGroupMembership.php
+++ b/includes/user/UserGroupMembership.php
@@ -256,7 +256,7 @@ class UserGroupMembership {
 
 		$lbFactory = $services->getDBLoadBalancerFactory();
 		$ticket = $lbFactory->getEmptyTransactionTicket( __METHOD__ );
-		$dbw = $services->getDBLoadBalancer()->getConnection( DB_MASTER );
+		$dbw = $services->getDBLoadBalancer()->getConnectionRef( DB_MASTER );
 
 		$lockKey = "{$dbw->getDomainID()}:UserGroupMembership:purge"; // per-wiki
 		$scopedLock = $dbw->getScopedLockAndFlush( $lockKey, __METHOD__, 0 );