Page MenuHomePhabricator

Remove use of legacy page.page_restrictions field
Open, NormalPublic

Description

In 2007 (MW 1.10), the page_restrictions table was introduced. Rows in this table take precedence over the old page.page_restrictions field (r19095).

Later that year, a maintenance script was introduced to migrate old values (28df3d07e6, r21455).

Today, 12 years later, we still require an extra query to page.page_restrictions when calling Title::getRestrictions(), in addition to the page_restrictions table.

Let's finished this forgotten migration and kill off that field.

Plan
  • Verify that the maintenance script still works. And update it to modern standards as needed in order to make it suitable for running on large WMF wikis.
  • Ensure the updater is automatically run by the database updater.
  • Run the maintenance script on all WMF wikis.
  • Remove all code for queries to the old field. This, in addition to an improved update script that nulls out the column, should fix T30751.
  • Add schema update that removes the field. (core: T35334, wmf: T60674)

I'm not suggesting we drop the column in prod as it seems low priority. The page tables are very large, and the maintenance script will already null-out this field. However, DBAs can decide what they want to do here.

Details

Event Timeline

Krinkle created this task.Mar 15 2019, 9:39 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 15 2019, 9:39 PM
Krinkle moved this task from Limbo to Perf recommendation on the Performance-Team (Radar) board.
Krinkle added a subscriber: aaron.

Marking as perf recommendation for the additional queries required currently.

To reduce the overhead of this in the interim, @aaron added page.page_restrictions to LinkCache (see 93b24207c1cb / https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/496391/).

Marostegui added a subscriber: Marostegui.

If the column isn't in used, I would prefer if we can remove it from tables.sql, merge it, and then get a ticket to get the schema change done in production (it doesn't have to be done right away, but at least created so we have it as a backlog).
Removing DBA (but I will remain subscribed) as there is not actionable for us here yet (until the schema change in prod needs to actually be done).

Thank you!

kchapman added a subscriber: kchapman.

Moving to Backlog, but should be broken into tickets for each task before we work on this.

Krinkle updated the task description. (Show Details)Jul 18 2019, 9:46 PM
Krinkle triaged this task as Normal priority.Jul 18 2019, 10:13 PM

Change 537353 had a related patch set uploaded (by TK-999; owner: TK-999):
[mediawiki/core@master] Refactor the updateRestrictions.php maintenance script

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

TK-999 added a subscriber: TK-999.Sep 17 2019, 10:33 AM
Krinkle added a subscriber: WDoranWMF.

@WDoranWMF This was initially set for later I believe, but given Wikia has started work on it for us, perhaps CR could be provided earlier. Just leaving this here for now :)

@WDoranWMF This was initially set for later I believe, but given Wikia has started work on it for us, perhaps CR could be provided earlier. Just leaving this here for now :)

Just to clarify, I contributed this patch in a volunteer capacity :) If in the future I submit a patch as a direct outcome of my work, I will indicate it as appropriate on the relevant ticket and the patch itself.

Change 537353 merged by jenkins-bot:
[mediawiki/core@master] Refactor the updateRestrictions.php maintenance script

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

Krinkle updated the task description. (Show Details)EditedOct 8 2019, 6:36 PM
Krinkle updated the task description. (Show Details)

Ticked off the "maintenance script" and "run by installer" boxes. The above patch fixes the maintenance script. And the installer seems to already be running it as well. E.g. MysqlUpdater::doRestrictionsUpdate since MW 1.10.

Next step unlocked: Run the maintenance script on all WMF wikis.

Change 541601 had a related patch set uploaded (by TK-999; owner: TK-999):
[mediawiki/core@master] [DNM] Remove usages of legacy page.page_restrictions field

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