Make WordPress Core

Opened 3 years ago

Closed 3 years ago

#54849 closed defect (bug) (fixed)

Site transients cause DB errors when installing

Reported by: peterwilsoncc's profile peterwilsoncc Owned by: hellofromtonya's profile hellofromTonya
Milestone: 5.9.1 Priority: normal
Severity: normal Version: 5.9
Component: Options, Meta APIs Keywords: has-patch commit fixed-major dev-reviewed
Focuses: Cc:

Description

Discovered while looking in to #54800 and #54801.

In WordPress 5.9 the *_site_transient() functions can be called during the installation process prior to the database tables been created. This only happens on WordPress configurations with multiple theme directory roots set via the $wp_theme_directories global.

create_initial_post_types(), _add_default_theme_supports() and wp_enable_block_templates() all call wp_is_block_theme() which ultimately calls get_theme_roots() and may try to retrieve/set a site wide transient.

Sample log entries:

[18-Jan-2022 03:20:24 UTC] WordPress database error Table 'wordpress.wp_sitemeta' doesn't exist for query SHOW FULL COLUMNS FROM `wp_sitemeta` made by include('phar:///usr/local/src/wp-cli/bin/wp/php/boot-phar.php'), include('phar:///usr/local/src/wp-cli/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php'), WP_CLI\bootstrap, WP_CLI\Bootstrap\LaunchRunner->process, WP_CLI\Runner->start, WP_CLI\Runner->load_wordpress, require('wp-settings.php'), do_action('setup_theme'), WP_Hook->do_action, WP_Hook->apply_filters, _add_default_theme_supports, wp_is_block_theme, wp_get_theme, get_raw_theme_root, get_theme_roots, search_theme_directories, set_site_transient, add_site_option, add_network_option

[18-Jan-2022 03:20:24 UTC] WordPress database error Table 'wordpress.wp_sitemeta' doesn't exist for query SHOW FULL COLUMNS FROM `wp_sitemeta` made by include('phar:///usr/local/src/wp-cli/bin/wp/php/boot-phar.php'), include('phar:///usr/local/src/wp-cli/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php'), WP_CLI\bootstrap, WP_CLI\Bootstrap\LaunchRunner->process, WP_CLI\Runner->start, WP_CLI\Runner->load_wordpress, require('wp-settings.php'), do_action('setup_theme'), WP_Hook->do_action, WP_Hook->apply_filters, wp_enable_block_templates, wp_is_block_theme, wp_get_theme, get_raw_theme_root, get_theme_roots, search_theme_directories, set_site_transient, add_site_option, add_network_option


[18-Jan-2022 03:20:24 UTC] WordPress database error Table 'wordpress.wp_sitemeta' doesn't exist for query SHOW FULL COLUMNS FROM `wp_sitemeta` made by include('phar:///usr/local/src/wp-cli/bin/wp/php/boot-phar.php'), include('phar:///usr/local/src/wp-cli/bin/wp/vendor/wp-cli/wp-cli/php/wp-cli.php'), WP_CLI\bootstrap, WP_CLI\Bootstrap\LaunchRunner->process, WP_CLI\Runner->start, WP_CLI\Runner->load_wordpress, require('wp-settings.php'), do_action('init'), WP_Hook->do_action, WP_Hook->apply_filters, create_initial_post_types, wp_is_block_theme, wp_get_theme, get_raw_theme_root, get_theme_roots, search_theme_directories, set_site_transient, add_site_option, add_network_option

The single site specific get_transient() and related functions include checks for wp_installing() that are lacking in the network wide get_site_transient() and related functions.


As this is super niche and requires multiple theme routes be set up, I've put this on 5.9.1 for consideration even though the bugs were introduced in the 5.9 cycle as part of full site editing.

cc @noisysocks, @hellofromtonya

Change History (22)

#2 in reply to: ↑ 1 @peterwilsoncc
3 years ago

Replying to noisysocks:

Would returning to the way that wp_is_block_theme() was implemented prior to [52330] fix this?

I don't think so, it looks like the older version attempts to get, and if needs be set, the same site wide transient via get_theme_file_path() > get_(stylesheet|template)_directory() > get_theme_root() > get_raw_theme_root() > get_theme_roots().

#3 @antonvlasenko
3 years ago

I'm working on this issue.

#4 @antonvlasenko
3 years ago

  • Keywords reporter-feedback added

@peterwilsoncc
I can't reproduce this bug.
I've tried to add additional theme roots to wp-settings.php like that:

// Register the default theme directory root.
register_theme_directory( get_theme_root() );
register_theme_directory(‘/‘wp/themeroot1’);
register_theme_directory(‘/‘wp/themeroot2’);

Then, I installed WordPress using WP-CLI.
I didn't notice any database errors.
Am I missing something?
Would you be able to add steps on how to reproduce this bug?
I really appreciate any help you can provide.

Last edited 3 years ago by antonvlasenko (previous) (diff)

#5 follow-up: @peterwilsoncc
3 years ago

  • Keywords reporter-feedback removed

For the configuration I was using, the theme directories were defined in the wp-config.php file.

The key portion of my config file follows, I use a custom content directory but any valid theme directory should work:

<?php
define( 'WP_DEBUG_LOG', true );

## THEME DIRECTORIES
if ( empty( $GLOBALS['wp_theme_directories'] ) ) {
        $GLOBALS['wp_theme_directories'] = array();
}
if ( file_exists( WP_CONTENT_DIR . '/themes' ) ) {
        $GLOBALS['wp_theme_directories'][] = WP_CONTENT_DIR . '/themes';
}
$GLOBALS['wp_theme_directories'][] = ABSPATH . 'wp-content/themes';

This will cause WP to attempt to write the transient during the install process.

This ticket was mentioned in Slack in #core-test by peterwilsoncc. View the logs.


3 years ago

#7 in reply to: ↑ 5 ; follow-up: @tnolte
3 years ago

Replying to peterwilsoncc:
So we have a setup where the WP_CONTENT_DIR is being set because we install WordPress Core in a subdirectory using Composer. Seeing the details in this report I can see how this didn't get caught since this customization is probably atypical for a new install.

This ticket was mentioned in PR #2250 on WordPress/wordpress-develop by peterwilsoncc.


3 years ago
#8

  • Keywords has-patch added

Trac ticket: https://core.trac.wordpress.org/ticket/54849

Proof of concept to disable the use of transients during installation as they rely on a database that hasn't been installed.

During installation, transients are set to always use wp_cache_*() functions. For sites with a persistent cache, nothing changes, for sites without a persistent cache, the transient is stored in memory until the end of the request.

Prior to opening this pull request, my prediction is this will cause a number of tests to fail.

#9 in reply to: ↑ 7 @peterwilsoncc
3 years ago

Replying to tnolte:

So we have a setup where the WP_CONTENT_DIR is being set because we install WordPress Core in a subdirectory using Composer. Seeing the details in this report I can see how this didn't get caught since this customization is probably atypical for a new install.

Thanks for your understanding, I use such an environment for my wordpress-develop environment but missed it because I don't often need to do a fresh install.

In your blog post, I see you were getting the error with options tables rather than sitemeta tables -- that's caused by a difference with the storage of site wide transients in multi-site vs single-site mode.


In the linked pull request I've got a proof of concept to prevent transients from using the database if wp_installing() === true. This ought to prevent the database errors during installation.

My inclination is that it will need additional work, especially for site transients on a multisite install.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#11 follow-up: @audrasjb
3 years ago

@peterwilsoncc do you think this PR will be ready in the next couple days?

This ticket was mentioned in Slack in #core by peterwilsoncc. View the logs.


3 years ago

#13 in reply to: ↑ 11 @peterwilsoncc
3 years ago

Replying to audrasjb:

@peterwilsoncc do you think this PR will be ready in the next couple days?

It appears to work, so I think I am happy with it but would like a review from another committer.

The bug has been around for a while, it was brought to light by 5.9 using transients (indirectly) during the installation process so some thoughts on whether it should be backported would be good to.

#14 @peterwilsoncc
3 years ago

  • Keywords commit added

@dd32 gave me a logic check on this yesterday and is happy with the approach, marking this ready for commit.

#15 @peterwilsoncc
3 years ago

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

In 52694:

Options: Disable transients while installing.

Prevent the transient setters and getters from attempting to use the database table before they exist during the installation process.

During installation transients now use the wp_cache_*() functions on all sites, including those without a persistent cache, to prevent database errors. The use of the caching functions stores the data in memory for the duration of the request to account for transient data that is used multiple times during installation.

Props dd32, audrasjb, tnolte, antonvlasenko, noisysocks, peterwilsoncc.
Fixes #54849.

#16 @peterwilsoncc
3 years ago

  • Keywords fixed-major added
  • Resolution fixed deleted
  • Status changed from closed to reopened

Reopening for 5.9.1 consideration.

As noted above, the bug has been around for a while but was brought to light by the indirect use of transients in some circumstances by wp_is_block_theme().

It's quite an edge case, as it requires the site use multiple theme roots per comment #7.

#18 @hellofromTonya
3 years ago

  • Keywords dev-reviewed added

Hmm, this bug was not introduced in 5.9.0 but as @peterwilsoncc notes it

was brought to light by the indirect use of transients in some circumstances by wp_is_block_theme().

I reviewed and LGTM! But will the final decision about the backport to @audrasjb for 5.9.1 inclusion.

Last edited 3 years ago by hellofromTonya (previous) (diff)

#19 @audrasjb
3 years ago

This looks good to go on my side as well! :)

#20 @hellofromTonya
3 years ago

  • Owner changed from peterwilsoncc to hellofromTonya
  • Status changed from reopened to reviewing

Thanks @audrasjb. I'll self-assign and get it committed.

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 years ago

#22 @hellofromTonya
3 years ago

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

In 52767:

Options: Disable transients while installing.

Prevent the transient setters and getters from attempting to use the database table before they exist during the installation process.

During installation transients now use the wp_cache_*() functions on all sites, including those without a persistent cache, to prevent database errors. The use of the caching functions stores the data in memory for the duration of the request to account for transient data that is used multiple times during installation.

Props dd32, audrasjb, tnolte, antonvlasenko, noisysocks, peterwilsoncc.
Merges [52694] to the 5.9 branch.
Fixes #54849.

Note: See TracTickets for help on using tickets.