Page MenuHomePhabricator
Create Task
Maniphest T270620

Schema change for renaming several indexes in logging table
Closed, ResolvedPublic
Actions

Description

Part of T270033: Fix and enforce table prefix usage in columns and indexes in core

  1. ALTERs to run: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/648817/3/maintenance/archives/patch-logging-rename-indexes.sql
  2. Where to run those changes: all.dblist
  3. When to run those changes: At any time
  4. If the schema change is backwards compatible: Yes
  5. If the schema change has been tested already on some of the test/beta wikis: Tested in beta cluster.
  6. if the data should be made available on the labs replicas and/or dumps: Yes, data in this table is public

Progress

  • s1
    • eqiad
    • codfw
  • s2
    • eqiad
    • codfw
  • s3
    • eqiad
    • codfw
  • s4
    • eqiad
    • codfw
  • s5
    • eqiad
    • codfw
  • s7
    • eqiad
    • codfw
  • s8
    • eqiad
    • codfw
  • labswiki
  • labtestwiki

Details

SubjectRepoBranchLines +/-
Remove logging table index hintmediawiki/coremaster+0 -15
Customize query in gerrit

Related Objects
Search...

Event Timeline

Ladsgroup created this task.Dec 21 2020, 12:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 21 2020, 12:36 PM
LSobanski triaged this task as Medium priority.Dec 21 2020, 12:37 PM
LSobanski moved this task from Triage to Ready on the DBA board.
LSobanski subscribed.
• Marostegui subscribed.Jan 13 2021, 7:18 AM

@Ladsgroup I have found one of these indexes being forced on code, can you double check that piece of code?
page_time: https://github.com/examknow/CentralCheckUser/blob/f3fc3ff5e2ce95db7650acac268c5a0c6965b4e0/includes/specials/SpecialGlobalCheckUser.php#L1664 https://gerrit.wikimedia.org/g/mediawiki/extensions/CheckUser/+/bc82fa56236990b47842cbded63aaa26c7a23763/includes/specials/SpecialCheckUser.php#2028

Ladsgroup added a comment.Jan 13 2021, 7:06 PM

The first link doesn't matter as it's not deployed in our systems AFAIK

The second one actually has changed to accommodates this change. See https://gerrit.wikimedia.org/r/c/mediawiki/extensions/CheckUser/+/652400

RhinosF1 subscribed.Jan 13 2021, 7:29 PM

I'll contact Manual re the first one shortly.

• Marostegui added a comment.Jan 14 2021, 8:30 AM

@RhinosF1 and myself just sync and it is fine to proceed as examknow extension isn't enabled, as Amir says, and it is likely it won't be in the future anyways.
Thank you both!

• Marostegui claimed this task.Jun 18 2021, 11:26 AM
• Marostegui added a comment.EditedJun 18 2021, 11:39 AM

Deployed this change on db1180 and db1096:3316, if nothing goes wrong on the query front, we can deploy it everywhere on codfw (or on eqiad if eqiad is passive already)

s6 eqiad progress

  • dbstore1005
  • db1180
  • db1173
  • db1168
  • db1165
  • db1155
  • db1140
  • db1131
  • db1113
  • db1098
  • db1096
  • clouddb1021
  • clouddb1019
  • clouddb1015
• Marostegui moved this task from Ready to In progress on the DBA board.Jun 18 2021, 11:40 AM
• Marostegui moved this task from Backlog to In progress on the Schema-change-in-production board.Jun 18 2021, 11:43 AM
• Marostegui updated the task description. (Show Details)
• Marostegui added a comment.Jun 21 2021, 4:45 AM

@Ladsgroup I have seen this query:

SELECT /* IndexPager::buildQueryInfo (LogPager) */ log_id, log_type, log_action, log_timestamp, log_namespace, log_title, log_params, log_deleted, user_id, user_name, user_editcount, log_actor, logging_actor.actor_user AS `log_user`, logging_actor.actor_name AS `log_user_text`, comment_log_comment.comment_text AS `log_comment_text`, comment_log_comment.comment_data AS `log_comment_data`, comment_log_comment.comment_id AS `log_comment_cid`, (SELECT GROUP_CONCAT(ctd_name SEPARATOR ', ') FROM `change_tag` JOIN `change_tag_def` ON ((ct_tag_id=ctd_id)) WHERE ct_log_id=log_id ) AS `ts_tags` FROM `logging` IGNORE INDEX (log_times) JOIN `actor` `logging_actor` ON ((actor_id=log_actor)) LEFT JOIN `user` ON ((user_id=logging_actor.actor_user)) JOIN `comment` `comment_log_comment` ON ((comment_log_comment.comment_id = log_comment_id)) JOIN `change_tag` ON ((ct_log_id=log_id)) WHERE (log_type NOT IN ('titleblacklist', 'urlshortener', 'abusefilterprivatedetails', 'oath', 'suppress')) AND log_type = 'create' AND ct_tag_id = 99 ORDER BY log_timestamp DESC LIMIT 51 /* 263df222a3933ac11fa72cdc4073333d db1096:3316 frwiki 13s */

That query is specifically ignoring log_times index, which is not present on any of the hosts apart from the two we've done in s6. We might want to remove this before proceeding.
The optimizer doesn't seem to be silly anymore, so the IGNORE might not be needed:

root@db1098.eqiad.wmnet[frwiki]> explain SELECT /* IndexPager::buildQueryInfo (LogPager) */ log_id, log_type, log_action, log_timestamp, log_namespace, log_title, log_params, log_deleted, user_id, user_name, user_editcount, log_actor, logging_actor.actor_user AS `log_user`, logging_actor.actor_name AS `log_user_text`, comment_log_comment.comment_text AS `log_comment_text`, comment_log_comment.comment_data AS `log_comment_data`, comment_log_comment.comment_id AS `log_comment_cid`, (SELECT GROUP_CONCAT(ctd_name SEPARATOR ', ') FROM `change_tag` JOIN `change_tag_def` ON ((ct_tag_id=ctd_id)) WHERE ct_log_id=log_id ) AS `ts_tags` FROM `logging` JOIN `actor` `logging_actor` ON ((actor_id=log_actor)) LEFT JOIN `user` ON ((user_id=logging_actor.actor_user)) JOIN `comment` `comment_log_comment` ON ((comment_log_comment.comment_id = log_comment_id)) JOIN `change_tag` ON ((ct_log_id=log_id)) WHERE (log_type NOT IN ('titleblacklist', 'urlshortener', 'abusefilterprivatedetails', 'oath', 'suppress')) AND log_type = 'create' AND ct_tag_id = 99 ORDER BY log_timestamp DESC LIMIT 51 /* 263df222a3933ac11fa72cdc4073333d db1096:3316 frwiki 13s */
    -> ;
+------+--------------------+---------------------+--------+------------------------------------------------------------------+---------------+---------+---------------------------------+--------+-----------------------------------------------------------+
| id   | select_type        | table               | type   | possible_keys                                                    | key           | key_len | ref                             | rows   | Extra                                                     |
+------+--------------------+---------------------+--------+------------------------------------------------------------------+---------------+---------+---------------------------------+--------+-----------------------------------------------------------+
|    1 | PRIMARY            | change_tag          | ref    | ct_log_tag_id,ct_tag_id_id                                       | ct_tag_id_id  | 4       | const                           | 950912 | Using where; Using index; Using temporary; Using filesort |
|    1 | PRIMARY            | logging             | eq_ref | PRIMARY,type_time,actor_time,log_actor_type_time,log_type_action | PRIMARY       | 4       | frwiki.change_tag.ct_log_id     | 1      | Using where                                               |
|    1 | PRIMARY            | logging_actor       | eq_ref | PRIMARY                                                          | PRIMARY       | 8       | frwiki.logging.log_actor        | 1      |                                                           |
|    1 | PRIMARY            | user                | eq_ref | PRIMARY                                                          | PRIMARY       | 4       | frwiki.logging_actor.actor_user | 1      | Using where                                               |
|    1 | PRIMARY            | comment_log_comment | eq_ref | PRIMARY                                                          | PRIMARY       | 8       | frwiki.logging.log_comment_id   | 1      |                                                           |
|    2 | DEPENDENT SUBQUERY | change_tag          | ref    | ct_log_tag_id,ct_tag_id_id                                       | ct_log_tag_id | 5       | frwiki.logging.log_id           | 7      | Using index                                               |
|    2 | DEPENDENT SUBQUERY | change_tag_def      | eq_ref | PRIMARY                                                          | PRIMARY       | 4       | frwiki.change_tag.ct_tag_id     | 1      |                                                           |
+------+--------------------+---------------------+--------+------------------------------------------------------------------+---------------+---------+---------------------------------+--------+-----------------------------------------------------------+
7 rows in set (0.012 sec)

And this is on enwiki:

+------+--------------------+---------------------+--------+------------------------------------------------------------------+-----------------------+---------+---------------------------------+------+-----------------------------------------------------------+
| id   | select_type        | table               | type   | possible_keys                                                    | key                   | key_len | ref                             | rows | Extra                                                     |
+------+--------------------+---------------------+--------+------------------------------------------------------------------+-----------------------+---------+---------------------------------+------+-----------------------------------------------------------+
|    1 | PRIMARY            | change_tag          | ref    | change_tag_log_tag_id,change_tag_tag_id_id                       | change_tag_tag_id_id  | 4       | const                           | 67   | Using where; Using index; Using temporary; Using filesort |
|    1 | PRIMARY            | logging             | eq_ref | PRIMARY,type_time,actor_time,log_actor_type_time,log_type_action | PRIMARY               | 4       | enwiki.change_tag.ct_log_id     | 1    | Using where                                               |
|    1 | PRIMARY            | logging_actor       | eq_ref | PRIMARY                                                          | PRIMARY               | 8       | enwiki.logging.log_actor        | 1    |                                                           |
|    1 | PRIMARY            | user                | eq_ref | PRIMARY                                                          | PRIMARY               | 4       | enwiki.logging_actor.actor_user | 1    | Using where                                               |
|    1 | PRIMARY            | comment_log_comment | eq_ref | PRIMARY                                                          | PRIMARY               | 8       | enwiki.logging.log_comment_id   | 1    |                                                           |
|    2 | DEPENDENT SUBQUERY | change_tag          | ref    | change_tag_log_tag_id,change_tag_tag_id_id                       | change_tag_log_tag_id | 5       | enwiki.logging.log_id           | 7    | Using index                                               |
|    2 | DEPENDENT SUBQUERY | change_tag_def      | eq_ref | PRIMARY                                                          | PRIMARY               | 4       | enwiki.change_tag.ct_tag_id     | 1    |                                                           |
+------+--------------------+---------------------+--------+------------------------------------------------------------------+-----------------------+---------+---------------------------------+------+-----------------------------------------------------------+
7 rows in set (0.012 sec)
Ladsgroup added a comment.Jun 21 2021, 8:11 AM

Okay this is interesting. The code for this is actually explicitly set to check which index exists and ignore that instead. The comment for using this index is:

			// T223151, T237026: MariaDB's optimizer, at least 10.1, likes to choose a wildly bad plan for
			// some reason for these code paths. Tell it not to use the wrong index it wants to pick.

Maybe it doesn't apply anymore? maybe since logging table got smaller now MariaDB is not going haywire?

Ladsgroup added a comment.Jun 21 2021, 8:11 AM

The code that checks is $index = $this->mDb->indexExists( 'logging', 'times', __METHOD__ ) ? 'times' : 'log_times';

• Marostegui added a comment.Jun 21 2021, 8:43 AM

I am going to double check across big wikis to see what the optimizer is doing, but yeah, likely it isn't needed anymore

• Marostegui added a comment.Jun 21 2021, 8:56 AM

Tested the following wikis:

  • enwiki
  • commonswiki
  • dewiki
  • eswiki
  • wikidatawiki

The optimizer is doing the right thing, so it is probably ok to remove that IGNORE.

gerritbot added a comment.Jun 21 2021, 10:48 AM

Change 700515 had a related patch set uploaded (by Ladsgroup; author: Ladsgroup):

[mediawiki/core@master] Remove logging table index hint

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

gerritbot added a project: Patch-For-Review.Jun 21 2021, 10:48 AM
• Marostegui changed the task status from Open to Stalled.Jun 22 2021, 9:03 AM
• Marostegui moved this task from In progress to Blocked on the DBA board.

Stalling this till the above is pushed

Ladsgroup added a subscriber: daniel.Jun 23 2021, 11:05 AM
In T270620#7165268, @gerritbot wrote:

Change 700515 had a related patch set uploaded (by Ladsgroup; author: Ladsgroup):

[mediawiki/core@master] Remove logging table index hint

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

Can someone from PET review this patch? @daniel

gerritbot added a comment.Jun 28 2021, 4:03 PM

Change 700515 merged by jenkins-bot:

[mediawiki/core@master] Remove logging table index hint

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

Maintenance_bot removed a project: Patch-For-Review.Jun 28 2021, 4:10 PM
ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.12; 2021-06-28).Jun 28 2021, 5:00 PM
Ladsgroup added a comment.Jul 3 2021, 6:02 AM

@Marostegui The index check has been removed and now deployed. I think this is good to go.

• Marostegui changed the task status from Stalled to Open.Jul 5 2021, 6:27 AM
• Marostegui updated the task description. (Show Details)EditedJul 7 2021, 10:34 AM

codfw s6

  • db2141
  • db2129
  • db2124
  • db2117
  • db2114
  • db2095
  • db2089
  • db2087
  • db2076
• Marostegui moved this task from Blocked to In progress on the DBA board.Jul 7 2021, 10:37 AM
• Marostegui updated the task description. (Show Details)

Altered two hosts in production in s6 to make sure we don't see anymore issues. If we don't, I will continue with all eqiad sections.

• Marostegui updated the task description. (Show Details)Jul 7 2021, 1:27 PM
• Marostegui added a comment.Jul 8 2021, 6:20 AM

s6 codfw is entirely altered.

• Marostegui updated the task description. (Show Details)Jul 8 2021, 6:20 AM
• Marostegui updated the task description. (Show Details)Jul 8 2021, 6:23 AM
• Marostegui updated the task description. (Show Details)
• Marostegui updated the task description. (Show Details)Jul 8 2021, 8:16 AM
• Marostegui updated the task description. (Show Details)Jul 8 2021, 9:21 AM
• Marostegui updated the task description. (Show Details)Jul 8 2021, 11:11 AM
• Marostegui updated the task description. (Show Details)Jul 8 2021, 12:03 PM
• Marostegui updated the task description. (Show Details)Jul 8 2021, 1:14 PM
• Marostegui changed the task status from Open to Stalled.Jul 21 2021, 5:12 AM
• Marostegui updated the task description. (Show Details)
• Marostegui moved this task from In progress to Unclear/blocked to proceed on the Schema-change-in-production board.

All eqiad is done - waiting for the switch back.

• Marostegui moved this task from In progress to Blocked on the DBA board.Jul 21 2021, 5:12 AM
• Marostegui added a parent task: T285139: DB maintenance work to do while eqiad is passive (June 2021).Sep 15 2021, 5:24 AM
• Marostegui changed the task status from Stalled to Open.Sep 15 2021, 5:31 AM
• Marostegui moved this task from Blocked to In progress on the DBA board.Sep 30 2021, 5:42 AM
Stashbot added a comment.Sep 30 2021, 5:45 AM

Mentioned in SAL (#wikimedia-operations) [2021-09-30T05:45:03Z] <marostegui> Deploy schema change on s2 codfw (lag will show up) T270620

• Marostegui updated the task description. (Show Details)Sep 30 2021, 5:45 AM
Stashbot added a comment.Sep 30 2021, 5:45 AM

Mentioned in SAL (#wikimedia-operations) [2021-09-30T05:45:35Z] <marostegui> Deploy schema change on s4 codfw (lag will show up) T270620

• Marostegui updated the task description. (Show Details)Sep 30 2021, 5:45 AM
Stashbot added a comment.Sep 30 2021, 5:47 AM

Mentioned in SAL (#wikimedia-operations) [2021-09-30T05:47:41Z] <marostegui> Deploy schema change on s5 codfw (lag will show up) T270620

• Marostegui updated the task description. (Show Details)Sep 30 2021, 5:48 AM
Stashbot added a comment.Sep 30 2021, 5:52 AM

Mentioned in SAL (#wikimedia-operations) [2021-09-30T05:52:09Z] <marostegui> Deploy schema change on s7 codfw (lag will show up) T270620

• Marostegui updated the task description. (Show Details)Sep 30 2021, 5:52 AM
Stashbot added a comment.Sep 30 2021, 5:53 AM

Mentioned in SAL (#wikimedia-operations) [2021-09-30T05:53:01Z] <marostegui> Deploy schema change on s3 codfw (lag will show up) T270620

• Marostegui updated the task description. (Show Details)Sep 30 2021, 6:00 AM
Stashbot added a comment.Sep 30 2021, 6:01 AM

Mentioned in SAL (#wikimedia-operations) [2021-09-30T06:01:31Z] <marostegui> Deploy schema change on s1 codfw (lag will show up) T270620

• Marostegui updated the task description. (Show Details)Sep 30 2021, 6:47 AM
Stashbot added a comment.Sep 30 2021, 6:48 AM

Mentioned in SAL (#wikimedia-operations) [2021-09-30T06:48:31Z] <marostegui> Deploy schema change on s8 codfw (lag will show up) T270620

• Marostegui closed this task as Resolved.Sep 30 2021, 7:01 AM
• Marostegui updated the task description. (Show Details)

This is all done

gerritbot added a comment.Jul 21 2022, 9:49 PM

Change 816048 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/CheckUser@master] Remove now redundant code now that T270620 is resolved

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

gerritbot added a project: Patch-For-Review.Jul 21 2022, 9:49 PM
Maintenance_bot moved this task from In progress to Done on the DBA board.Jul 21 2022, 10:29 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL