Scheduling just one recurring action makes more than one

[Nemi5150]

trafficstars

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).

[eddr]

I will be happy to test it, if that's of any help

I might be utilizing an unorthodox code, but the 'unique' parameter doesn't help in my case:

• Running a specific action via \ActionScheduler::runner()->process_action via the 'init' hook. Not sure it's the right way
• The reason is wanting to make sure it runs every 15 seconds, no matter if the WP cron is scheduled via Linux CRON ( and then probably way less then 15 seconds ) or in the standard way.

I actually think to automatically unhook the actions every now and then via CRON to handle this specific issue.

[barryhughes]

Thanks for the offer! We may reach out once we're ready for some help.

I suspect you are seeing the problem more frequently, because you're processing things more frequently. We'll try to update the PR I referenced earlier and move this task to completion, but I can't really offer a timeline as we have quite a lot of work on our hands at present.

[eddr]

For the meanwhile, went for a custom mechanism for my specific needs. Maybe it will be of interest:

• Used MySQL "SELECT ... FOR UPDATE NOWAIT" to get a lock on a custom _option row and prevent simultaneous runs. I can describe in more details if needed
• Seems to work well and also not requiring that much SQL queries compared to the usual 100s of queries done anyway. Locks are not waited for. Not sure what is the performance hit for now, but doesn't seem to be a taxing operation in my configuration