Page MenuHomePhabricator

Migrate GUC to actor database table
Closed, ResolvedPublic

Description

Per T223406: Remove reference to fields replaced by the actor table from WMCS views.

Need to come up with a strategy for how to best handle this.

In particular, in a way that remains reasonably performant because the tool still hasn't recovered from the previous migration / T195515: GUC query performance regressed 100x from <3s to 80-300s.

Event Timeline

Krinkle created this task.May 27 2019, 6:09 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 27 2019, 6:09 PM
Krinkle triaged this task as High priority.May 27 2019, 6:09 PM
Krinkle added a subscriber: Anomie.May 27 2019, 6:31 PM

The way GUC currently works when processing a query:

  1. Create connection to s1.web.db.svc.eqiad.wmflabs.
  2. Get list of all wikis.
  3. For each db section ({section}.web.db.svc.eqiad.wmflabs) (GUC does dns resolve on the hostname, and re-uses db connections from other sections if share the same IP. Right now this means we will only ever establish one connection to the replicas as they're all cnames right now):
    1. Find wikis that have matching revisions.
  4. Query centralauth_p for local usernames.
  5. For each wiki with non-zero matching revisions, two queries:
    1. Query user data.
    2. Query up to 20 descending contributions from the specified source (last 1h of recent changes, any recent changes, any revision).

Example:

Debug log
--- 2) Get list of all wikis
SELECT * FROM `meta_p`.`wiki` WHERE is_closed = 0 LIMIT 1500;

-- 3A) Quering wikis on `s3.labsdb` for matching revisions
SELECT COUNT(rev_id) AS counter, 'abwiki' AS dbname FROM abwiki_p.revision_userindex WHERE rev_user_text = :user UNION ALL
  
UNION ALL SELECT COUNT(rev_id) AS counter, 'zuwiktionary' AS dbname FROM zuwiktionary_p.revision_userindex WHERE rev_user_text = :user
--- ⤷ +31.04s

--- 3A) Quering wikis on `s7.labsdb` for matching revisions
SELECT COUNT(rev_id) AS counter, 'arwiki' AS dbname FROM arwiki_p.revision_userindex WHERE rev_user_text = :user UNION ALL
  
UNION ALL SELECT COUNT(rev_id) AS counter, 'viwiki' AS dbname FROM viwiki_p.revision_userindex WHERE rev_user_text = :user
---  ⤷ +0.32s

--- 3A) Quering wikis on `s1.labsdb` for matching revisions
SELECT COUNT(rev_id) AS counter, 'enwiki' AS dbname FROM enwiki_p.revision_userindex WHERE rev_user_text = :user
--- 3A) Quering wikis on `s8.labsdb` for matching revisions
SELECT COUNT(rev_id) AS counter, 'wikidatawiki' AS dbname FROM wikidatawiki_p.revision_userindex WHERE rev_user_text = :user

  

--- 5A) Query user data for commons.wikimedia.org
SELECT `user_id`, `user_name` FROM `user` WHERE `user_name` = :user LIMIT 10;

--- 5B) Query contributions on commons.wikimedia.org
SELECT `comment_text`, `rev_timestamp`, `rev_minor_edit`, `rev_len`, `rev_id`, `rev_parent_id`, `rev_user_text`, `page_title`, `page_namespace`, `page_latest` = `rev_id` AS `guc_is_cur` FROM `revision_userindex` INNER JOIN `page` ON `rev_page` = `page_id` LEFT OUTER JOIN `comment` ON `rev_comment_id` = `comment_id` WHERE `rev_deleted` = 0 AND `rev_user` = '607047' ORDER BY `rev_timestamp` DESC LIMIT 0, 20;

--- 5A) Query user data for en.wikipedia.org
SELECT `user_id`, `user_name` FROM `user` WHERE `user_name` = :user LIMIT 10;
--- 5B) Query contributions on en.wikipedia.org
SELECT `comment_text`, `rev_timestamp`, `rev_minor_edit`, `rev_len`, `rev_id`, `rev_parent_id`, `rev_user_text`, `page_title`, `page_namespace`, `page_latest` = `rev_id` AS `guc_is_cur` FROM `revision_userindex` INNER JOIN `page` ON `rev_page` = `page_id` LEFT OUTER JOIN `comment` ON `rev_comment_id` = `comment_id` WHERE `rev_deleted` = 0 AND `rev_user` = '9014223' ORDER BY `rev_timestamp` DESC LIMIT 0, 20;

  

--- Close remaining connection for s1.web.db.svc.eqiad.wmflabs
--- Connections opened: 1
--- Highest connection count: 1

Total backend time: 273.81s.

Debug log
--- 3A) Quering wikis on `s1.labsdb` for matching revisions
SELECT COUNT(rev_id) AS counter, 'abwiki' AS dbname FROM abwiki_p.revision_userindex WHERE rev_user_text = :user UNION ALL

This query would change to look something like

SELECT COUNT(rev_id) AS counter, 'enwiki' AS dbname FROM enwiki_p.actor JOIN enwiki_p.revision_userindex ON (rev_actor = actor_id) WHERE actor_name = :user

In a quick test using User:AnomieBOT on enwiki (counter = 3757214), your old query took 8 min 48.61 sec on one of the replicas while this new one took 9 min 6.83 sec on a different replica. The new query should improve to roughly match the old once T215466 happens, although that's at least a few months out.

However, you might wind up with the same sort of too many tables in the s3 join that you had in T195515, since the actor view has to join against many others for permissions checking just as the comment view does. If that does turn out to be the case, you might wind up having to look up the actor IDs first, individually. Or you could push for progress on T215445 to eliminate all those subqueries.

--- 5A) Query user data for en.wikipedia.org
SELECT `user_id`, `user_name` FROM `user` WHERE `user_name` = :user LIMIT 10;
--- 5B) Query contributions on en.wikipedia.org
SELECT `comment_text`, `rev_timestamp`, `rev_minor_edit`, `rev_len`, `rev_id`, `rev_parent_id`, `rev_user_text`, `page_title`, `page_namespace`, `page_latest` = `rev_id` AS `guc_is_cur` FROM `revision_userindex` INNER JOIN `page` ON `rev_page` = `page_id` LEFT OUTER JOIN `comment` ON `rev_comment_id` = `comment_id` WHERE `rev_deleted` = 0 AND `rev_user` = '9014223' ORDER BY `rev_timestamp` DESC LIMIT 0, 20;

These should change like

--- 5A) Query user data for en.wikipedia.org
SELECT `actor_id`, `actor_name` FROM `actor` WHERE `actor_name` = :user LIMIT 10;
--- 5B) Query contributions on en.wikipedia.org
SELECT `comment_text`, `rev_timestamp`, `rev_minor_edit`, `rev_len`, `rev_id`, `rev_parent_id`, `rev_user_text`, `page_title`, `page_namespace`, `page_latest` = `rev_id` AS `guc_is_cur` FROM `revision_userindex` INNER JOIN `page` ON `rev_page` = `page_id` LEFT OUTER JOIN `comment` ON `rev_comment_id` = `comment_id` WHERE `rev_deleted` = 0 AND `rev_actor` = '281698' ORDER BY `rev_timestamp` DESC LIMIT 0, 20;

I guess that you might currently have a different 5B if :user is an IP, checking against rev_user_text instead of rev_user. There would be no difference using actors, these queries work for both registered users and IPs.

Note that work on T221339 has made your old 5B start being really slow, to allow queries like the new 5B to be fast.

BTW, why the "LIMIT 10" on the 5A query? Both user.user_name and actor.actor_name are unique, so it should always return either 0 or 1 row.

BTW, why the "LIMIT 10" on the 5A query

This is because of prefix support. In the non-prefix case, it is indeed redundant. (source code)

Change 517576 had a related patch set uploaded (by MusikAnimal; owner: MusikAnimal):
[labs/tools/guc@master] [Experimental] Implement actor migration

https://gerrit.wikimedia.org/r/517576

Change 517576 merged by jenkins-bot:
[labs/tools/guc@master] Adopt actor tables and use dedicated comment table views

https://gerrit.wikimedia.org/r/517576

Krinkle closed this task as Resolved.Jun 21 2019, 11:05 PM
Krinkle assigned this task to MusikAnimal.
Krinkle added a subscriber: MusikAnimal.

I'm gonna land the patch now, although it only works for recent changes right now. Not for "All contributions".
I've fixed the query for "All contributions" (which just means the 20 latest edits including those not in the last 30 days from recent changes) using the same pattern as @MusikAnimal showed. But it now fails for a different reason: The query is now so slow for the revision_userindex view that it times out on the s3 small wikis. And then the query for enwiki/commons/wikidata after that is trying to query the now broken connection object, and get skipped. I'll fix that in a separate change, but closing this for now.

Change 518337 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[labs/tools/guc@master] Fix revision query after actor migration (user_id = actor_user, not actor_name)

https://gerrit.wikimedia.org/r/518337

Change 518337 merged by jenkins-bot:
[labs/tools/guc@master] Fix revision query after actor migration (user_id = actor_user, not actor_name)

https://gerrit.wikimedia.org/r/518337

Change 518347 had a related patch set uploaded (by Krinkle; owner: Krinkle):
[labs/tools/guc@master] Use the optimised actor_recentchanges/actor_revision everywhere

https://gerrit.wikimedia.org/r/518347

Change 518347 merged by jenkins-bot:
[labs/tools/guc@master] Use the optimised actor_recentchanges/actor_revision everywhere

https://gerrit.wikimedia.org/r/518347