From 8699b3c01bcda6c023ff292cf1e39d0438d58c02 Mon Sep 17 00:00:00 2001 From: addshore Date: Sat, 26 Sep 2026 01:37:01 +0100 Subject: [PATCH] perf: query platform wiki entity counts directly Improves some SQL queries as part of T424505, but the main changes is the removal of API calls entirely. Replace per-wiki API requests with a single direct database query and avoid WikiDb N+1 lookups when preparing platform statistics. Restore PlatformStatsSummaryJob integration coverage by isolating non-transactional MediaWiki schema and user provisioning from Laravel's RefreshDatabase transaction. Looking by eye, at https://www.wikibase.cloud/discovery there are certianly 7+ million "pages" that are on cloud Which would reuslt in 14k http requests being made probably with 500 results per page (lots of overheads) The description of the origional ticket at https://phabricator.wikimedia.org/T314481 links to https://gerrit.wikimedia.org/g/analytics/wmde/scripts/+/3c0f9791912774bae41546412a048d2019b74b4b/src/wikidata/site_stats/pages_by_namespace.php and nearby https://gerrit.wikimedia.org/r/plugins/gitiles/analytics/wmde/scripts/+/3c0f9791912774bae41546412a048d2019b74b4b/src/wikidata/site_stats/sql/select_pages_by_namespace.sql which shows how wikidata has been doing this for some time. When testing locally on a page table with 1 million rows the qury still returns quickly, and I expect this will also be quick enough (and better than the API calls) in wb cloud. Another thing to consider here is that this single query is getting rather large, and that group_concat_max_len might be reached depending on the prod setup, if you are not there already, your liekly getting there soon. The query could be split easily into appropriate sized chunks and join the results later, OR increase group_concat_max_len Bug: T314481 Bug: T424505 --- app/Jobs/PlatformStatsSummaryJob.php | 47 +++---- tests/Jobs/PlatformStatsSummaryJobTest.php | 151 ++++++++++----------- 2 files changed, 88 insertions(+), 110 deletions(-) diff --git a/app/Jobs/PlatformStatsSummaryJob.php b/app/Jobs/PlatformStatsSummaryJob.php index f8c66dc46..8cccc0607 100644 --- a/app/Jobs/PlatformStatsSummaryJob.php +++ b/app/Jobs/PlatformStatsSummaryJob.php @@ -2,10 +2,7 @@ namespace App\Jobs; -use App\Constants\MediawikiNamespace; use App\Helper\MWTimestampHelper; -use App\Services\MediaWikiHostResolver; -use App\Traits; use App\User; use App\Wiki; use Carbon\CarbonImmutable; @@ -31,8 +28,6 @@ * Example: php artisan job:dispatch PlatformStatsSummaryJob */ class PlatformStatsSummaryJob extends Job { - use Traits\PageFetcher; - public $timeout = 3600; private $inactiveThreshold; @@ -41,8 +36,6 @@ class PlatformStatsSummaryJob extends Job { private $platformSummaryStatsVersion = 'v1'; - private MediaWikiHostResolver $mwHostResolver; - public function __construct() { $this->inactiveThreshold = Config::get('wbstack.platform_summary_inactive_threshold'); $this->creationRateRanges = Config::get('wbstack.platform_summary_creation_rate_ranges'); @@ -78,6 +71,7 @@ public function prepareStats(array $allStats, $wikis): array { $propertiesCount = []; $currentTime = CarbonImmutable::now(); + $statsByWiki = array_column($allStats, null, 'wiki'); foreach ($wikis as $wiki) { if (!is_null($wiki->deleted_at)) { @@ -86,23 +80,7 @@ public function prepareStats(array $allStats, $wikis): array { continue; } - $this->apiUrl = $this->mwHostResolver->getBackendUrlForDomain($wiki->domain) . '/w/api.php'; // used in PageFetcher::fetchPagesInNamespace - - // add items and properties counts of the wiki to the corresponded arrays - try { - $nextItemCount = count($this->fetchPagesInNamespace($wiki->domain, MediawikiNamespace::item)); - array_push($itemsCount, $nextItemCount); - } catch (\Exception $ex) { - Log::warning('Failed to fetch item count for wiki ' . $wiki->domain . ', will use 0 instead.'); - } - try { - $nextPropertyCount = count($this->fetchPagesInNamespace($wiki->domain, MediawikiNamespace::property)); - array_push($propertiesCount, $nextPropertyCount); - } catch (\Exception $ex) { - Log::warning('Failed to fetch property count for wiki ' . $wiki->domain . ', will use 0 instead.'); - } - - $wikiDb = $wiki->wikiDb()->first(); + $wikiDb = $wiki->wikiDb; if (!$wikiDb) { Log::error(__METHOD__ . ": Could not find WikiDB for {$wiki->domain}"); @@ -110,15 +88,16 @@ public function prepareStats(array $allStats, $wikis): array { continue; } - $found_key = array_search($wiki->domain, array_column($allStats, 'wiki')); + $stats = $statsByWiki[$wiki->domain] ?? null; - if ($found_key === false) { + if ($stats === null) { Log::warning(__METHOD__ . ": Could not find stats for {$wiki->domain}"); continue; } - $stats = $allStats[$found_key]; + $itemsCount[] = $stats['items']; + $propertiesCount[] = $stats['properties']; // is it empty? if ($this->isNullOrEmpty($stats['edits']) && $this->isNullOrEmpty($stats['pages']) && $this->isNullOrEmpty($stats['lastEdit'])) { @@ -170,9 +149,8 @@ public function prepareStats(array $allStats, $wikis): array { ]; } - public function handle(DatabaseManager $manager, MediaWikiHostResolver $mwHostResolver): void { - $this->mwHostResolver = $mwHostResolver; - $wikis = Wiki::withTrashed()->with('wikidb')->get(); + public function handle(DatabaseManager $manager): void { + $wikis = Wiki::withTrashed()->with('wikiDb')->get(); $conn = $manager->connection('mysql'); $mwConn = $manager->connection('mw'); @@ -232,8 +210,15 @@ public function handle(DatabaseManager $manager, MediaWikiHostResolver $mwHostRe WHERE ct_rev_id < 100 ) t4, ( +SELECT + COUNT(CASE WHEN page_namespace = 120 THEN 1 END) AS items, + COUNT(CASE WHEN page_namespace = 122 THEN 1 END) AS properties +FROM ",wiki_dbs.name,".",wiki_dbs.prefix,"_page +WHERE page_namespace IN (120, 122) +) t5, + ( SELECT '",wikis.domain,"' as wiki -) t5 +) t6 " ) SEPARATOR ' UNION ALL ') diff --git a/tests/Jobs/PlatformStatsSummaryJobTest.php b/tests/Jobs/PlatformStatsSummaryJobTest.php index c0b310c1c..d3524063c 100644 --- a/tests/Jobs/PlatformStatsSummaryJobTest.php +++ b/tests/Jobs/PlatformStatsSummaryJobTest.php @@ -2,7 +2,6 @@ namespace Tests\Jobs; -use App\Constants\MediawikiNamespace; use App\Helper\MWTimestampHelper; use App\Jobs\PlatformStatsSummaryJob; use App\Jobs\ProvisionWikiDbJob; @@ -28,6 +27,8 @@ class PlatformStatsSummaryJobTest extends TestCase { private $users = []; + private $provisionedWikiDbs = []; + private $db_prefix = 'somecoolprefix'; private $db_name = 'some_cool_db_name'; @@ -39,10 +40,11 @@ class PlatformStatsSummaryJobTest extends TestCase { protected function setUp(): void { parent::setUp(); for ($n = 0; $n < $this->numWikis; $n++) { - DB::connection('mysql')->getPdo()->exec("DROP DATABASE IF EXISTS {$this->db_name}{$n};"); + DB::connection('mw')->getPdo()->exec("DROP DATABASE IF EXISTS {$this->db_name}{$n};"); } $this->wikis = []; $this->users = []; + $this->provisionedWikiDbs = []; $this->mwBackendHost = 'http://mediawiki.localhost'; @@ -53,13 +55,18 @@ protected function setUp(): void { } protected function tearDown(): void { - Wiki::query()->delete(); - User::query()->delete(); - WikiManager::query()->delete(); - WikiDb::query()->delete(); + $this->cleanupProvisionedWikiDbs(); parent::tearDown(); } + private function cleanupProvisionedWikiDbs(): void { + $pdo = DB::connection('mw')->getPdo(); + foreach ($this->provisionedWikiDbs as $wikiDb) { + $pdo->exec("DROP DATABASE IF EXISTS `{$wikiDb->name}`"); + $pdo->exec('DROP USER IF EXISTS ' . $pdo->quote($wikiDb->user) . "@'%'"); + } + } + private function seedWikis() { $manager = $this->app->make('db'); for ($n = 0; $n < $this->numWikis; $n++) { @@ -76,13 +83,12 @@ private function seedWikis() { $this->wikis[] = $wiki; $this->users[] = $user; + $this->provisionedWikiDbs[] = $wikiDb; } } public function testQueryGetsStats() { - $this->markTestSkipped('Pollutes the deleted wiki list'); - Http::fake(); $this->seedWikis(); $manager = $this->app->make('db'); @@ -91,12 +97,11 @@ public function testQueryGetsStats() { $job = new PlatformStatsSummaryJob(); $job->setJob($mockJob); - - $job->handle($manager, $this->mockMwHostResolver); + Http::preventStrayRequests(); + $job->handle($manager); } public function testGroupings() { - $this->markTestSkipped('Pollutes the deleted wiki list'); $mockJob = $this->createMock(Job::class); $mockJob->expects($this->never())->method('fail'); @@ -120,42 +125,6 @@ public function testGroupings() { 'prefix' => 'asdasd', 'wiki_id' => $wiki->id, ]); - // Generate some items/properties for testing, each wiki will have 3 props and 9 items - Http::fake([ - $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=122&apcontinue=&aplimit=max&format=json' => Http::response([ - 'query' => [ - 'allpages' => [ - ['title' => 'Property:P1', 'namespace' => MediawikiNamespace::property], - ['title' => 'Property:P9', 'namespace' => MediawikiNamespace::property], - ['title' => 'Property:P11', 'namespace' => MediawikiNamespace::property], - ], - ], - ], 200), - $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=&aplimit=max&format=json' => Http::response([ - 'continue' => [ - 'apcontinue' => 'Q6', - ], - 'query' => [ - 'allpages' => [ - ['title' => 'Item:Q1', 'namespace' => MediawikiNamespace::item], - ['title' => 'Item:Q2', 'namespace' => MediawikiNamespace::item], - ['title' => 'Item:Q3', 'namespace' => MediawikiNamespace::item], - ['title' => 'Item:Q4', 'namespace' => MediawikiNamespace::item], - ['title' => 'Item:Q5', 'namespace' => MediawikiNamespace::item], - ], - ], - ], 200), - $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=Q6&aplimit=max&format=json' => Http::response([ - 'query' => [ - 'allpages' => [ - ['title' => 'Item:Q6', 'namespace' => MediawikiNamespace::item], - ['title' => 'Item:Q7', 'namespace' => MediawikiNamespace::item], - ['title' => 'Item:Q8', 'namespace' => MediawikiNamespace::item], - ['title' => 'Item:Q9', 'namespace' => MediawikiNamespace::item], - ], - ], - ], 200), - ]); } $stats = [ @@ -166,6 +135,8 @@ public function testGroupings() { 'users' => 1, 'active_users' => null, 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()->subDays(100)), + 'items' => 9, + 'properties' => 3, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ], @@ -176,6 +147,8 @@ public function testGroupings() { 'users' => 1, 'active_users' => null, 'lastEdit' => null, + 'items' => 9, + 'properties' => 3, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ], @@ -186,6 +159,8 @@ public function testGroupings() { 'users' => null, 'active_users' => null, 'lastEdit' => null, + 'items' => 9, + 'properties' => 3, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ], @@ -197,6 +172,8 @@ public function testGroupings() { 'users' => 3, 'active_users' => 1, 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()), + 'items' => 9, + 'properties' => 3, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ], @@ -208,6 +185,8 @@ public function testGroupings() { 'users' => 3, 'active_users' => 0, 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()), + 'items' => 9, + 'properties' => 3, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ], @@ -234,27 +213,14 @@ public function testGroupings() { ); } - public function testSkipDeletedWikisBeforeResolvingBackendUrl() { + public function testSkipDeletedWikis() { $deletedWiki = Wiki::factory()->create(['deleted_at' => CarbonImmutable::yesterday(), 'domain' => 'deleted.cloud']); WikiDb::factory()->for($deletedWiki)->create(['name' => 'deleted_db']); $activeWiki = Wiki::factory()->create(['deleted_at' => null, 'domain' => 'active.cloud']); WikiDb::factory()->for($activeWiki)->create(['name' => 'active_db']); - $this->mockMwHostResolver - ->expects($this->once()) - ->method('getBackendUrlForDomain') - ->with('active.cloud') - ->willReturn($this->mwBackendHost); - $job = new PlatformStatsSummaryJob(); - // This is a hack to override the `private` `PlatformStatsSummaryJob::mwHostResolver` property. - // See https://www.php.net/manual/en/closure.call.php for more details on how this works. - // TODO: figure out how to stub the `DatabaseManager` correctly and/or refactor the Job so that - // we can more easily inject dependencies in the tests. - (function ($resolver): void { - $this->mwHostResolver = $resolver; - })->call($job, $this->mockMwHostResolver); $groups = $job->prepareStats([ [ @@ -264,6 +230,8 @@ public function testSkipDeletedWikisBeforeResolvingBackendUrl() { 'users' => 1, 'active_users' => 1, 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()), + 'items' => 0, + 'properties' => 0, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ], @@ -274,6 +242,8 @@ public function testSkipDeletedWikisBeforeResolvingBackendUrl() { 'users' => 1, 'active_users' => 1, 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()), + 'items' => 0, + 'properties' => 0, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ], @@ -283,8 +253,46 @@ public function testSkipDeletedWikisBeforeResolvingBackendUrl() { $this->assertSame(1, $groups['edited_last_90_days']); } + public function testPrepareStatsDoesNotQueryEagerLoadedWikiDbs(): void { + $wikiCount = 25; + $itemsPerWiki = 10; + $propertiesPerWiki = 3; + $wikis = Wiki::factory()->count($wikiCount)->create(['deleted_at' => null]); + foreach ($wikis as $wiki) { + WikiDb::factory()->for($wiki)->create(); + } + + $wikis = Wiki::with('wikiDb')->get(); + $stats = $wikis->map(fn (Wiki $wiki) => [ + 'wiki' => $wiki->domain, + 'edits' => 1, + 'pages' => 1, + 'users' => 1, + 'active_users' => 1, + 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon(CarbonImmutable::now()), + 'items' => $itemsPerWiki, + 'properties' => $propertiesPerWiki, + ])->all(); + + $job = new PlatformStatsSummaryJob(); + Http::preventStrayRequests(); + + $connection = DB::connection('mysql'); + $connection->flushQueryLog(); + $connection->enableQueryLog(); + $groups = $job->prepareStats($stats, $wikis); + $queries = $connection->getQueryLog(); + $connection->disableQueryLog(); + + $wikiDbQueries = array_filter($queries, fn (array $query) => str_contains($query['query'], 'from `wiki_dbs`')); + + $this->assertSame($wikiCount, $groups['edited_last_90_days']); + $this->assertSame($wikiCount * $itemsPerWiki, $groups['total_items_count']); + $this->assertSame($wikiCount * $propertiesPerWiki, $groups['total_properties_count']); + $this->assertCount(0, $wikiDbQueries); + } + public function testCreationStats() { - $this->markTestSkipped('Pollutes the deleted wiki list'); $mockJob = $this->createMock(Job::class); $mockJob->expects($this->never())->method('fail'); @@ -337,25 +345,8 @@ public function testPrepareStatsTreatsSecondPrecisionTimestampAtThresholdAsActiv 'wiki_id' => $wiki->id, ]); - Http::fake([ - $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=122&apcontinue=&aplimit=max&format=json' => Http::response([ - 'query' => ['allpages' => []], - ], 200), - $this->mwBackendHost . '/w/api.php?action=query&list=allpages&apnamespace=120&apcontinue=&aplimit=max&format=json' => Http::response([ - 'query' => ['allpages' => []], - ], 200), - ]); - $job = new PlatformStatsSummaryJob(); - // This is a hack to override the `private` `PlatformStatsSummaryJob::mwHostResolver` property. - // See https://www.php.net/manual/en/closure.call.php for more details on how this works. - // TODO: figure out how to stub the `DatabaseManager` correctly and/or refactor the Job so that - // we can more easily inject dependencies in the tests. - (function ($resolver): void { - $this->mwHostResolver = $resolver; - })->call($job, $this->mockMwHostResolver); - $groups = $job->prepareStats([ [ 'wiki' => 'thresholdtest.com', @@ -366,6 +357,8 @@ public function testPrepareStatsTreatsSecondPrecisionTimestampAtThresholdAsActiv 'lastEdit' => MWTimestampHelper::getMWTimestampFromCarbon( $currentTime->subSeconds(config('wbstack.platform_summary_inactive_threshold')) ), + 'items' => 0, + 'properties' => 0, 'first100UsingOauth' => '0', 'platform_summary_version' => 'v1', ],