watcheditem: Switch out of LB for picking db connection

A bit of a clean up. Switching out of lbFactory to ICP is not possible
at the moment because there is $this->lbFactory->getLocalDomainID()
for cache prefix. Fixable later.

Bug: T330641
Change-Id: I2d4537f75f5877552c6d9fd2b76c5be1959ea400
This commit is contained in:
Amir Sarabadani 2023-05-05 13:53:32 +02:00
parent 8c6c387046
commit d44c45c76a
8 changed files with 55 additions and 90 deletions

View file

@ -906,7 +906,7 @@ return [
$services->getTitleFormatter(),
$services->getContentLanguage(),
$services->getGenderCache(),
$services->getDBLoadBalancer(),
$services->getDBLoadBalancerFactory(),
$services->getLinksMigration(),
LoggerFactory::getInstance( 'LinkBatch' )
);

View file

@ -29,7 +29,7 @@ use MediaWiki\Page\PageReference;
use MediaWiki\Page\ProperPageIdentity;
use Psr\Log\LoggerInterface;
use Wikimedia\Assert\Assert;
use Wikimedia\Rdbms\ILoadBalancer;
use Wikimedia\Rdbms\IConnectionProvider;
use Wikimedia\Rdbms\IResultWrapper;
use Wikimedia\Rdbms\Platform\ISQLPlatform;
@ -76,9 +76,9 @@ class LinkBatch {
private $genderCache;
/**
* @var ILoadBalancer
* @var IConnectionProvider
*/
private $loadBalancer;
private $dbProvider;
/** @var LinksMigration */
private $linksMigration;
@ -95,7 +95,7 @@ class LinkBatch {
* @param TitleFormatter $titleFormatter
* @param Language $contentLanguage
* @param GenderCache $genderCache
* @param ILoadBalancer $loadBalancer
* @param IConnectionProvider $dbProvider
* @param LinksMigration $linksMigration
* @param LoggerInterface $logger
*/
@ -105,7 +105,7 @@ class LinkBatch {
TitleFormatter $titleFormatter,
Language $contentLanguage,
GenderCache $genderCache,
ILoadBalancer $loadBalancer,
IConnectionProvider $dbProvider,
LinksMigration $linksMigration,
LoggerInterface $logger
) {
@ -113,7 +113,7 @@ class LinkBatch {
$this->titleFormatter = $titleFormatter;
$this->contentLanguage = $contentLanguage;
$this->genderCache = $genderCache;
$this->loadBalancer = $loadBalancer;
$this->dbProvider = $dbProvider;
$this->linksMigration = $linksMigration;
$this->logger = $logger;
@ -321,7 +321,7 @@ class LinkBatch {
}
// This is similar to LinkHolderArray::replaceInternal
$dbr = $this->loadBalancer->getConnection( DB_REPLICA );
$dbr = $this->dbProvider->getReplicaDatabase();
$queryBuilder = $dbr->newSelectQueryBuilder()
->select( LinkCache::getSelectFields() )
->from( 'page' )

View file

@ -32,7 +32,7 @@ use MediaWiki\Linker\LinkTarget;
use MediaWiki\Page\PageReference;
use Psr\Log\LoggerInterface;
use TitleFormatter;
use Wikimedia\Rdbms\ILoadBalancer;
use Wikimedia\Rdbms\IConnectionProvider;
/**
* @ingroup Cache
@ -61,9 +61,9 @@ class LinkBatchFactory {
private $genderCache;
/**
* @var ILoadBalancer
* @var IConnectionProvider
*/
private $loadBalancer;
private $dbProvider;
/** @var LinksMigration */
private $linksMigration;
@ -76,7 +76,7 @@ class LinkBatchFactory {
TitleFormatter $titleFormatter,
Language $contentLanguage,
GenderCache $genderCache,
ILoadBalancer $loadBalancer,
IConnectionProvider $dbProvider,
LinksMigration $linksMigration,
LoggerInterface $logger
) {
@ -84,7 +84,7 @@ class LinkBatchFactory {
$this->titleFormatter = $titleFormatter;
$this->contentLanguage = $contentLanguage;
$this->genderCache = $genderCache;
$this->loadBalancer = $loadBalancer;
$this->dbProvider = $dbProvider;
$this->linksMigration = $linksMigration;
$this->logger = $logger;
}
@ -101,7 +101,7 @@ class LinkBatchFactory {
$this->titleFormatter,
$this->contentLanguage,
$this->genderCache,
$this->loadBalancer,
$this->dbProvider,
$this->linksMigration,
$this->logger
);

View file

@ -13,7 +13,6 @@ use Wikimedia\ParamValidator\TypeDef\ExpiryDef;
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\ILBFactory;
use Wikimedia\Rdbms\IResultWrapper;
use Wikimedia\Rdbms\LoadBalancer;
use Wikimedia\Rdbms\ReadOnlyMode;
use Wikimedia\Rdbms\SelectQueryBuilder;
use Wikimedia\ScopedCallback;
@ -43,11 +42,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
*/
private $lbFactory;
/**
* @var LoadBalancer
*/
private $loadBalancer;
/**
* @var JobQueueGroup
*/
@ -154,7 +148,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
$this->watchlistPurgeRate = $options->get( MainConfigNames::WatchlistPurgeRate );
$this->lbFactory = $lbFactory;
$this->loadBalancer = $lbFactory->getMainLB();
$this->queueGroup = $queueGroup;
$this->stash = $stash;
$this->cache = $cache;
@ -278,15 +271,6 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
return $this->cache->get( $this->getCacheKey( $user, $target ) );
}
/**
* @param int $dbIndex DB_PRIMARY or DB_REPLICA
*
* @return IDatabase
*/
private function getConnectionRef( $dbIndex ): IDatabase {
return $this->loadBalancer->getConnectionRef( $dbIndex );
}
/**
* Helper method to deduplicate logic around queries that need to be modified
* if watchlist expiration is enabled
@ -319,7 +303,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
return false;
}
$dbw = $this->loadBalancer->getConnectionRef( DB_PRIMARY );
$dbw = $this->lbFactory->getPrimaryDatabase();
if ( $this->expiryEnabled ) {
$ticket = $this->lbFactory->getEmptyTransactionTicket( __METHOD__ );
@ -426,7 +410,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @return int The maximum current wl_id
*/
public function getMaxId(): int {
return (int)$this->getConnectionRef( DB_REPLICA )->newSelectQueryBuilder()
return (int)$this->lbFactory->getReplicaDatabase()->newSelectQueryBuilder()
->select( 'MAX(wl_id)' )
->from( 'watchlist' )
->caller( __METHOD__ )
@ -439,8 +423,8 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @return int
*/
public function countWatchedItems( UserIdentity $user ): int {
$dbr = $this->getConnectionRef( DB_REPLICA );
$queryBuilder = $this->getConnectionRef( DB_REPLICA )->newSelectQueryBuilder()
$dbr = $this->lbFactory->getReplicaDatabase();
$queryBuilder = $this->lbFactory->getReplicaDatabase()->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'watchlist' )
->where( [ 'wl_user' => $user->getId() ] )
@ -457,7 +441,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @return int
*/
public function countWatchers( $target ): int {
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbr = $this->lbFactory->getReplicaDatabase();
$queryBuilder = $dbr->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'watchlist' )
@ -479,7 +463,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @return int
*/
public function countVisitingWatchers( $target, $threshold ): int {
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbr = $this->lbFactory->getReplicaDatabase();
$queryBuilder = $dbr->newSelectQueryBuilder()
->select( 'COUNT(*)' )
->from( 'watchlist' )
@ -516,7 +500,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
$rows = $this->getTitleDbKeysGroupedByNamespace( $titles );
$this->uncacheTitlesForUser( $user, $titles );
$dbw = $this->getConnectionRef( DB_PRIMARY );
$dbw = $this->lbFactory->getPrimaryDatabase();
$ticket = count( $titles ) > $this->updateRowsPerQuery ?
$this->lbFactory->getEmptyTransactionTicket( __METHOD__ ) : null;
$affectedRows = 0;
@ -582,7 +566,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
return $target;
}, $targets );
$lb = $this->linkBatchFactory->newLinkBatch( $linkTargets );
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbr = $this->lbFactory->getReplicaDatabase();
$queryBuilder = $dbr->newSelectQueryBuilder();
$queryBuilder
->select( [ 'wl_title', 'wl_namespace', 'watchers' => 'COUNT(*)' ] )
@ -628,7 +612,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
return [];
}
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbr = $this->lbFactory->getReplicaDatabase();
$queryBuilder = $dbr->newSelectQueryBuilder()
->select( [ 'wl_namespace', 'wl_title', 'watchers' => 'COUNT(*)' ] )
->from( 'watchlist' )
@ -743,7 +727,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
return false;
}
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbr = $this->lbFactory->getReplicaDatabase();
$rows = $this->fetchWatchedItems(
$dbr,
@ -783,7 +767,11 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
$options += [ 'forWrite' => false ];
$vars = [ 'wl_namespace', 'wl_title', 'wl_notificationtimestamp' ];
$orderBy = [];
$db = $this->getConnectionRef( $options['forWrite'] ? DB_PRIMARY : DB_REPLICA );
if ( $options['forWrite'] ) {
$db = $this->lbFactory->getPrimaryDatabase();
} else {
$db = $this->lbFactory->getReplicaDatabase();
}
if ( array_key_exists( 'sort', $options ) ) {
Assert::parameter(
( in_array( $options['sort'], [ self::SORT_ASC, self::SORT_DESC ] ) ),
@ -961,7 +949,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
return $timestamps;
}
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbr = $this->lbFactory->getReplicaDatabase();
$lb = $this->linkBatchFactory->newLinkBatch( $targetsToLoad );
$res = $dbr->newSelectQueryBuilder()
@ -1057,7 +1045,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
$this->uncache( $user, $target );
}
$dbw = $this->getConnectionRef( DB_PRIMARY );
$dbw = $this->lbFactory->getPrimaryDatabase();
$ticket = count( $targets ) > $this->updateRowsPerQuery ?
$this->lbFactory->getEmptyTransactionTicket( __METHOD__ ) : null;
$affectedRows = 0;
@ -1214,7 +1202,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
$rows = $this->getTitleDbKeysGroupedByNamespace( $targets );
$dbw = $this->getConnectionRef( DB_PRIMARY );
$dbw = $this->lbFactory->getPrimaryDatabase();
if ( $timestamp !== null ) {
$timestamp = $dbw->timestamp( $timestamp );
}
@ -1327,7 +1315,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
$target,
$timestamp
): array {
$dbw = $this->getConnectionRef( DB_PRIMARY );
$dbw = $this->lbFactory->getPrimaryDatabase();
$queryBuilder = $dbw->newSelectQueryBuilder()
->select( [ 'wl_id', 'wl_user' ] )
->from( 'watchlist' )
@ -1358,7 +1346,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
call_user_func(
$this->deferredUpdatesAddCallableUpdateCallback,
function () use ( $timestamp, $wlIds, $target, $fname ) {
$dbw = $this->getConnectionRef( DB_PRIMARY );
$dbw = $this->lbFactory->getPrimaryDatabase();
$ticket = $this->lbFactory->getEmptyTransactionTicket( $fname );
$wlIdsChunks = array_chunk( $wlIds, $this->updateRowsPerQuery );
@ -1596,7 +1584,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @return int|bool
*/
public function countUnreadNotifications( UserIdentity $user, $unreadLimit = null ) {
$queryBuilder = $this->getConnectionRef( DB_REPLICA )->newSelectQueryBuilder()
$queryBuilder = $this->lbFactory->getReplicaDatabase()->newSelectQueryBuilder()
->select( '1' )
->from( 'watchlist' )
->where( [
@ -1646,7 +1634,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @param LinkTarget|PageIdentity $newTarget deprecated passing LinkTarget since 1.36
*/
public function duplicateEntry( $oldTarget, $newTarget ) {
$dbw = $this->getConnectionRef( DB_PRIMARY );
$dbw = $this->lbFactory->getPrimaryDatabase();
$result = $this->fetchWatchedItemsForPage( $dbw, $oldTarget );
$newNamespace = $newTarget->getNamespace();
$newDBkey = $newTarget->getDBkey();
@ -1793,7 +1781,7 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @inheritDoc
*/
public function countExpired(): int {
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbr = $this->lbFactory->getReplicaDatabase();
return $dbr->newSelectQueryBuilder()
->select( '*' )
->from( 'watchlist_expiry' )
@ -1806,8 +1794,8 @@ class WatchedItemStore implements WatchedItemStoreInterface, StatsdAwareInterfac
* @inheritDoc
*/
public function removeExpired( int $limit, bool $deleteOrphans = false ): void {
$dbr = $this->getConnectionRef( DB_REPLICA );
$dbw = $this->getConnectionRef( DB_PRIMARY );
$dbr = $this->lbFactory->getReplicaDatabase();
$dbw = $this->lbFactory->getPrimaryDatabase();
$ticket = $this->lbFactory->getEmptyTransactionTicket( __METHOD__ );
// Get a batch of watchlist IDs to delete.

View file

@ -7,7 +7,7 @@ use MediaWiki\Logger\LoggerFactory;
use MediaWiki\Page\PageReference;
use MediaWiki\Page\PageReferenceValue;
use MediaWiki\Title\Title;
use Wikimedia\Rdbms\ILoadBalancer;
use Wikimedia\Rdbms\IConnectionProvider;
/**
* @group Database
@ -28,7 +28,7 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$this->createMock( TitleFormatter::class ),
$this->createMock( Language::class ),
$this->createMock( GenderCache::class ),
$this->createMock( ILoadBalancer::class ),
$this->createMock( IConnectionProvider::class ),
$this->createMock( LinksMigration::class ),
LoggerFactory::getInstance( 'LinkBatch' )
);
@ -52,7 +52,7 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$this->createMock( TitleFormatter::class ),
$this->createMock( Language::class ),
$this->createMock( GenderCache::class ),
$this->createMock( ILoadBalancer::class ),
$this->createMock( IConnectionProvider::class ),
$this->createMock( LinksMigration::class ),
LoggerFactory::getInstance( 'LinkBatch' )
);
@ -74,7 +74,7 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$this->createMock( TitleFormatter::class ),
$this->createMock( Language::class ),
$this->createMock( GenderCache::class ),
$this->getServiceContainer()->getDBLoadBalancer(),
$this->getServiceContainer()->getDBLoadBalancerFactory(),
$this->getServiceContainer()->getLinksMigration(),
LoggerFactory::getInstance( 'LinkBatch' )
);
@ -149,7 +149,7 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$services->getTitleFormatter(),
$services->getContentLanguage(),
$services->getGenderCache(),
$services->getDBLoadBalancer(),
$services->getDBLoadBalancerFactory(),
$services->getLinksMigration(),
LoggerFactory::getInstance( 'LinkBatch' )
);
@ -195,7 +195,7 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$this->createMock( TitleFormatter::class ),
$this->createNoOpMock( Language::class ),
$this->createNoOpMock( GenderCache::class ),
$this->createMock( ILoadBalancer::class ),
$this->createMock( IConnectionProvider::class ),
$this->createMock( LinksMigration::class ),
LoggerFactory::getInstance( 'LinkBatch' )
);
@ -213,7 +213,7 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$this->createMock( TitleFormatter::class ),
$language,
$this->createNoOpMock( GenderCache::class ),
$this->createMock( ILoadBalancer::class ),
$this->createMock( IConnectionProvider::class ),
$this->createMock( LinksMigration::class ),
LoggerFactory::getInstance( 'LinkBatch' )
);
@ -237,7 +237,7 @@ class LinkBatchTest extends MediaWikiIntegrationTestCase {
$this->createMock( TitleFormatter::class ),
$language,
$genderCache,
$this->createMock( ILoadBalancer::class ),
$this->createMock( IConnectionProvider::class ),
$this->createMock( LinksMigration::class ),
LoggerFactory::getInstance( 'LinkBatch' )
);

View file

@ -14,7 +14,6 @@ use MediaWiki\User\UserIdentityValue;
use PHPUnit\Framework\MockObject\MockObject;
use Wikimedia\Rdbms\DBConnRef;
use Wikimedia\Rdbms\FakeResultWrapper;
use Wikimedia\Rdbms\ILoadBalancer;
use Wikimedia\Rdbms\IResultWrapper;
use Wikimedia\Rdbms\LBFactory;
use Wikimedia\Rdbms\SelectQueryBuilder;
@ -41,27 +40,6 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
return $mock;
}
/**
* @param DBConnRef $mockDb
* @param string|null $expectedConnectionType
* @return MockObject&ILoadBalancer
*/
private function getMockLoadBalancer(
DBConnRef $mockDb,
$expectedConnectionType = null
) {
$mock = $this->createMock( ILoadBalancer::class );
if ( $expectedConnectionType !== null ) {
$mock->method( 'getConnectionRef' )
->with( $expectedConnectionType )
->willReturn( $mockDb );
} else {
$mock->method( 'getConnectionRef' )
->willReturn( $mockDb );
}
return $mock;
}
/**
* @param DBConnRef $mockDb
* @param string|null $expectedConnectionType
@ -71,12 +49,13 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
DBConnRef $mockDb,
$expectedConnectionType = null
) {
$loadBalancer = $this->getMockLoadBalancer( $mockDb, $expectedConnectionType );
$mock = $this->createMock( LBFactory::class );
$mock->method( 'getLocalDomainID' )
->willReturn( 'phpunitdb' );
$mock->method( 'getMainLB' )
->willReturn( $loadBalancer );
$mock->method( 'getPrimaryDatabase' )
->willReturn( $mockDb );
$mock->method( 'getReplicaDatabase' )
->willReturn( $mockDb );
$mock->method( 'getLBsForOwner' )
->willReturn( [] );
return $mock;
@ -155,7 +134,7 @@ class WatchedItemStoreUnitTest extends MediaWikiIntegrationTestCase {
$this->createMock( TitleFormatter::class ),
$this->createMock( Language::class ),
$this->createMock( GenderCache::class ),
$this->getMockLoadBalancer( $mockDb ),
$this->getMockLBFactory( $mockDb ),
$this->createMock( LinksMigration::class ),
LoggerFactory::getInstance( 'LinkBatch' )
);

View file

@ -402,14 +402,12 @@ class CommentParserTest extends \MediaWikiIntegrationTestCase {
// LinkBatch does not provide a transparent read-through cache.
// TODO: Generic $this->assertQueryCount() would do the job.
$lbf = $services->getDBLoadBalancerFactory();
$fakeLB = $lbf->newMainLB();
$fakeLB->disable( __METHOD__ );
$linkBatchFactory = new LinkBatchFactory(
$services->getLinkCache(),
$services->getTitleFormatter(),
$services->getContentLanguage(),
$services->getGenderCache(),
$fakeLB,
$lbf,
$services->getLinksMigration(),
LoggerFactory::getInstance( 'LinkBatch' )
);

View file

@ -5,7 +5,7 @@ use MediaWiki\Linker\LinksMigration;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\Page\PageReference;
use MediaWiki\Page\PageReferenceValue;
use Wikimedia\Rdbms\ILoadBalancer;
use Wikimedia\Rdbms\IConnectionProvider;
/**
* @group Cache
@ -40,7 +40,7 @@ class LinkBatchFactoryTest extends MediaWikiUnitTestCase {
$this->createMock( TitleFormatter::class ),
$this->createMock( Language::class ),
$this->createMock( GenderCache::class ),
$this->createMock( ILoadBalancer::class ),
$this->createMock( IConnectionProvider::class ),
$this->createMock( LinksMigration::class ),
LoggerFactory::getInstance( 'LinkBatch' )
);