Page MenuHomePhabricator

Move the "big deletion" logic from Title to DeletePage
Closed, ResolvedPublic3 Estimated Story Points

Description

As explained in T288242. Callers were audited in T288281, here are the conclusions:

I don't see any showstopper or anything that prevents us from moving the logic from Title to DeletePage and make it not be page-wise but deletion-wise, as long as the following things are done/kept in mind:

  • The old behaviour should be deprecated: soft deprecation and a mention in the release notes. Hard deprecation is impossible as callers have no way to specify whether they want that code to be executed or not.
  • Add a notice to Tech/News once the old code is removed, in particular for the change to ApiQueryInfo behaviour
  • It'd be nice if we could migrate the methods listed above that also delete pages to the new DeletePage service
    • I'd like to better understand the code in Translate and see if it can be migrated, too
  • Obviously, make sure that deletion-related code in core (DeleteAction, FileDeleteForm, ApiDelete, WikiPage) will still perform the bigdeletion check.

Note: The removal of the deprecated code is tracked in T293463.

Event Timeline

Change 714048 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Move more logic from WikiPage to DeletePage

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

Change 714383 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@master] Deprecate WikiPage methods replaced by DeletePage

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

Change 714048 merged by jenkins-bot:

[mediawiki/core@master] Move more logic from WikiPage to DeletePage

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

Daimona set the point value for this task to 3.Aug 31 2021, 11:22 AM

Change 714383 merged by jenkins-bot:

[mediawiki/core@master] Deprecate WikiPage methods replaced by DeletePage

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

Change 721667 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/core@REL1_37] Deprecate WikiPage methods replaced by DeletePage

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

Change 721667 merged by jenkins-bot:

[mediawiki/core@REL1_37] Deprecate WikiPage methods replaced by DeletePage

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

Daimona added subscribers: β€’ imaigwilo, dom_walden.

I think this can be QAed, given that I've created another task to track the cleanup.

@dom_walden, @imaigwilo: In T288758#7404009 I mentioned testing the deletion of pages with many revisions ($wgDeleteRevisionsLimit, with/without the 'bigdelete' permission). That's exactly what needs to be tested for this ticket, so if you can confirm that deletion of "big pages" is working correctly from ApiDelete, the API part of this task is done. Once testing T288758 again for the web interface (T288758#7427975), if big deletions work correctly you can move this task to produt sign-off as well.

I think this can be QAed, given that I've created another task to track the cleanup.

@dom_walden, @imaigwilo: In T288758#7404009 I mentioned testing the deletion of pages with many revisions ($wgDeleteRevisionsLimit, with/without the 'bigdelete' permission). That's exactly what needs to be tested for this ticket, so if you can confirm that deletion of "big pages" is working correctly from ApiDelete, the API part of this task is done. Once testing T288758 again for the web interface (T288758#7427975), if big deletions work correctly you can move this task to produt sign-off as well.

@Daimona So I deleted via the API:

Is that enough for now? Shall I leave this here for now?

Daimona changed the task status from Open to Stalled.Oct 19 2021, 10:08 PM

I think this can be QAed, given that I've created another task to track the cleanup.

@dom_walden, @imaigwilo: In T288758#7404009 I mentioned testing the deletion of pages with many revisions ($wgDeleteRevisionsLimit, with/without the 'bigdelete' permission). That's exactly what needs to be tested for this ticket, so if you can confirm that deletion of "big pages" is working correctly from ApiDelete, the API part of this task is done. Once testing T288758 again for the web interface (T288758#7427975), if big deletions work correctly you can move this task to produt sign-off as well.

@Daimona So I deleted via the API:

Is that enough for now? Shall I leave this here for now?

Yes, thank you. This is now blocked on the UI part of T288758. Once that's done and you will have tested big deletions via the UI, you can consider this task done as well.

ldelench_wmf changed the task status from Stalled to In Progress.Nov 24 2021, 12:07 PM

I have tested deleting and suppressing various "big" pages and files on beta via the UI[1].

I checked that the appropriate changes were made to the database (e.g. old revisions and file uploads were archived). A single entry was generated in the delete or suppress log and recent changes (unless it was a suppress).

There were no errors in the logs, except /rpc/RunSingleJob.php Wikimedia\Rdbms\DBQueryError: Error 1062: Duplicate entry '102122' for key 'ar_revid_uniq'. I think this error always happened (I see this report from 2018 T208672), and does not appear to affect the outcome of the delete (i.e. I couldn't detect any loss of data).

As deleting a "big" page happens in batches using scheduled jobs, I tried to see if I could interrupt this process. I experimented with this by making my local wiki read-only just after I had kicked off a delete job for a "big" page. I could pause the delete jobs by doing this, but I couldn't prevent the delete from eventually happening after I turned the writes back on (although sometimes I had to manually restart the scheduled jobs using the runJobs.php maintenance script). It is very hard to simulate at will the kinds of errors/interrupts we might see in production, so I am not confident I have done this very realistically.

Test environment: Various. Latest https://en.wikipedia.beta.wmflabs.org MediaWiki 1.38.0-alpha (496507b) 07:20, 3 December 2021.

Notes

  1. The UI delete form is accessible when viewing an article/file while logged in as a user who is an administrator
    delete_ui.png (264Γ—978 px, 22 KB)
β€’ NRodriguez subscribed.

For more technical tickets like this that I can't test as PM, will approve based on Dom QA! Lmk if I am missing something though