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

Spike: Can we add caching to as_next_scheduled() and as_has_scheduled_action()? #1240

Open
prettyboymp opened this issue Jan 27, 2025 · 1 comment
Labels
priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: enhancement The issue is a request for an enhancement.

Comments

@prettyboymp
Copy link
Contributor

Issue

There is a common pattern amongst WooCommerce core and extensions to guarantee that actions are scheduled by running code similar to:

if ( false === as_next_scheduled_action( 'my_custom_action' ) ) {
	as_schedule_single_action( time(), 'my_custom_action' );
}

With multiple extensions using ActionScheduler, this can quickly add 20 or more extra queries to every wp-admin page load. Not only that, the performance of the as_schedule_single_action() query can get pretty slow on stores with heavy action scheduler usage, e.g. those running subscription renewals.

Ideally all these extensions would only ever attempt to schedule these actions during updates or periodically to make sure they are working correctly, but that isn't happening in reality and expecting all the extensions that use ActionScheduler to change their pattern isn't likely going to happen.

Proposal

Add caching around the underlying queries within as_next_scheduled() and as_has_scheduled_action().

Because the query vars for those methods can vary quite a bit, we likely won't be able to cache every combination. However, we should be able to cache the result in the most common scenarios, e.g. when the $args are empty or the default values and when only a $hook or a $hook and $group parameter are used.

When an action is modified, we should be able to purge the cache specific to that action's $hook, and $hook/$group combination.

Compatibility Concerns

The simplest implementation of caching would be at the top level as_.. functions. However, ensuring cache is properly purged when a change occurs is more difficult at this layer because the underlying implementation may bypass cache clearing triggers. So the safest implementation will probably be to put the caching within the datastores themselves. This still holds a slight risk if a certain implementation runs actions in the background and bypasses the datastore, but the positive community impact should be evaluated in the decision.

@prettyboymp
Copy link
Contributor Author

related woocommerce/woocommerce#53564

@albarin albarin added the type: enhancement The issue is a request for an enhancement. label Feb 6, 2025
@prettyboymp prettyboymp added the priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. label Feb 6, 2025
@kalessil kalessil moved this from Needs triage to Collaboration in WC Performance 🚀🚀 Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high The issue/PR is high priority—it affects lots of customers substantially, but not critically. type: enhancement The issue is a request for an enhancement.
Projects
Status: Collaboration
Development

No branches or pull requests

2 participants