Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prune duplicate recurring events #251

Merged
merged 2 commits into from
Jan 24, 2022
Merged

Conversation

WPprodigy
Copy link
Contributor

@WPprodigy WPprodigy commented Jan 20, 2022

As apart of #234, this will help cleanup duplicate events that exist (daily). It's pretty safe, as we only look at recurring events that have the exact same action/args/schedule. There is really no need to have events like that. If you need the same hook on the same schedule, then the args need to be different to make them unique. If the args don't matter though, then probably the event should be looked into being added onto a schedule that occurs more often.

This alone won't solve all our problems mentioned in that issue, but no matter what we're going to need to cleanup the messes that currently exist. So this seems like a good starting point. Notably, this should drastically help reduce the "duplicate entry" db errors that can cause events to become stuck at the top of the queue and repeatedly run each cycle.

I opted for not including duplicate single events, since they technically clean themselves up. But could possibly revisit that in the future. For those will probably want to respect the "10minute window" that core checks: https://developer.wordpress.org/reference/functions/wp_schedule_single_event/. As in, only remove duplicate single events that exist within 10minutes of each other.

Testing

The unit test is pretty good for this as well :)

Did fresh WP install w/ cron control disabled and ran the following:

wp shell

wp_schedule_single_event( time() + 100, 'custom_single_event' );
wp_schedule_single_event( time() + 200, 'custom_single_event' );

wp_schedule_event( time() + 100, 'hourly', 'custom_recurring_event' );
wp_schedule_event( time() + 200, 'hourly', 'custom_recurring_event' );

Then activated cron control, and refreshed twice so the table & internal events could be setup. Then ran cron (so the cleanup legacy data task runs). Afterwards, both custom_single_event were in the custom table and only one custom_recurring_event was left.

Base automatically changed from feature/migrate-legacy-cron-events to master January 24, 2022 17:03
$duplicate_events = [];
foreach ( $events as $event ) {
if ( ! $event->is_recurring() ) {
// Only interested in recurring events.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because we may have single scheduled events with the same args that are supposed to run synchronously?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorta. Wrote some about it in the description:

I opted for not including duplicate single events, since they technically clean themselves up. But could possibly revisit that in the future. For those will probably want to respect the "10minute window" that core checks: https://developer.wordpress.org/reference/functions/wp_schedule_single_event/. As in, only remove duplicate single events that exist within 10minutes of each other.

@WPprodigy WPprodigy merged commit 83631b4 into master Jan 24, 2022
@WPprodigy WPprodigy deleted the fix/remove-duplicate-events branch January 24, 2022 20:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants