Skip to content

Commit

Permalink
Merge "Add virtual-wikibase-terms virtual domain"
Browse files Browse the repository at this point in the history
  • Loading branch information
jenkins-bot authored and Gerrit Code Review committed Jan 28, 2025
2 parents 7e656b9 + bbbf1d9 commit ad19f41
Show file tree
Hide file tree
Showing 12 changed files with 174 additions and 28 deletions.
10 changes: 9 additions & 1 deletion client/WikibaseClient.ServiceWiring.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
use DataValues\UnknownValue;
use MediaWiki\Language\Language;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MainConfigNames;
use MediaWiki\MediaWikiServices;
use MediaWiki\Registration\ExtensionRegistry;
use MediaWiki\Site\MediaWikiSite;
Expand Down Expand Up @@ -103,6 +104,7 @@
use Wikibase\Lib\Rdbms\DomainDb;
use Wikibase\Lib\Rdbms\RepoDomainDbFactory;
use Wikibase\Lib\Rdbms\TermsDomainDbFactory;
use Wikibase\Lib\Rdbms\VirtualTermsDomainDb;
use Wikibase\Lib\ServiceBySourceAndTypeDispatcher;
use Wikibase\Lib\SettingsArray;
use Wikibase\Lib\Store\CachingPropertyInfoLookup;
Expand Down Expand Up @@ -1015,7 +1017,13 @@ function ( EntityNamespaceLookup $nsLookup, DatabaseEntitySource $source ): Enti
},

'WikibaseClient.TermsDomainDbFactory' => function ( MediaWikiServices $services ): TermsDomainDbFactory {
return new TermsDomainDbFactory( WikibaseClient::getRepoDomainDbFactory( $services ) );
$virtualDomainsMapping = $services->getMainConfig()->get( MainConfigNames::VirtualDomainsMapping );

return new TermsDomainDbFactory(
isset( $virtualDomainsMapping[VirtualTermsDomainDb::VIRTUAL_DOMAIN_ID] ),
$services->getDBLoadBalancerFactory(),
WikibaseClient::getRepoDomainDbFactory( $services )
);
},

'WikibaseClient.TermsLanguages' => function ( MediaWikiServices $services ): ContentLanguages {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ protected static function getBadPatternsWithAllowedUsages(): array {
'tests/phpunit/unit/includes/ServiceWiringTestCase.php' => true, // mock
],
'/\b((get)?(DB)?|I|)LoadBalancer(Factory)?(?!::|;)/' => [
'WikibaseClient.ServiceWiring.php' => 2, // RepoDomainDbFactory+ClientDomainDbFactory service wiring
'WikibaseClient.ServiceWiring.php' => 3, // RepoDomainDbFactory+ClientDomainDbFactory+TermsDomainDbFactory service wiring
'tests/phpunit/integration/includes/RecentChanges/RecentChangesFinderTest.php' => true, // TODO migrate?
'tests/phpunit/integration/includes/Usage/Sql/SqlSubscriptionManagerTest.php' => true, // TODO migrate?
'tests/phpunit/integration/includes/Usage/Sql/SqlUsageTrackerTest.php' => true, // TODO migrate?
Expand Down
3 changes: 3 additions & 0 deletions extension-repo.json
Original file line number Diff line number Diff line change
Expand Up @@ -1317,5 +1317,8 @@
},
"callback": "\\Wikibase\\Repo\\RepoHooks::onRegistration",
"load_composer_autoloader": true,
"DatabaseVirtualDomains": [
"virtual-wikibase-terms"
],
"manifest_version": 2
}
19 changes: 16 additions & 3 deletions lib/includes/Rdbms/TermsDomainDbFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,37 @@
namespace Wikibase\Lib\Rdbms;

use Wikibase\DataAccess\DatabaseEntitySource;
use Wikimedia\Rdbms\ILBFactory;

/**
* @license GPL-2.0-or-later
*/
class TermsDomainDbFactory {

private bool $hasVirtualTermsDomain;
private ILBFactory $lbFactory;
private RepoDomainDbFactory $repoDomainDbFactory;

public function __construct( RepoDomainDbFactory $repoDomainDbFactory ) {
public function __construct(
bool $hasVirtualTermsDomain,
ILBFactory $lbFactory,
RepoDomainDbFactory $repoDomainDbFactory
) {
$this->hasVirtualTermsDomain = $hasVirtualTermsDomain;
$this->lbFactory = $lbFactory;
$this->repoDomainDbFactory = $repoDomainDbFactory;
}

public function newTermsDb(): TermsDomainDb {
return new RepoDomainTermsDb( $this->repoDomainDbFactory->newRepoDb() );
return $this->hasVirtualTermsDomain ?
new VirtualTermsDomainDb( $this->lbFactory ) :
new RepoDomainTermsDb( $this->repoDomainDbFactory->newRepoDb() );
}

public function newForEntitySource( DatabaseEntitySource $entitySource ): TermsDomainDb {
return new RepoDomainTermsDb( $this->repoDomainDbFactory->newForEntitySource( $entitySource ) );
return $this->hasVirtualTermsDomain ?
new VirtualTermsDomainDb( $this->lbFactory ) :
new RepoDomainTermsDb( $this->repoDomainDbFactory->newForEntitySource( $entitySource ) );
}

}
43 changes: 43 additions & 0 deletions lib/includes/Rdbms/VirtualTermsDomainDb.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<?php

declare( strict_types=1 );

namespace Wikibase\Lib\Rdbms;

use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\ILBFactory;
use Wikimedia\Rdbms\IReadableDatabase;

/**
* Accesses terms (labels, descriptions, aliases) database tables via virtual domain.
*
* @license GPL-2.0-or-later
*/
class VirtualTermsDomainDb implements TermsDomainDb {

public const VIRTUAL_DOMAIN_ID = 'virtual-wikibase-terms';

private ILBFactory $lbFactory;

public function __construct( ILBFactory $lbFactory ) {
$this->lbFactory = $lbFactory;
}

public function getWriteConnection(): IDatabase {
return $this->lbFactory->getPrimaryDatabase( self::VIRTUAL_DOMAIN_ID );
}

public function getAutoCommitPrimaryConnection(): IDatabase {
return $this->lbFactory->getAutoCommitPrimaryConnection( self::VIRTUAL_DOMAIN_ID );
}

public function getReadConnection( ?array $groups = null ): IReadableDatabase {
return $this->lbFactory->getReplicaDatabase( self::VIRTUAL_DOMAIN_ID );
}

public function waitForReplicationOfAllAffectedClusters( ?int $timeout = null ): void {
$this->lbFactory->waitForReplication( array_filter( [
'timeout' => $timeout,
] ) );
}
}
19 changes: 15 additions & 4 deletions lib/includes/Store/Sql/Terms/DatabaseTermStoreWriterBase.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
use Wikibase\Lib\Rdbms\TermsDomainDb;
use Wikibase\Lib\Store\Sql\Terms\Util\StatsMonitoring;
use Wikibase\Lib\StringNormalizer;
use Wikimedia\Rdbms\DBReadOnlyError;
use Wikimedia\Rdbms\IDatabase;

/**
Expand Down Expand Up @@ -54,13 +55,23 @@ private function getDbw(): IDatabase {
}

protected function delete( Int32EntityId $entityId ): void {
$termInLangIdsToClean = $this->deleteTermsWithoutClean( $entityId );
$this->submitJobToCleanTermStorageRowsIfUnused( $termInLangIdsToClean );
try {
$termInLangIdsToClean = $this->deleteTermsWithoutClean( $entityId );
$this->submitJobToCleanTermStorageRowsIfUnused( $termInLangIdsToClean );
} catch ( DBReadOnlyError $e ) {
// The terms DB may be different from the repo wiki's main DB, and an external DB's read-only state is not handled as
// gracefully. There is nothing to do here, though. The secondary terms storage will fix itself eventually.
}
}

protected function store( Int32EntityId $entityId, Fingerprint $fingerprint ): void {
$termInLangIdsToClean = $this->acquireAndInsertTerms( $entityId, $fingerprint );
$this->submitJobToCleanTermStorageRowsIfUnused( $termInLangIdsToClean );
try {
$termInLangIdsToClean = $this->acquireAndInsertTerms( $entityId, $fingerprint );
$this->submitJobToCleanTermStorageRowsIfUnused( $termInLangIdsToClean );
} catch ( DBReadOnlyError $e ) {
// The terms DB may be different from the repo wiki's main DB, and an external DB's read-only state is not handled as
// gracefully. There is nothing to do here, though. The secondary terms storage will fix itself eventually.
}
}

private function submitJobToCleanTermStorageRowsIfUnused( array $termInLangIdsToClean ): void {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Wikibase\Lib\Store\Sql\Terms;

use Wikibase\Lib\Rdbms\TermsDomainDb;
use Wikimedia\Rdbms\DBReadOnlyError;

/**
* @license GPL-2.0-or-later
Expand Down Expand Up @@ -39,13 +40,17 @@ public function __construct( TermsDomainDb $termsDb, DatabaseInnerTermStoreClean
* @param array $termInLangIds
*/
public function cleanTermInLangIds( array $termInLangIds ): void {

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

$dbw->startAtomic( __METHOD__ );
$unusedTermInLangIds = $this->findActuallyUnusedTermInLangIds( $termInLangIds, $dbw );
$this->innerCleaner->cleanTermInLangIds( $dbw, $dbr, $unusedTermInLangIds );
$dbw->endAtomic( __METHOD__ );
try {
$dbw->startAtomic( __METHOD__ );
$unusedTermInLangIds = $this->findActuallyUnusedTermInLangIds( $termInLangIds, $dbw );
$this->innerCleaner->cleanTermInLangIds( $dbw, $dbr, $unusedTermInLangIds );
$dbw->endAtomic( __METHOD__ );
} catch ( DBReadOnlyError $error ) {
// The terms DB may be different from the repo wiki's main DB, and an external DB's read-only state is not handled as
// gracefully. There is nothing to do here, though. The secondary terms storage will fix itself eventually.
}
}
}
7 changes: 6 additions & 1 deletion lib/tests/phpunit/Rdbms/LocalRepoDbTestHelper.php
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use Wikibase\Lib\Rdbms\RepoDomainDbFactory;
use Wikibase\Lib\Rdbms\RepoDomainTermsDb;
use Wikibase\Lib\Rdbms\TermsDomainDbFactory;
use Wikibase\Lib\Tests\Store\Sql\Terms\Util\FakeLBFactory;
use Wikimedia\Rdbms\IDatabase;
use Wikimedia\Rdbms\LBFactorySingle;

Expand Down Expand Up @@ -40,7 +41,11 @@ public function getRepoDomainDbFactory( ?IDatabase $db = null ): RepoDomainDbFac
}

public function getTermsDomainDbFactory( ?IDatabase $db = null ): TermsDomainDbFactory {
return new TermsDomainDbFactory( $this->getRepoDomainDbFactory( $db ) );
return new TermsDomainDbFactory(
false,
new FakeLBFactory( [ 'lb' => null ] ),
$this->getRepoDomainDbFactory( $db )
);
}

}
61 changes: 53 additions & 8 deletions lib/tests/phpunit/Rdbms/TermsDomainDbFactoryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use Wikibase\Lib\Rdbms\RepoDomainDbFactory;
use Wikibase\Lib\Rdbms\RepoDomainTermsDb;
use Wikibase\Lib\Rdbms\TermsDomainDbFactory;
use Wikibase\Lib\Rdbms\VirtualTermsDomainDb;
use Wikimedia\Rdbms\LBFactory;

/**
* @covers \Wikibase\Lib\Rdbms\TermsDomainDbFactory
Expand All @@ -19,30 +21,73 @@
*/
class TermsDomainDbFactoryTest extends \PHPUnit\Framework\TestCase {

public function testNewTermsDb(): void {
$repoDbFactory = $this->createMock( RepoDomainDbFactory::class );
$repoDbFactory->expects( $this->once() )
private bool $hasVirtualTermsDomain;
private LBFactory $lbFactory;
private RepoDomainDbFactory $repoDbFactory;

protected function setUp(): void {
parent::setUp();

$this->hasVirtualTermsDomain = false;
$this->lbFactory = $this->createStub( LBFactory::class );
$this->repoDbFactory = $this->createStub( RepoDomainDbFactory::class );
}

public function testGivenNoVirtualTermsDomain_newTermsDbReturnsRepoDomainTermsDb(): void {
$this->hasVirtualTermsDomain = false;

$this->repoDbFactory = $this->createMock( RepoDomainDbFactory::class );
$this->repoDbFactory->expects( $this->once() )
->method( 'newRepoDb' )
->willReturn( $this->createStub( RepoDomainDb::class ) );

$this->assertInstanceOf(
RepoDomainTermsDb::class,
( new TermsDomainDbFactory( $repoDbFactory ) )->newTermsDb()
$this->newFactory()->newTermsDb()
);
}

public function testNewForEntitySource(): void {
public function testGivenNoVirtualTermsDomain_newForEntitySourceReturnsRepoDomainTermsDb(): void {
$entitySource = $this->createStub( DatabaseEntitySource::class );
$repoDbFactory = $this->createMock( RepoDomainDbFactory::class );
$repoDbFactory->expects( $this->once() )
$this->repoDbFactory = $this->createMock( RepoDomainDbFactory::class );
$this->repoDbFactory->expects( $this->once() )
->method( 'newForEntitySource' )
->with( $entitySource )
->willReturn( $this->createStub( RepoDomainDb::class ) );

$this->assertInstanceOf(
RepoDomainTermsDb::class,
( new TermsDomainDbFactory( $repoDbFactory ) )->newForEntitySource( $entitySource )
$this->newFactory()->newForEntitySource( $entitySource )
);
}

public function testGivenVirtualTermsDomain_newTermsDbReturnsVirtualTermsDomainDb(): void {
$this->hasVirtualTermsDomain = true;

$this->repoDbFactory = $this->createMock( RepoDomainDbFactory::class );
$this->repoDbFactory->expects( $this->never() )->method( $this->anything() );

$this->assertInstanceOf(
VirtualTermsDomainDb::class,
$this->newFactory()->newTermsDb()
);
}

public function testGivenVirtualTermsDomain_newForEntitySourceReturnsVirtualTermsDomainDb(): void {
$this->hasVirtualTermsDomain = true;
$entitySource = $this->createStub( DatabaseEntitySource::class );

$this->repoDbFactory = $this->createMock( RepoDomainDbFactory::class );
$this->repoDbFactory->expects( $this->never() )->method( $this->anything() );

$this->assertInstanceOf(
VirtualTermsDomainDb::class,
$this->newFactory()->newForEntitySource( $entitySource )
);
}

private function newFactory(): TermsDomainDbFactory {
return new TermsDomainDbFactory( $this->hasVirtualTermsDomain, $this->lbFactory, $this->repoDbFactory );
}

}
10 changes: 9 additions & 1 deletion repo/WikibaseRepo.ServiceWiring.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
use MediaWiki\Context\RequestContext;
use MediaWiki\Language\Language;
use MediaWiki\Logger\LoggerFactory;
use MediaWiki\MainConfigNames;
use MediaWiki\MediaWikiServices;
use MediaWiki\Registration\ExtensionRegistry;
use MediaWiki\Site\MediaWikiPageNameNormalizer;
Expand Down Expand Up @@ -93,6 +94,7 @@
use Wikibase\Lib\Rdbms\DomainDb;
use Wikibase\Lib\Rdbms\RepoDomainDbFactory;
use Wikibase\Lib\Rdbms\TermsDomainDbFactory;
use Wikibase\Lib\Rdbms\VirtualTermsDomainDb;
use Wikibase\Lib\ServiceBySourceAndTypeDispatcher;
use Wikibase\Lib\SettingsArray;
use Wikibase\Lib\SimpleCacheWithBagOStuff;
Expand Down Expand Up @@ -1965,7 +1967,13 @@ function ( $types, $localTypeName ) use ( $subEntityTypes ) {
},

'WikibaseRepo.TermsDomainDbFactory' => function ( MediaWikiServices $services ): TermsDomainDbFactory {
return new TermsDomainDbFactory( WikibaseRepo::getRepoDomainDbFactory( $services ) );
$virtualDomainsMapping = $services->getMainConfig()->get( MainConfigNames::VirtualDomainsMapping );

return new TermsDomainDbFactory(
isset( $virtualDomainsMapping[VirtualTermsDomainDb::VIRTUAL_DOMAIN_ID] ),
$services->getDBLoadBalancerFactory(),
WikibaseRepo::getRepoDomainDbFactory( $services )
);
},

'WikibaseRepo.TermsLanguages' => function ( MediaWikiServices $services ): ContentLanguages {
Expand Down
11 changes: 8 additions & 3 deletions repo/includes/Store/Sql/DatabaseSchemaUpdater.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Wikibase\DataModel\Services\Lookup\LegacyAdapterItemLookup;
use Wikibase\DataModel\Services\Lookup\LegacyAdapterPropertyLookup;
use Wikibase\DataModel\Term\TermTypes;
use Wikibase\Lib\Rdbms\VirtualTermsDomainDb;
use Wikibase\Lib\Store\Sql\Terms\TermTypeIds;
use Wikibase\Repo\RangeTraversable;
use Wikibase\Repo\Store\ItemTermsRebuilder;
Expand Down Expand Up @@ -63,10 +64,14 @@ public function onLoadExtensionSchemaUpdates( $updater ) {
$updater->dropExtensionTable( 'wb_entity_per_page' );
}

$updater->addExtensionTable(
$updater->addExtensionUpdateOnVirtualDomain( [
VirtualTermsDomainDb::VIRTUAL_DOMAIN_ID,
'addTable',
'wbt_text',
$this->getScriptPath( 'term_store', $db->getType() )
);
$this->getScriptPath( 'term_store', $db->getType() ),
true,
] );

if ( !$updater->updateRowExists( __CLASS__ . '::rebuildPropertyTerms' ) ) {
$updater->addExtensionUpdate( [
[ __CLASS__, 'rebuildPropertyTerms' ],
Expand Down
2 changes: 1 addition & 1 deletion repo/tests/phpunit/unit/RepoNoBadUsageTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ protected static function getBadPatternsWithAllowedUsages(): array {
'tests/phpunit/unit/ServiceWiringTestCase.php' => true, // mock
],
'/\b((get)?(DB)?|I|)LoadBalancer(Factory)?(?!::|;)/' => [
'WikibaseRepo.ServiceWiring.php' => 1, // RepoDomainDbFactory service wiring
'WikibaseRepo.ServiceWiring.php' => 2, // RepoDomainDbFactory+TermsDomainDbFactory service wiring
'tests/phpunit/includes/Store/Sql/WikiPageEntityMetaDataLookupTest.php' => true, // mock
'tests/phpunit/unit/ServiceWiringTestCase.php' => true, // mock
],
Expand Down

0 comments on commit ad19f41

Please sign in to comment.