WordPress.org

Make WordPress Core

Opened 9 years ago

Closed 9 years ago

#12140 closed enhancement (fixed)

Reduce DB queries during install by limiting use of wp_load_alloptions()

Reported by: donncha Owned by: dd32
Milestone: 3.0 Priority: high
Severity: blocker Version:
Component: Upgrade/Install Keywords: dev-feedback
Focuses: multisite Cc:

Description

This is http://trac.mu.wordpress.org/ticket/1192 on the original MU Trac.

Creating a new blog through the signup form uses about 700 queries. This patch reduces this by about half by not populating the option cache all the time.

This patch applies to the MU 2.9 branch.

Attachments (1)

functions.diff (4.2 KB) - added by donncha 9 years ago.

Download all attachments as: .zip

Change History (29)

@donncha
9 years ago

#1 @nacin
9 years ago

Closing #11997 as a duplicate of this.

#2 @wpmuguru
9 years ago

(In [12973]) Reduce DB queries by half during MU signup, props donncha, see #12140

#3 @donncha
9 years ago

Applied the patch above in http://trac.mu.wordpress.org/changeset/2072 and works for me and wpmuguru. Leaving this open for more feedback.

#4 @nacin
9 years ago

  • Component changed from Multisite to Upgrade/Install
  • Keywords dev-feedback added; signup removed
  • Owner set to dd32
  • Priority changed from normal to high
  • Summary changed from Reduce DB queries by half during MU signup to Reduce DB queries by half during blog install by limiting use of wp_load_alloptions()

This applied not only to MU signup but to regular WP installs as well. This does cut down the number of queries, though I'm curious what other effects it may have and it deserves a second look.

Cross referencing #12144 -- it doesn't look like this was tested with WP_DEBUG, and some notices snuck in.

#5 @nacin
9 years ago

  • Keywords multisite added
  • Milestone set to 3.0
  • Summary changed from Reduce DB queries by half during blog install by limiting use of wp_load_alloptions() to Reduce DB queries during install by lmiting use of wp_load_alloptions()

#6 @nacin
9 years ago

  • Summary changed from Reduce DB queries during install by lmiting use of wp_load_alloptions() to Reduce DB queries during install by limiting use of wp_load_alloptions()

(Fixing spelling mistake in summary -- I'll get it right eventually.)

#7 @dd32
9 years ago

Has anyone noticed any side effects that might be related yet?

#8 @wpmuguru
9 years ago

(In [13445]) cleanup warnings - add site, See #12140

#9 @wpmuguru
9 years ago

That was the notices when adding a site/blog in a network scenario. I've created 20+ sites since applying donncha's patch with no issues. I did not test doing a new install with WP_DEBUG.

#10 @nacin
9 years ago

I was curious whether we did any damage to the non-multisite install process... Doesn't appear to have any side effects, but my head hurts when I start going through the install step by step.

#11 @wpmuguru
9 years ago

On that MU ticket there was a log of the queries - kind of whoa.

dd32: if you do a fresh install with DEBUG and get no warnings, I'd suggest closing the ticket.

#12 @wpmuguru
9 years ago

  • Resolution set to fixed
  • Status changed from new to closed

By this time we must have had lots of installs & have had no reported issues. Closing pending a reported issue.

#13 @westi
9 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Found an issue with this :-)

wordpress-tests does everything with WP_INSTALLING defined (unfortunately)

And this has broken cron in that environment.

Basically get_option is caching on fetch from db but update_option isn't overwriting it.

#14 @westi
9 years ago

(In [14502]) Don't cache option values while installing. See #12140.

#15 @westi
9 years ago

  • Cc westi added
  • Severity changed from normal to major

That papers over the cracks so the API is consistent.

Needs more review to ensure that this is correctly implemented

#16 @wpmuguru
9 years ago

(In [14515]) consistency & simplicity in get_option(), see #12140

#17 @ryan
9 years ago

Fixed?

#18 follow-up: @nacin
9 years ago

A hunch, this may be breaking plugin inclusion on wp-activate.

#19 @nacin
9 years ago

This did indeed break plugin inclusion on wp-activate.php.

#21 follow-up: @nacin
9 years ago

And #12022, which is the MU 1151 ticket copied over.

#22 in reply to: ↑ 21 @eddiemoya
9 years ago

#12022 started in the forums, and more conversation took place there. http://mu.wordpress.org/forums/topic/15324?replies=11

#23 @ryan
9 years ago

  • Severity changed from major to blocker

#24 @eddiemoya
9 years ago

  • Cc eddiemoya added

#25 @jpotkanski
9 years ago

  • Cc jpotkanski added

Watching.

#26 in reply to: ↑ 18 ; follow-up: @westi
9 years ago

Replying to nacin:

A hunch, this may be breaking plugin inclusion on wp-activate.

Plugins aren't included in wp-activate because it has define( "WP_INSTALLING", true ); at the top and we don't include plugins while installing.

#27 in reply to: ↑ 26 @eddiemoya
9 years ago

Replying to westi:

Plugins aren't included in wp-activate because it has define( "WP_INSTALLING", true ); at the top and we don't include plugins while installing.

That is correct, and we figured out that removing that WP_INSTALLING allows the inclusion of plugin function. They are necessary because some themes depend on plugin functions to generate parts of the header/footer, which wp-activate pulls in.

The only thing I am not certain of is what the negative effects of removing that constant are. It was put there for a reason, and taking it out must have some effect. I will do some testing but perhaps someone else already knows the answer. Some discussions elsewhere lead my to presume that its removal will negatively effect the activation process itself somehow.

#28 @ryan
9 years ago

  • Resolution set to fixed
  • Status changed from reopened to closed

WP_INTALLING prevents certain cache operations that can lead to bad values being inserted into blogs during install_blog(). The answer here is probably to have separate constants that control loading plugins and shorting cache operations. Activation, I believe, can get away with allowing plugins to load.

Ideally, plugins that mess with activation would be mu-plugins and avoid the whole mess. Such plugins seem perfect candidates for being in mu-plugins.

This issue predates 3.0 and is not a result of this ticket. Several releases ago plugins stopped being loaded when WP_INSTALLING to accommodate auto upgrades.

Since this is not a 3.0 regression and requires a good amount change and re-testing to remedy, this is not going to be fixed for 3.0. I re-opened #12022 against the 3.1 milestone. There we can fracture WP_INSTALLING into separate constants for controlling cache shorting, plugin loading, installation, and activation.

Note: See TracTickets for help on using tickets.