Make WordPress Core

Opened 4 years ago

Last modified 4 years ago

#51716 new defect (bug)

WP Cron - looses entries

Reported by: nate1's profile Nate1 Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Cron API Keywords: has-patch
Focuses: performance Cc:

Description

Hello, I've been working with an external API for submitting orders from Woocommerce. To ensure the system is responsive I've been calling the scheduled events to save the orders to the API shortly after the order is submitted.

I have noted that if items are saved at the same time. One of the events will not be saved.

This appears to be caused by the cron task being build around a single database record (object), and in many cases would be completely fine, but is not reliable when being used in any great capacity.

I understand my use-case isn't potentially what this was designed for, but there is room for improvement with the scheduled events to ensure reliability.

I'm not 100% sure if I'm correct here, but it also appears that these tasks are also run when subsequent web requests are made, is there any current normalized interface for setting this up to run via an OS cron task or similar. To ensure other visitors requests aren't slowed down by checks (small) and events being activated (potentially slow).

Attachments (1)

51716.diff (3.1 KB) - added by Takahashi_Fumiki 4 years ago.
Separate scheduling and execution on WP-Cron task.

Download all attachments as: .zip

Change History (8)

#1 follow-up: @johnbillion
4 years ago

  • Component changed from General to Cron API
  • Focuses performance added
  • Keywords reporter-feedback added

Thanks for the report, @Nate1.

To address your last point first, yes this is how the WP-Cron event system works. Info about WP-Cron here and info about how to use system cron here.

Scheduling multiple events in quick succession from a single process shouldn't be an issue. If you are seeing this problem then it would be great if you can provide some more information, a code example, information about your caching configuration, etc.

Scheduling multiple events simultaneously from more than one process -- for example when two requests hit your server at the same time or via two separate Ajax or REST API calls -- can indeed fail or overwrite one another due to the inherent problem caused by all events being stored in a single option that leads to a race condition with the read-then-write operations that the event scheduling functions perform. I maintain some info about this here.

#2 in reply to: ↑ 1 @Nate1
4 years ago

Replying to johnbillion:

Thanks for the report, @Nate1.

To address your last point first, yes this is how the WP-Cron event system works. Info about WP-Cron here and info about how to use system cron here.

Scheduling multiple events in quick succession from a single process shouldn't be an issue. If you are seeing this problem then it would be great if you can provide some more information, a code example, information about your caching configuration, etc.

Scheduling multiple events simultaneously from more than one process -- for example when two requests hit your server at the same time or via two separate Ajax or REST API calls -- can indeed fail or overwrite one another due to the inherent problem caused by all events being stored in a single option that leads to a race condition with the read-then-write operations that the event scheduling functions perform. I maintain some info about this here.

I can see it doesn't work, what I find a little troubling is that rather largely limited feature this seems to be acceptable?

Even with disabling the cron system, such as https://www.hostgator.com/help/article/how-to-replace-wordpress-cron-with-a-real-cron-job leaves the the same problem that two simultaneous requests could loose a cron task? The only solution being having to rebuild the wheel to have a reliable piece of software?

If this is something you've built, please don't be offended I do appreciate the work done and in part has served some purpose, but I would like to see a more robust implementation and am happy to help if needed.

#3 @johnbillion
4 years ago

  • Keywords reporter-feedback removed

I'd love to see this feature made more robust, but I expect that will require moving the cron event storage out of a single option in order to make it thread safe.

#4 @peterwilsoncc
4 years ago

#51747 was marked as a duplicate.

#5 follow-up: @peterwilsoncc
4 years ago

@Takahashi_Fumiki reports a similar issue in #51747 although rather than scheduling jobs in two processes, the race condition comes from scheduling while wp_cron is running.

#6 in reply to: ↑ 5 @Nate1
4 years ago

Replying to peterwilsoncc:

@Takahashi_Fumiki reports a similar issue in #51747 although rather than scheduling jobs in two processes, the race condition comes from scheduling while wp_cron is running.

I haven't run any tests on it to be 100% sure of the direct instance at where it occurs, but appears to be a simple transactional conflict. I don't think storing it in a single location is going to have much benefit, i.e. I often see a number of tasks in my setups, many which would be observed more than needed to be, as well as the risk of data being overwritten.

I'd propose if this was inline with the WordPress way of doing things (I'm rather new here), that it gets placed into its own DB table, based on a similar table structure to the object.
This would enable the next event/s to be relatively easily queried. So WHERE timetorun <= @TIMESTAMP.
Parameters could be useful in wp-config.php to limit the number of items to run on a given call LIMIT 5.

Just like this https://www.hostgator.com/help/article/how-to-replace-wordpress-cron-with-a-real-cron-job parameters could be used, such as removing the cron from being called on subsequent HTTP web calls, rather have an option to call the cron directly from the Operating System cron, to help make it more inline with the expected definition of cron.
define('DISABLE_WP_CRON', true);

  • Is there any concerns with doing it in the manner above?
  • Would using wp_options be better than a unique table for any reason?
  • WooCommerce - appears to have its own list of tasks that get run, I suppose that should be checked to see how that works, and if different better - see if there are any features in that method, that could be used.
  • Are there any other features that could be considered to be added to make it better, or more usable for WordPress users?

I have a fair bit of work on, but would be good to have some feedback then will look at designing an improved version and getting something submitted. I know Ill benefit from this alot, but would also be good to ensure there is nothing else that could be added to benefit everyone.

Thoughts?

Last edited 4 years ago by Nate1 (previous) (diff)

@Takahashi_Fumiki
4 years ago

Separate scheduling and execution on WP-Cron task.

#7 @Takahashi_Fumiki
4 years ago

  • Keywords has-patch added

I've uploaded a new ticket the changes are along with #51747

  1. Separate scheduling and execution
  2. Add hooks for before/after execution
  3. Add hook for cron-processor(default is do_action_ref_afrray)

With this patch, the race condition score improved but the fundamental problem(race condition) still remains. Below is the stats for 30min execution.

Patch All Scheduled Missed Schedule
NO 173 54
YES 133 4

To reproduce, see https://github.com/kuno1/nabeatsu-of-the-world

Note: See TracTickets for help on using tickets.