Page MenuHomePhabricator

Add img_deleted column
Open, Stalled, Needs TriagePublic

Description

ALTER TABLE image ADD img_deleted tinyint unsigned NOT NULL default 0; (or more precisely, a complex list of operations with the same result but without affecting production) has been run on ALL WMF wikis.

I suppose that concludes the first step.

Be careful with Commons: there, image is a very busy 30-million row table, and even simple operations can generate lag.

ALTER TABLE: ALTER TABLE /*_*/image ADD img_deleted tinyint unsigned NOT NULL default 0;
WHERE: All wikis
WHEN: At DBA's leisure
BACK COMPAT: Yes, was previously live
TESTED: As above

Event Timeline

Reedy created this task.Jul 6 2020, 3:41 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 6 2020, 3:41 PM
Majavah added a subscriber: Majavah.Jul 6 2020, 3:44 PM
Reedy added a comment.Jul 6 2020, 3:45 PM

Sorry :(

I guess it wasn't checked where it came from, and whether it would be used? No reference to T90300 in that task

Reedy updated the task description. (Show Details)Jul 6 2020, 3:46 PM
Reedy renamed this task from Add img_deleted column on wikis where it's missing to Add img_deleted column.Jul 6 2020, 3:51 PM

Sorry :(

I guess it wasn't checked where it came from, and whether it would be used? No reference to T90300 in that task

Looks like a phab search would've definitely found it as it's used in the task description etc...

Checking tables.sql I don't see that img_deleted column there, so should we get a patch merged before that to avoid those issues again?

Reedy added a comment.Jul 6 2020, 3:58 PM

Yup, already started with that; I've taken the original patch and slimmed it down

Change 609798 had a related patch set uploaded (by Reedy; owner: CSteipp):
[mediawiki/core@master] Add img_deleted column

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

With how it was originally implemented, it would add the column, and then do some UPDATE queries against a few tables, unbounded, not batched etc - https://gerrit.wikimedia.org/r/c/mediawiki/core/+/609798/1/maintenance/archives/patch-img_deleted.sql

It might be worth stalling this (for now), until the other patches are reworked and ready for merge, as at a minimum they're going to be rather rotten, and needs someone to finish them them off

Tgr added a subscriber: Tgr.Jul 6 2020, 4:22 PM
Tgr added a comment.Jul 6 2020, 4:31 PM

With how it was originally implemented, it would add the column, and then do some UPDATE queries against a few tables, unbounded, not batched etc - https://gerrit.wikimedia.org/r/c/mediawiki/core/+/609798/1/maintenance/archives/patch-img_deleted.sql

See T90300#1109157 and surrounding comments - the number of rows to be changed is actually very small (or at least was three years ago). The number of rows to be read is large but possibly still within reason.

But yeah, the code was written three years ago, we had major DB structure changes since then, all this needs to be updated and re-tested. Adding the column would probably be safe, but let's wait for code review first.

Tgr changed the task status from Open to Stalled.Jul 6 2020, 4:31 PM

Removing Blocked-on-schema-change as this schema change isn't really blocked (there is no merged patch on tables.sql with the new column). Once we are finally ready for it we can add it back.