Add a faster and simpler version of as_next_scheduled_action()

[danielbitzer]

as_next_scheduled_action() is the standard way that plugins can determine if a specific action is currently upcoming/scheduled but it could be optimized.

For example, the Facebook for WooCommerce plugin performs a check on every init hook to determine if the wc_facebook_regenerate_feed recurring action is scheduled. We've had a report that this is slowing down sites and arguable we should be storing this as a transient.

But after looking into as_next_scheduled_action() I realized it performs up to 4 queries to gain additional information about the scheduled action while most use cases I see in plugins simply need a boolean response. My proposal is to introduce a as_has_scheduled_action() function that is similar to as_next_scheduled_action() but is optimized for this common use case of checking that a specific action is scheduled.

Advantages

• Since we don't need the "next" action we can drop the ORDER BY scheduled_date_gmt clause which has been reported to significantly slow down queries, see Speed up clean up and batch selects. #678 and large actions table results in non-performant query facebook-for-woocommerce#1796 (comment)
• Swaps out 3 queries for a single query

Existing as_next_scheduled_action() queries:

SELECT a.action_id
FROM wp_actionscheduler_actions a
WHERE a.hook='wc_facebook_regenerate_feed'
AND a.status='in-progress'
ORDER BY scheduled_date_gmt DESC
LIMIT 1

SELECT a.action_id
FROM wp_actionscheduler_actions a
WHERE a.hook='wc_facebook_regenerate_feed'
AND a.status='pending'
ORDER BY scheduled_date_gmt ASC
LIMIT 1

SELECT a.*, g.slug AS `group`
FROM wp_actionscheduler_actions a
LEFT JOIN wp_actionscheduler_groups g
ON a.group_id=g.group_id
WHERE a.action_id= {%SOME ACTION ID%}

New as_has_scheduled_action() query:

SELECT a.action_id
FROM wp_actionscheduler_actions a
WHERE 1=1
AND a.hook='wc_facebook_regenerate_feed'
AND a.status IN ('in-progress', 'pending')
LIMIT 0, 1

Next steps

• Get initial feedback
• Update tests
• Does the WP Posts store need updating?
• Update docs

Changelog

Add - New helper function as_has_scheduled_action() which provides a faster way to test if an action is scheduled.

[thenbrent]

We've had a report that this is slowing down sites and arguable we should be storing this as a transient.

Just noting that this has come up before, including in WC core usage. The previous recommendation was to add the transient / caching in the calling code, both because as_next_scheduled_action() needs to be real time for backward compatibility and also because the calling code has insight into the purpose of the action, so it can set a smarter transient expiration time (e.g. some things should be cached for a day, others for no more than an hour).

That doesn't mean AS can't offer a new function with a transient or other cache to improve performance, and this issue has come up enough times that it would be worth doing that. It looks like the introduction of as_has_scheduled_action() might be the perfect time to do that given its purpose.

There's still an issue of how long to set the cache for, but even a short lived expiry, like 1-2 minutes, could provide significant performance improvements by default, and any sites that want a longer expiry still have the option to use as_next_scheduled_action() directly with their our cache time.

[crstauf]

IMO leaving caching to the caller is perfectly acceptable and preferable: as_has_scheduled_action() with only a fast query would be great.

If you implement caching into the function, then you also have to clear the cached item when an action is scheduled, which adds complexity. IMO: intro the new function with a fast query, and let the caller do anything else.

[danielbitzer]

Just noting that this has come up before, including in WC core usage. ...because the calling code has insight into the purpose of the action, so it can set a smarter transient expiration time (e.g. some things should be cached for a day, others for no more than an hour).

I'm not familiar with any use cases where the expiration time is used like this. Every time I've used or seen as_next_scheduled_action()used it was checking whether a specific action was "scheduled" or "running" in the case of background jobs (e.g. AW jobs, WC Admin importer). At any rate it seems like it would be useful to have both functions available for developers.

There's still an issue of how long to set the cache for, but even a short lived expiry, like 1-2 minutes, could provide significant performance improvements by default, and any sites that want a longer expiry still have the option to use as_next_scheduled_action() directly with their our cache time.

I don't think there will be much of a measurable benefit between the proposed as_has_scheduled_action() query and caching the value as a transient unless the site has a massive number of AS actions rows and a normal number of WP options rows. However, I haven't actually done any tests.

Perhaps there's a role for persistent object caching here. I think cache invalidation will be important and could be difficult. I guess it would need to potentially invalidate the cache after every action run and when every action is created, updated or deleted.

I tend to agree with @crstauf that the caller should handle transient-style caching.

[barryhughes]

This looks good, thank you @danielbitzer!

We should go ahead and add to procedural_api_Test.php and probably also ActionScheduler_DBStore_Test (handling of a status array).

Does the WP Posts store need updating?

Less sure about this (though it's "legacy", both stores have maintained feature parity up until now, afaik), but will start a discussion and let you know what we come up with 👍

[WPprodigy]

Would it make sense to have an index that helps optimize for this? hook,status

[barryhughes]

Does the WP Posts store need updating?

Following up on this once more: yes, we'd need a matching change there (even if in that context it exists purely for compatibility reasons, vs a performance boost).

[danielbitzer]

@barryhughes

I've made a few changes:

• Added support for the WP Posts data store
• Added unit tests for the single vs array usage of the status param for the query_actions method
• Added unit tests for as_has_scheduled_action()
• Added a couple of extra unit tests for query_actions since there are none
• Tests are located in an abstract test class (\Action_Scheduler\Tests\DataStores\AbstractStoreTest) which is inherited by both ActionScheduler_DBStore_Test and ActionScheduler_wpPostStore_Test. This way the tests don't need to be duplicated like many tests are in these 2 test classes.

I found I had to run $wpdb->query( "DELETE FROM {$wpdb->actionscheduler_actions}" ); before each test for the custom table store to get the tests to pass.

[danielbitzer]

@barryhughes thanks for reviewing! I've updated everything based on your feedback.

[danielbitzer]

I've just documented the query params docs for query ::query_actions() in 6cebfd0

[barryhughes]

This is great, left some thoughts in (related) PR 752—figured it made sense to work through that first since it targets this branch.

[barryhughes]

Excellent stuff!

[barryhughes]

Updated the PR description to add a changelog entry, feel free to revise if needed.