Make WordPress Core

Opened 4 years ago

Last modified 2 months ago

#20537 assigned defect (bug)

Don't spawn cron requests for suspended blogs

Reported by: ryan Owned by: jrf
Milestone: Future Release Priority: normal
Severity: normal Version: 3.3.1
Component: Cron API Keywords: has-patch good-first-bug needs-testing
Focuses: multisite Cc:

Description (last modified by swissspidy)

For multisite, spawning cron requests is wasteful and unnecessary. wp_cron()/spawn_cron()/wp-cron.php should check the blog's status and bail if the blog is suspended.

Attachments (3)

20537.diff (481 bytes) - added by ryan 4 years ago.
20537.2.patch (838 bytes) - added by kurtpayne 4 years ago.
Alternate patch
20537-with-filter.patch (2.8 KB) - added by jrf 2 months ago.
Refreshed patch, including filter which allows to force skip/force run cron anyhow.

Download all attachments as: .zip

Change History (13)

#1 @ryan
4 years ago

Cribbing from ms_site_check(), the conditions to check:

if ( '1' == $current_blog->deleted || '2' == $current_blog->deleted || '1' == $current_blog->archived || '1' == $current_blog->spam )

Or from get_blogs_of_user():

if ( $blog->archived || $blog->spam || $blog->deleted )
Last edited 4 years ago by ryan (previous) (diff)

4 years ago

#2 @nacin
4 years ago

I think spawning is unnecessary. wp-cron.php should still be able to be triggered, though. Someone somewhere is surely doing something weird with regards to this.

I think it would be best if bailed early in spawn_cron() and didn't hook wp_cron() directly into sanitize_comment_cookies. That would allow for wp_cron() and wp-cron.php to still execute.

I could also just be overly careful.

#3 follow-up: @ryan
4 years ago

  • Description modified (diff)

I can see it being good and bad, at least when considering core events. A hard freeze prevents future posts from accidentally getting published and pingbacks from accidentally going out from a blog that should be dead. The blog and the cron queue are frozen. Then again, there could be some events that should still be processed. This was requested by some wp.com folks to shut up zombie blogs. How dead is dead enough?

#4 @ryan
4 years ago

And, yes, spawning should be shut down and the hook avoided. Finishing the patch tomorrow.

4 years ago

Alternate patch

#5 @kurtpayne
4 years ago

  • Cc kpayne@… added
  • Keywords has-patch 2nd-opinion added

20537.2.patch is the same as 20537.diff, but in a different place. It prevents the cron request from spawning for zombie blogs, but still allows wp-cron.php to be called directly.

Still a bit new to the cron code. Is this right?

#6 in reply to: ↑ 3 @nacin
3 years ago

Replying to ryan:

How dead is dead enough?

I'm convinced. Both spawn_cron() and wp-cron should bail. Maybe a filter in wp-cron for the creative.

#7 @jeremyfelt
11 months ago

  • Focuses multisite added

#8 @swissspidy
2 months ago

  • Keywords needs-refresh good-first-bug added; 2nd-opinion removed

20537.2.patch is a good start. Needs a refresh using a new filter, so people could still spawn cron requests for these blogs when needed.

2 months ago

Refreshed patch, including filter which allows to force skip/force run cron anyhow.

#9 @jrf
2 months ago

  • Keywords needs-refresh removed

New patch uploaded which combines the previous two patches and adds a filter as suggested.

Both previous patches are needed if we want to bail early:

  • wp_cron() would be the earliest point of entry for the poor mans cron run
  • wp-cron.php for those of us who run real cron jobs.

#10 @swissspidy
2 months ago

  • Description modified (diff)
  • Keywords needs-testing added
  • Owner set to jrf
  • Status changed from new to assigned

Marking this "good-first-bug" as claimed.

Note: See TracTickets for help on using tickets.