Switch various LoadBalancer::getConnection() callers to getConnectionRef()
authorAaron Schulz <aschulz@wikimedia.org>
Fri, 28 Jun 2019 16:40:05 +0000 (09:40 -0700)
committerAaron Schulz <aschulz@wikimedia.org>
Fri, 12 Jul 2019 17:56:30 +0000 (10:56 -0700)
This is the preferred method as it enforces read-only mode for DB_REPLICA
and handles LoadBalancer::reuseConnection() calls automatically.

Change-Id: Iab9439ba8e0810fa14c302661ed7a3534f6bfc0d

15 files changed:
includes/SiteStats.php
includes/Storage/PageEditStash.php
includes/block/BlockRestrictionStore.php
includes/deferred/SiteStatsUpdate.php
includes/deferred/UserEditCountUpdate.php
includes/jobqueue/jobs/CategoryMembershipChangeJob.php
includes/jobqueue/jobs/ClearUserWatchlistJob.php
includes/jobqueue/jobs/ClearWatchlistNotificationsJob.php
includes/jobqueue/jobs/RefreshLinksJob.php
includes/page/WikiPage.php
includes/search/SearchOracle.php
includes/search/SearchSqlite.php
includes/site/DBSiteStore.php
includes/user/User.php
includes/user/UserGroupMembership.php

index e3cb617..cf3a1eb 100644 (file)
@@ -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(
index 2285f4a..6caca29 100644 (file)
@@ -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'],
index df09ead..4fa4cfe 100644 (file)
@@ -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',
index 7cb2950..007cf0e 100644 (file)
@@ -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();
index ed7e00c..687dfbe 100644 (file)
@@ -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 );
index 882ae64..cb4b051 100644 (file)
@@ -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;
index 74b90ed..e373605 100644 (file)
@@ -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 ) ) {
index f53174a..054718d 100644 (file)
@@ -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 ) {
index 3179a2f..b4046a6 100644 (file)
@@ -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)
index fa01ce4..6f94ccf 100644 (file)
@@ -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 ) {
index a5d351b..7240e81 100644 (file)
@@ -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' ],
                        [
index 3646b27..dedcdff 100644 (file)
@@ -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;
                }
index bb6a6b3..6076aba 100644 (file)
@@ -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__ );
index 1f61cb9..7c2f038 100644 (file)
@@ -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 );
                                },
index dff19ff..fdac4a2 100644 (file)
@@ -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 );