WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 8 months ago

#26410 closed defect (bug) (fixed)

Updating or deleting options via update_option or delete_option leaves blog-details cache object stale

Reported by: codix Owned by: wonderboymusic
Milestone: 4.0 Priority: normal
Severity: normal Version: 3.8
Component: Options, Meta APIs Keywords: has-patch commit
Focuses: multisite, performance Cc:

Description

Updating or deleting 'blogname', 'siteurl', 'post_count' options using update_option or delete_option leaves blog-details cache object stale.

Attachments (4)

patch.diff (1.9 KB) - added by codix 17 months ago.
26410.diff (1.9 KB) - added by kovshenin 11 months ago.
26410.2.diff (1.5 KB) - added by pento 8 months ago.
26410.3.diff (1.9 KB) - added by nacin 8 months ago.

Download all attachments as: .zip

Change History (19)

@codix17 months ago

comment:1 @codix17 months ago

  • Component changed from General to Multisite

comment:2 @ryan17 months ago

The *-blog_option cache hasn't existed since 3.4. See [21357].

comment:3 follow-up: @jeremyfelt16 months ago

Without the *-blog_option cache portions of the patch, it looks like the intent is to use refresh_blog_details() whenever blog_name, siteurl, or post_count are updated.

Are there specific places in core where these options are updated without refreshing the cache? It seems like we would want to nail that down rather than adding an action on updated_option.

comment:4 @codix16 months ago

It was some plugins that were chaging blog_name leaving blog details stale.

Since you can change blog_name, siteurl and post_count with update_option which will leave blog details stale. IMO it makes sense to refresh the blog details when these options are updated.

Thanks!

comment:5 in reply to: ↑ 3 @uglyrobot16 months ago

Replying to jeremyfelt:

Are there specific places in core where these options are updated without refreshing the cache? It seems like we would want to nail that down rather than adding an action on updated_option.

Settings->General (blog_name), update_posts_count() in ms-functions.php, and in the "Edit Site" feature where you can change any option.

comment:6 @jeremyfelt15 months ago

  • Component changed from Multisite to Options and Meta
  • Focuses multisite performance added

@kovshenin11 months ago

comment:7 @kovshenin11 months ago

  • Keywords has-patch added

Took a stab in 26410.diff. I was also going to remove refresh_blog_details() from update_blog_option() since it'll be triggered anyway, but I'm not sure it isn't going to break anything else, so left it as is for now.

comment:8 @wonderboymusic10 months ago

  • Keywords needs-testing added
  • Milestone changed from Awaiting Review to 4.0

comment:9 @wonderboymusic10 months ago

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

In 28881:

In multisite, on the updated_option action, if the option name is one of: 'blogname', 'siteurl', 'post_count' - refresh the blog details cache for the current blog id.

Adds unit test.

Props kovshenin.
Fixes #26410.

comment:10 @nacin8 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

This seems unnecessarily slow and complicated. Would it be better to have this?

add_action( 'update_option_blogname', '_wp_refresh_blog_details_on_updated_option' );
add_action( 'update_option_siteurl', '_wp_refresh_blog_details_on_updated_option' );
add_action( 'update_option_post_count', '_wp_refresh_blog_details_on_updated_option' );

@pento8 months ago

comment:11 @pento8 months ago

attachment:26410.2.diff uses the specific action names to run the update function.

This could be cut down even further if we added a get_current_blog_id() call to refresh_blog_details().

ie:

if ( empty( $blog_id ) ) {
    $blog_id = get_current_blog_id();
}

But I'm not sure if that would have any unintended side effects.

@nacin8 months ago

comment:12 @nacin8 months ago

  • Keywords commit added; needs-testing removed

refresh_blog_details() passes its $blog_id argument directly to get_blog_details(), which when given a scalar that evaluates to empty (so, 0) it ends up using get_current_blog_id(). Making it use get_current_blog_id() makes a lot of sense.

I like 26410.3.diff.

comment:13 @ircbot8 months ago

This ticket was mentioned in IRC in #wordpress-dev by nacin. View the logs.

comment:14 @pento8 months ago

26410.3.diff works for me, too.

comment:15 @SergeyBiryukov8 months ago

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

In 29668:

Simplify the code for calling refresh_blog_details() whenever 'blogname', 'siteurl', or 'post_count' option is updated.

props pento, nacin.
fixes #26410.

Note: See TracTickets for help on using tickets.