Make WordPress Core

Opened 6 years ago

Last modified 5 years ago

#44370 new enhancement

wp_privacy_delete_old_export_files runs too much on some setups

Reported by: boonebgorges's profile boonebgorges Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: needs-patch
Focuses: Cc:

Description

The wp_privacy_delete_old_export_files cron job added in [43046] #43546 runs on an hourly schedule. On large multisite networks, this means that these cleanup are running more or less constantly. Yet they're almost never needed.

It's possible to write a plugin that modifies the behavior, by changing the way that scheduling is done so that it conforms better to the needs of a large network. However, I thought it might be worth considering whether there are changes that WP itself could make that would mitigate the problem for all installations. A few ideas, some of which are mutually compatible:

  1. Instead of scheduling the event on 'init', only schedule it when an export file is generated.
  2. During the cleanup routine, check for the existence of export files. If none are found, unschedule the recurring event (or don't schedule the next one)
  3. In combination with the above, use single events rather than recurring ones
  4. Perhaps the cleanup routine itself would be a single event, while a less-frequent (say, daily) recurring event would be to check whether specific cleanup events need to be scheduled (these would be cases missed by 1)

If the team thinks this is too complex, or should be left up to the maintainer of the site in cases where it makes a difference, feel free to close the ticket.

Change History (4)

#1 @nunomorgadinho
6 years ago

@boonebgorges if you feel strongly about this you may want to submit a patch so it can be considered.

#2 @boonebgorges
6 years ago

@nunomorgadinho I'll be glad to write a patch if members of the team who are more actively involved in the privacy tools agree that something like what I've suggested in a good idea. A premature patch is probably not a good use of time, and it may get us bogged down in implementation details, when I would first like others' thoughts on the general thinking behind the cleanup cron job :-D

#3 @desrosj
6 years ago

  • Keywords needs-patch added

I think this is a good idea. I like the option of single cleanup events paired with a daily event to check for missed files the best.

My one concern is that once a site has many completed requests, the daily cron checking for missed files will not be performant (I don't think it will be uncommon for users to keep completed requests for the record). A date query could help with that, only querying requests updated within the last X days. For this value, we can default to 3 days, but we should make sure to use the wp_privacy_export_expiration filter that is also used in wp_privacy_delete_old_export_files() in case a site changes the expiration time.

Multisite support is on the Privacy Component Road Map. @boonebgorges if you have any more multisite feedback, please do collect it and drop it in #core-privacy.

#4 @jetxpert
5 years ago

Comments and/or Fix Appreciated:

When using a Real Cron in lieu of WordPress' native WP Cron, your above cron [wp_privacy_delete_old_export_files] is being flagged by WP's Site Health tool.

Details: http://prntscr.com/o3uqzi

Thank you!

Note: See TracTickets for help on using tickets.