Skip to content

Commit

Permalink
Merge "TermsDomainDb: Avoid ConnectionManager & ReplicationWaiter"
Browse files Browse the repository at this point in the history
  • Loading branch information
jenkins-bot authored and Gerrit Code Review committed Dec 17, 2024
2 parents 794432c + 0f863e4 commit fe6fead
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 28 deletions.
15 changes: 10 additions & 5 deletions lib/includes/Rdbms/TermsDomainDb.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@

namespace Wikibase\Lib\Rdbms;

use Wikimedia\Rdbms\ConnectionManager;
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\ILoadBalancer;
use Wikimedia\Rdbms\IReadableDatabase;

/**
* Database abstraction to access terms (labels, descriptions, aliases) database tables created by the WikibaseRepository extension.
Expand All @@ -23,12 +24,16 @@ public function __construct( RepoDomainDb $repoDb ) {
$this->repoDb = $repoDb;
}

public function connections(): ConnectionManager {
return $this->repoDb->connections();
public function getWriteConnection( int $flags = 0 ): IDatabase {
return $this->repoDb->connections()->getWriteConnection( $flags );
}

public function replication(): ReplicationWaiter {
return $this->repoDb->replication();
public function getReadConnection( ?array $groups = null, int $flags = 0 ): IReadableDatabase {
return $this->repoDb->connections()->getReadConnection( $groups, $flags );
}

public function waitForReplicationOfAllAffectedClusters( ?int $timeout = null ): void {
$this->repoDb->replication()->waitForAllAffectedClusters( $timeout );
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,6 @@ private function getEntityId( object $termRow ): ?EntityId {
}

private function getDbr(): IReadableDatabase {
return $this->termsDb->connections()->getReadConnection();
return $this->termsDb->getReadConnection();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,7 @@ function ( $record, $idToRestore ) {
private function restoreCleanedUpIds( array $termsArray, array $termInLangIds = [] ) {
$uniqueTermIds = array_values( array_unique( $termInLangIds ) );

$dbMaster = $this->termsDb->connections()->getWriteConnection();
$dbMaster = $this->termsDb->getWriteConnection();
$persistedTermIds = $dbMaster->newSelectQueryBuilder()
->select( 'wbtl_id' )
->from( 'wbt_term_in_lang' )
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ private function lookupTypeIds( array $typeNames ) {
}

private function getDbr() {
return $this->termsDb->connections()->getReadConnection();
return $this->termsDb->getReadConnection();
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public function __construct(
}

private function getDbw(): IDatabase {
return $this->termsDb->connections()->getWriteConnection();
return $this->termsDb->getWriteConnection();
}

protected function delete( Int32EntityId $entityId ): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ public function __construct( TermsDomainDb $termsDb, DatabaseInnerTermStoreClean
*/
public function cleanTermInLangIds( array $termInLangIds ): void {

$dbw = $this->termsDb->connections()->getWriteConnection();
$dbr = $this->termsDb->connections()->getReadConnection();
$dbw = $this->termsDb->getWriteConnection();
$dbr = $this->termsDb->getReadConnection();

$dbw->startAtomic( __METHOD__ );
$unusedTermInLangIds = $this->findActuallyUnusedTermInLangIds( $termInLangIds, $dbw );
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ private function insertNonExistingRecords(
// gets stuck in an infinite loop. To avoid this, we read it with CONN_TRX_AUTOCOMMIT
// Surprisingly it's not too rare not to happen in production: T247553

$dbw = $this->termsDb->connections()->getWriteConnection( ILoadBalancer::CONN_TRX_AUTOCOMMIT );
$dbw = $this->termsDb->getWriteConnection( ILoadBalancer::CONN_TRX_AUTOCOMMIT );

$insertedRecords = array_merge(
$insertedRecords,
Expand Down Expand Up @@ -216,7 +216,7 @@ private function fetchExistingRecordsFromReplica( array $neededRecords ): array
// If not all needed records exist in replica,
// try wait for replication and fetch again from replica
if ( $neededRecords ) {
$this->termsDb->replication()->waitForAllAffectedClusters( $this->waitForReplicationTimeout );
$this->termsDb->waitForReplicationOfAllAffectedClusters( $this->waitForReplicationTimeout );

$existingRecords = array_merge(
$existingRecords,
Expand All @@ -230,11 +230,11 @@ private function fetchExistingRecordsFromReplica( array $neededRecords ): array
}

private function getDbReplica(): IReadableDatabase {
return $this->termsDb->connections()->getReadConnection();
return $this->termsDb->getReadConnection();
}

private function getDbPrimary(): IDatabase {
return $this->termsDb->connections()->getWriteConnection();
return $this->termsDb->getWriteConnection();
}

/**
Expand Down
53 changes: 45 additions & 8 deletions lib/tests/phpunit/Rdbms/TermsDomainDbTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Wikibase\Lib\Rdbms\RepoDomainDb;
use Wikibase\Lib\Rdbms\TermsDomainDb;
use Wikimedia\Rdbms\ConnectionManager;
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\ILoadBalancer;

/**
Expand All @@ -19,20 +20,56 @@
*/
class TermsDomainDbTest extends \PHPUnit\Framework\TestCase {

public function testConnections(): void {
$expected = $this->createStub( ConnectionManager::class );
public function testGetReadConnection(): void {
$loadGroups = [ 'some group' ];
$flags = 123;
$expected = $this->createStub( IDatabase::class );

$connectionManager = $this->createMock( ConnectionManager::class );
$connectionManager->expects( $this->once() )
->method( 'getReadConnection' )
->with( $loadGroups, $flags )
->willReturn( $expected );

$repoDomainDb = $this->createStub( RepoDomainDb::class );
$repoDomainDb->method( 'connections' )->willReturn( $expected );
$repoDomainDb->method( 'connections' )->willReturn( $connectionManager );

$this->assertSame( $expected, ( new TermsDomainDb( $repoDomainDb ) )->connections() );
$this->assertSame(
$expected,
( new TermsDomainDb( $repoDomainDb ) )->getReadConnection( $loadGroups, $flags )
);
}

public function testReplication(): void {
$expected = $this->createStub( ReplicationWaiter::class );
public function testGetWriteConnection(): void {
$flags = 321;
$expected = $this->createStub( IDatabase::class );

$connectionManager = $this->createMock( ConnectionManager::class );
$connectionManager->expects( $this->once() )
->method( 'getWriteConnection' )
->with( $flags )
->willReturn( $expected );

$repoDomainDb = $this->createStub( RepoDomainDb::class );
$repoDomainDb->method( 'connections' )->willReturn( $connectionManager );

$this->assertSame(
$expected,
( new TermsDomainDb( $repoDomainDb ) )->getWriteConnection( $flags )
);
}

public function testWaitForReplication(): void {
$timeout = 42;
$replicationWaiter = $this->createMock( ReplicationWaiter::class );
$replicationWaiter->expects( $this->once() )
->method( 'waitForAllAffectedClusters' )
->with( $timeout );

$repoDomainDb = $this->createStub( RepoDomainDb::class );
$repoDomainDb->method( 'replication' )->willReturn( $expected );
$repoDomainDb->method( 'replication' )->willReturn( $replicationWaiter );

$this->assertSame( $expected, ( new TermsDomainDb( $repoDomainDb ) )->replication() );
( new TermsDomainDb( $repoDomainDb ) )->waitForReplicationOfAllAffectedClusters( $timeout );
}

public function testLoadBalancer(): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ class EntityTermsSelectQueryBuilder extends SelectQueryBuilder {
private string $entityIdColumn;

public function __construct( TermsDomainDb $db, string $entityType ) {
parent::__construct( $db->connections()->getReadConnection() );
parent::__construct( $db->getReadConnection() );

if ( $entityType === Item::ENTITY_TYPE ) {
$this->entityTermsTable = 'wbt_item_terms';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
use PHPUnit\Framework\TestCase;
use Wikibase\Lib\Rdbms\TermsDomainDb;
use Wikibase\Repo\Store\Sql\Terms\EntityTermsSelectQueryBuilder;
use Wikimedia\Rdbms\ConnectionManager;
use Wikimedia\Rdbms\IExpression;

/**
Expand Down Expand Up @@ -124,10 +123,8 @@ public function testWhereMultiTerm(): void {

public function newTermsDb( MockDatabase $db = null ): TermsDomainDb {
$db ??= new MockDatabase();
$connectionManager = $this->createStub( ConnectionManager::class );
$connectionManager->method( 'getReadConnection' )->willReturn( $db );
$termsDb = $this->createStub( TermsDomainDb::class );
$termsDb->method( 'connections' )->willReturn( $connectionManager );
$termsDb->method( 'getReadConnection' )->willReturn( $db );

return $termsDb;
}
Expand Down

0 comments on commit fe6fead

Please sign in to comment.