Make WordPress Core

Opened 14 years ago

Closed 5 years ago

#13158 closed defect (bug) (wontfix)

Cron : some events may not be scheduled

Reported by: arena's profile arena Owned by: westi's profile westi
Milestone: Priority: normal
Severity: normal Version: 3.0
Component: Cron API Keywords:
Focuses: Cc:

Description

the problem appears when two different events are scheduled at the same time

what happens in code is :

1st event : _get_cron_array();
2nd event : _get_cron_array();

1st event : _set_cron_array( $crons );
2nd event : _set_cron_array( $crons );

1st event is lost.

Change History (20)

#1 @mdawaffe
14 years ago

The root problem is that we store the cron data in an array. A naive get, process, set algorithm will always be prone to race conditions.

We can rework those scheduling functions to move as much of the processing before the get so that there's less time between the get and the set, but that won't solve the problem. Most of the bottleneck is probably in the db write, so reducing the time spent in PHP between get and set probably won't help that much.

We could try putting in some locking (in the application layer) and maybe making a queue of stuff that needs to be added (one per row or we're back to where we started... transients?), but I don't think that will guarantee we prevent the race condition. More complicated and easy to just move the race condition from one area to another.

#2 @ryan
14 years ago

  • Milestone changed from 3.0 to Future Release

#3 @arena
14 years ago

  • Milestone changed from Future Release to 3.0

Best would be to convert cron arrays in a set of rows in options table with same meta_key, meta_value containing timestamp, hook, args


#4 @nacin
14 years ago

The options table doesn't have "meta_key" or "meta_value." Option names must be unique.

We will not do any sort of restructuring here this close to release. Moving back to future.

#5 @nacin
14 years ago

  • Milestone changed from 3.0 to Future Release

#6 @arena
14 years ago

  • Version changed from 3.0 to 3.1

#7 @mintindeed
13 years ago

  • Cc gabriel.koen@… added

Why does it have to stay in the wp_options table? Looking at implementations of a cron or job queue (I'm thinking Magento at the moment, but there are other examples), the implementations that are built for concurrency and volume use a separate table for maintaining the queue. That seems like it would be appropriate here as well.

#8 @mintindeed
13 years ago

It looks like #15148 would fix this by extension, allowing those to whom concurrency is an issue to implement their own cron storage and queueing options, a la HyperDB.

#9 @vickybiswas
13 years ago

A Method I used successfully for this specific case maybe helps here

/**
 * Clean Cron from cache so that fresh is fetched
 *
 * @since 2011-05-18 Vicky Biswas
 */
add_action( 'pre_update_option_cron', 'mmc_pre_update_option_cron',10 );
function mmc_pre_update_option_cron() {
	$alloptions = wp_load_alloptions();
    	if ( isset( $alloptions['cron'] ) ) {
		unset ($alloptions['cron']);
		wp_cache_set( 'alloptions', $alloptions, 'options' );
    	} else
        	wp_cache_delete( 'cron', 'options' );
    	return false;
}

#10 @agupta_pmc
13 years ago

  • Cc agupta_pmc added

#11 @obenland
12 years ago

FWIW, I wouldn't consider this a bug, but rather a misusage of the API. _get_cron_array() and _set_cron_array() are both private functions, which shouldn't be called directly anyway.

The phenomenon will not occur when one of the wp_schedule_*_event() wrappers are used.

#12 @arena
12 years ago

@obenland, as long as the api is not using some kind of semaphore this can occur !

#13 follow-up: @obenland
12 years ago

I agree! :)

But it would be a blunt disregard of the API and the purpose of these functions, which shouldn't be supported by making it work, IMO.

#14 in reply to: ↑ 13 @arena
12 years ago

Replying to obenland:

I agree! :)

so reconsider your first message as 'this is a misusage of the API'.
This is a bug !

#15 @wonderboymusic
9 years ago

  • Keywords needs-patch added

#16 @chriscct7
8 years ago

  • Priority changed from high to normal
  • Severity changed from major to normal
  • Version changed from 3.1 to 3.0

#17 @dd32
7 years ago

#39924 was marked as a duplicate.

#18 @danielhuesken
7 years ago

Littel bit better solution as this from @vickybiswas because we don't need the getting of alloptions.

<?php
add_filter( 'pre_option_cron', 
        function( $value, $option ) {
                global $wpdb;

                $row = $wpdb->get_row( $wpdb->prepare( "SELECT option_value FROM $wpdb->options WHERE option_name = %s LIMIT 1", $option ) );
                if ( is_object( $row ) ) {
                        $value = \maybe_unserialize( $row->option_value );
                }

                return $value;
        },
        10,
        2
);

#19 @ocean90
6 years ago

#41098 was marked as a duplicate.

#20 @peterwilsoncc
5 years ago

  • Keywords needs-patch removed
  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from new to closed

Creating a custom storage system for sites with a large number of scheduled tasks is now possible with the changes introduced in WordPress 5.1, please refer to the dev note on the Make WordPress Core P2.

I'll close this as wontfix, as it's a scaling issue best fixed via a plugin for large sites.

Note: See TracTickets for help on using tickets.