Scheduling just one recurring action makes more than one

[Nemi5150]

On the Usage page there is an example of how to "schedule a function to run at midnight if it is not already scheduled". See here
https://actionscheduler.org/usage/

I have used this example to create an action that runs every minute, and only one gets scheduled. However, every once in a while I check on the scheduled actions and there are two of those running. My guess is that there must be a race condition two requests are running at the same time and end up scheduling two upcoming actions. They then stay in perpetuity.

What is the best way to keep this from happening?

[barryhughes]

Thanks for flagging this, @Nemi5150.

My guess is that there must be a race condition two requests are running at the same time and end up scheduling two upcoming actions.

Yes, that seems like a real possibility. For example, simplifying a little, when an action is picked up for processing things shake out as follows:

• Action is fetched
• Corresponding record is updated to reflect that it is now in progress
• Action executes
• Status is updated to 'complete' (or 'failed', etc) as appropriate
• If it was a recurring action, the next action is now scheduled

So there is a gap between those last two points (the final status update, and the scheduling of the next individual action) and if, concurrently, a call to as_next_scheduled_action() is made then I imagine it will return false, leading to the creation of a surplus action.

Probably, we need to update our docs at the very least to provide a more robust way of handling this. Possibly, we also need some changes in Action Scheduler itself to handle this sort of scenario more gracefully.

[ovidiul]

I was able to duplicate this issue as well, with an aside, I've created a batch script that I was running concurrently(4 concurrent processes) to maybe try and simulate race conditions as:

action_scheduler_loop.sh

while true; do wp action-scheduler run; done

In my screenshots, the duplicate appeared at 18:01:21.

I've used this code for every minute(60 seconds) schedule:

/**
 * Schedule an action with the hook 'eg_midnight_log' to run at midnight each day
 * so that our callback is run then.
 */
function eg_schedule_midnight_log() {
	if ( false === as_has_scheduled_action( 'eg_each_minute_log' ) ) {
		as_schedule_recurring_action( strtotime( 'now' ), MINUTE_IN_SECONDS, 'eg_each_minute_log' );
	}
}
add_action( 'init', 'eg_schedule_midnight_log' );

/**
 * A callback to run when the 'eg_each_minute_log' scheduled action is run.
 */
function eg_log_action_data() {
	error_log( 'Date is: ' . date( 'Y-m-d' ) );
}
add_action( 'eg_each_minute_log', 'eg_log_action_data' );

Looking at this code line, that seems the one responsible for creating the duplicate, would it be worth changing the scheduled action store status to failed/complete only after the call to schedule the next event was sent?

[barryhughes]

Hi!

Would it be worth changing the scheduled action store status to failed/complete only after the call to schedule the next event was sent?

Possibly. I think that would reduce the potential for this problem to occur, but I also suspect the 'core' race condition is the one that arises directly from the snippet found here.

It's pretty aggressive in that it fires every request during init and, especially on a busy site, that could lead to the duplicates being created even if we did update the process_action() method in the way you outlined.

Altering our recommended approach in the docs may therefore be the safer and better approach (short of a larger change that revises how recurring actions work and are stored).

[eddr]

Hello there

Happens to me as well in the same manner
Any news? Can I somehow help with that?

[barryhughes]

@eddr,

We have a further fix in the works, though efforts have been paused for some time while we prioritized other work. That said, in case you were not already aware of this, it has been possible to declare actions as 'unique' since our 3.6.0 release which may help to reduce instances of recurring event duplication. For instance, you can now replace this:

if ( ! as_has_scheduled_action( 'foo' ) ) {
	as_schedule_recurring_action( time(), HOUR_IN_SECONDS, 'foo' );
}

With:

as_schedule_recurring_action( time(), HOUR_IN_SECONDS, 'foo', [], '', true );

Or, using some PHP 8.x syntax:

as_schedule_recurring_action( time(), HOUR_IN_SECONDS, 'foo', unique: true );

[eddr]

Thanks barryhughes!
I'm pretty aure i tried the 'unique' parameter, but I test again to make sure and let you know what are the results

[barryhughes]

Yep, it's not a complete solution: it (hopefully) solves one part of the problem, but there is still a race condition in the logic that schedules the replacement action (which is what the PR I linked to tries to solve).