Page MenuHomePhabricator

update.php needs to drop both archive.usertext_timestamp and archive.ar_usertext_timestamp, if they exist, on MySQL
Closed, ResolvedPublic

Description

In MW 1.11, rSVN23917: *Include usertext_timestamp index here and rSVN23491: *Add usertext_timestamp index to archive table added a usertext_timestamp index to archive.

In MW 1.15, a series of patches affected this index:

It looks like after all of that the result was that even though tables.sql said /*i*/ar_usertext_timestamp, the installer would create the index named usertext_timestamp. But if you create the tables directly from tables.sql, you're liable to wind up with ar_usertext_timestamp instead. See T104756.

In MW 1.28, rMWbec6151e5fe3: Remove old remapping hacks from Database::indexName() tried removing the /*i*/-aliasing. That wound up being reverted in rMW307851618430: Revert "Remove old remapping hacks from Database::indexName()". Also, rMWc13dd55f4e04: Fix incorrect index name in new 1.28.0 installations created a MySQL-specific patch-rename-ar_usertext_timestamp.sql to try to rename ar_usertext_timestamp to usertext_timestamp, but as far as I can tell it'll never actually be run by update.php because patch-archive-user-index.sql from MW 1.11 (rSVN23917) will create usertext_timestamp first.

In MW 1.31, rMW4ccb228bde92: rdbms: inject the mysql index name aliases into Database removed the hard-coding of the /*i*/ overrides from DatabaseMySQLBase, instead injecting them from MWLBFactory. But the installer doesn't use MWLBFactory, so now on installation it'll create ar_usertext_timestamp. The first time update.php is run, the patch from 1.11 will run to add usertext_timestamp. Apparently no one filed a bug about this breaking anything, or they just decided it was a duplicate of T104756.

In MW 1.34, rMWc29909e59fd8: Mostly drop old pre-actor user schemas drops /*i*/ar_usertext_timestamp, which currently actually drops usertext_timestamp. ar_usertext_timestamp, if it exists, isn't dropped. When the ar_user_text column is dropped, MySQL apparently just goes ahead and silently changes the index from (ar_user_text, ar_timestamp) to just (ar_timestamp). Grr.

At some point (e.g. now) we'll realize that the index alias in MWLBFactory isn't being used anymore and we'll delete it, at which point it'll try to drop ar_usertext_timestamp (possibly causing an error if that doesn't exist) but leave usertext_timestamp in place.

Event Timeline

Anomie created this task.Sep 18 2019, 3:22 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 18 2019, 3:22 PM
Anomie claimed this task.Sep 18 2019, 3:22 PM
Anomie moved this task from Inbox to Doing on the Platform Team Workboards (Clinic Duty Team) board.

Change 537676 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Clean up ar_usertext_timestamp index mess

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

Anomie updated the task description. (Show Details)Sep 18 2019, 4:06 PM
Anomie updated the task description. (Show Details)Sep 18 2019, 4:08 PM

Change 537694 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Remove MySQL index aliasing for user_newtalk indexes

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

Change 537694 had a related patch set uploaded (by Anomie; owner: Anomie):
[mediawiki/core@master] Remove MySQL index aliasing for user_newtalk indexes

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

Oops, wrong bug link.

Change 537676 merged by jenkins-bot:
[mediawiki/core@master] Clean up ar_usertext_timestamp index mess

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

Anomie closed this task as Resolved.Sep 26 2019, 7:50 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:38 PM