Make WordPress Core

Opened 9 months ago

Last modified 26 hours ago

#61467 assigned defect (bug)

Inconsistent cache handling for network (site) meta

Reported by: xparham's profile xParham Owned by: spacedmonkey's profile spacedmonkey
Milestone: 6.9 Priority: normal
Severity: major Version: 3.0
Component: Cache API Keywords: has-testing-info dev-feedback has-patch
Focuses: multisite, performance Cc:

Description

It appears that there are multiple ways to work with wp_sitemeta and each use different cache keys and groups, resulting in inconsistent behavior and out-of-sync cached values.

One method is using update_site_option()/update_network_option() which uses "$network_id:$option" for the cache key, and site-options for the group.

The other method is update_metadata( 'site', ... ) which uses the object ID (here the network ID) for the cache key, and site_meta for the cache group.

Normally, in the code, we use update_site_option()/update_network_option(), however, WP CLI's wp network meta ... uses the *_metadata() functions, and when using both we end up with out of sync cached values.

Steps to reproduce the issue via WP CLI:

wp eval 'update_network_option( 1, "mykey", "123" );'
wp eval 'echo get_network_option( 1, "mykey" );' #Displays 123

wp network meta update 1 mykey 456
wp network meta get 1 mykey #Displays 456

wp eval 'echo get_network_option( 1, "mykey" );' #Still displays 123, but 456 is the expected value

Change History (16)

#1 @narenin
9 months ago

Hi @xParham ,

I am also able to replicate the same issue. The issue arises from the inconsistent use of different WordPress functions for updating site (network) options and metadata. These different functions use distinct caching mechanisms, leading to out-of-sync values between update_network_option()/get_network_option() and update_metadata()/get_metadata().

Understanding the Functions and Their Cache Groups

update_site_option() / update_network_option():

Cache Key: $network_id:$option
Cache Group: site-options
Usage: Typically used for storing network-wide options.

update_metadata('site', ...):

Cache Key: $object_id (where $object_id is the network ID)
Cache Group: site_meta
Usage: Typically used for more granular metadata associated with the site (network).

Why This Happens

When you update a network option using update_network_option(), it sets a value in the site-options cache group. However, if you then update the same value using update_metadata('site', ...), it sets a value in the site_meta cache group, bypassing the site-options cache group.

This discrepancy means that get_network_option() may retrieve stale data because it checks the site-options cache group, which hasn't been updated by update_metadata('site', ...).

Steps to Reproduce

Set value using update_network_option():

wp eval 'update_network_option( 1, "mykey", "123" );'

Cache Key: 1:mykey in site-options

Retrieve value using get_network_option():

wp eval 'echo get_network_option( 1, "mykey" );'

Output: 123

Set value using update_metadata('site', ...):

wp network meta update 1 mykey 456

Cache Key: 1 in site_meta

Retrieve value using get_metadata('site', ...):

wp network meta get 1 mykey

Output: 456

Retrieve value again using get_network_option():

wp eval 'echo get_network_option( 1, "mykey" );'

Output: 123 (still, because the site-options cache was not updated)

Solution: Invalidate Caches

One solution to ensure consistency is to manually invalidate or update both caches when you change a network option or metadata.

Example Code to Synchronize Caches:

<?php
function sync_network_option_and_meta($network_id, $key, $value) {
    // Update using update_network_option()
    update_network_option($network_id, $key, $value);
    
    // Update the metadata as well
    update_metadata('site', $network_id, $key, $value);

    // Clear both caches
    wp_cache_delete("$network_id:$key", 'site-options');
    wp_cache_delete($network_id, 'site_meta');
}


Usage in WP CLI

wp eval 'sync_network_option_and_meta(1, "mykey", "456");'

Ensuring Consistency in WP CLI Scripts

When writing WP CLI scripts or commands, ensure that any update to a network option also updates the corresponding metadata and clears the relevant caches.

WP CLI Command Example

wp eval 'function sync_network_option_and_meta($network_id, $key, $value) {
    // Update using update_network_option()
    update_network_option($network_id, $key, $value);
    
    // Update the metadata as well
    update_metadata("site", $network_id, $key, $value);

    // Clear both caches
    wp_cache_delete("$network_id:$key", "site-options");
    wp_cache_delete($network_id, "site_meta");
}
sync_network_option_and_meta(1, "mykey", "456");'

By ensuring that both the network option and site meta are updated and their respective caches are cleared, we can maintain consistent and synchronized values across our WordPress network.

#2 @narenin
9 months ago

  • Keywords has-testing-info dev-feedback added

#3 @xParham
9 months ago

@narenin Thanks for confirming this. I understand the workaround you have suggested, but I believe this should be fixed at the core level. The core should pick one method for working with wp_sitemeta and stick to it everywhere.

Perhaps in update_metadata() it can check if $meta_type === 'site' and then just bail early with a return update_network_option( $object_id, $meta_key, $meta_value );, and similarly in other *_metadata() functions. Or the other way, change update_network_option() to let update_metadata() handle the update.

#4 @narenin
9 months ago

Hi,

It could be like this.

<?php
// Modify update_metadata function
function update_metadata($meta_type, $object_id, $meta_key, $meta_value) {
    if ($meta_type === 'site') {
        return update_network_option($object_id, $meta_key, $meta_value);
    }

    // Existing code for other meta types...

    // Example of existing metadata handling code:
    // return update_meta($meta_type, $object_id, $meta_key, $meta_value);
}

// Modify update_network_option function
function update_network_option($network_id, $option_name, $newvalue) {
    // Call update_metadata to handle the update
    return update_metadata('site', $network_id, $option_name, $newvalue);
}

// Function to synchronize caches
function sync_network_option_and_meta($network_id, $key, $value) {
    // Update the option
    update_network_option($network_id, $key, $value);

    // Clear both caches to ensure consistency
    wp_cache_delete("$network_id:$key", 'site-options');
    wp_cache_delete($network_id, 'site_meta');
}

// Example usage in WP CLI
wp eval 'sync_network_option_and_meta(1, "mykey", "456");'

#5 @spacedmonkey
9 months ago

There was a ticket #37181 that was designed to get the *_network_option to use metadata api. However, this was merged in [54080] and reverted in [54637], it is generated performance regressions.

This issue how how WP CLI implements network options, not how core does it. A ticket should be created on the WP CLI repo.

#7 @peterwilsoncc
9 months ago

I think this should be resolved in WP Core. The problem is that the name of the tables used suggest that get_metadata( 'site', 1, 'siteurl', true ) is an appropriate way of getting the option. And the code works as expected.

It also presents a problem that it is possible to add duplicate options to the table using code such as add_metadata( 'site', 1, 'siteurl', '61467' ). Once such code is run, the option returned by get_site_option() will vary depending on the database engine being used.

Calls to update_site_option() once that happens can behave very strangely indeed. In my case update_site_option( 'siteurl', 'testing' ) returned true but the second copy of the option was updated to the value of the first.

This ticket was mentioned in Slack in #core-performance by mukeshpanchal27. View the logs.


9 months ago

This ticket was mentioned in PR #8161 on WordPress/wordpress-develop by @spacedmonkey.


2 months ago
#9

  • Keywords has-patch added

Add filters for site metadata handling in multisite setups

Introduce filters for get, update, add, and delete site metadata, specific to multisite installations. These functions extend metadata API compatibility while marking their use as deprecated in favor of network options functions. Warnings are added to guide developers toward preferred alternatives.

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

#10 @spacedmonkey
2 months ago

  • Milestone changed from Awaiting Review to Future Release
  • Version set to 3.0

#11 @spacedmonkey
2 months ago

I have created with a fix for this issue #8161.

This hook into the get_site_metadata, update_site_metadata, add_site_metadata and delete_site_metadata filters. These filters load early in there functions and will bypass loading via the metadata api and defaults to the network option. They also add a doing it wrong function call, to tell developers that they are doing something not recommended.

Can @peterwilsoncc @xParham take a look at the PR and see if you are happy with the approach, if you are happy, I can add unit tests and get this committed.

#12 follow-up: @flixos90
2 months ago

@spacedmonkey I would like to flag that this ticket conflicts with the intent of #37181.

For what it's worth, our attempts at implementing #37181 so far always ran into some blockers, and with the proposed direction to address this ticket, I think we should reconsider to close that ticket. Basically, it's either that ticket or this ticket that we need to go with. If we want to discourage using the *_metadata functions to access network options, we cannot at the same time switch the network option functions to use the *_metadata functions.

My vote goes for closing #37181.

#13 in reply to: ↑ 12 @spacedmonkey
2 months ago

Replying to flixos90:

@spacedmonkey I would like to flag that this ticket conflicts with the intent of #37181.

For what it's worth, our attempts at implementing #37181 so far always ran into some blockers, and with the proposed direction to address this ticket, I think we should reconsider to close that ticket. Basically, it's either that ticket or this ticket that we need to go with. If we want to discourage using the *_metadata functions to access network options, we cannot at the same time switch the network option functions to use the *_metadata functions.

My vote goes for closing #37181.

Sadly, I agree. I have marked #37181 as wontfix

@johnjamesjacoby commented on PR #8161:


8 weeks ago
#14

I disagree, that the doing it wrong messages are not needed. I think we want to deprecate the miss use of metadata api for network options.

Why do we want to do that?

Isn't it kind of nice to have two sets of functions here?

Options is an API with a specific intention, and Meta is a lower-level API without intention?

#15 @spacedmonkey
26 hours ago

  • Milestone changed from Future Release to 6.9
  • Owner set to spacedmonkey
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-multisite by spacedmonkey. View the logs.


26 hours ago

Note: See TracTickets for help on using tickets.