Use metadata api in *_network_options

[spacedmonkey]

The network (site) options are stored in the database as sitemeta. The table is formatted as a meta table. However the CRUD of this data in get_network_option, update_network_option, add_network_option and delete_network_option doesn't use the metadata api. Using the metadata api has many advantages, such as filters and a more consistent caching api.

[swissspidy]

Note that the default_site_option_ filter needs to be kept in order to not break BC. This includes the documentation for this hook.

[sc0ttkclark]

This could utilize the stuff in WP 4.6 for register_meta() now too:

register_meta( 'site', 'my_site_option_name', array( … ) );

For testing, you'd also need to add 'site' to the array in the new wp_object_types filter. However, in the patch, you'd add 'site' in the wp_object_types array within wp_object_type_exists() in the wp-includes/functions.php file.

[johnjamesjacoby]

Related: #25344.

[spacedmonkey]

I have been looking at this ticket again. There seems to be existing unit tests for the *_network_option functions. I believe me and @jeremyfelt worked on those original tests. The tests seem good to me. We will need some more test, but I would like some advise on what those tests look like.

Related: #28290

[flixos90]

Briefly summarizing today's office-hours proposed goals:

• Introduce *_network_meta() functions using the Metadata API for direct interaction with the sitemeta table.
• Add support for meta_query in WP_Network_Query.
• Use the new functions in the *_network_option() functions. This gives performance benefits and allows us getting rid of the alloptions mess. The *_network_option() functions will continue to maintain their current hooks for BC, while the *_network_meta() functions will be clean as other object metadata functions.
• Encourage using the new functions and use *_network_meta() in Core going forward. The *_network_option() functions will stay around for BC though, and should not be deprecated.
• This will also benefit a future /networks REST API endpoint.

See #37923 for related plans of introducing a blogmeta table and surrounding *_site_meta() functions.

[johnjamesjacoby]

I'd like to make a motion to close this ticket in favor of recent activity on #25344.

I believe doing so will concentrate our efforts and be a bit simpler for everyone to pay attention to.

[flixos90]

Replying to johnjamesjacoby:

I'd like to make a motion to close this ticket in favor of recent activity on #25344.

I believe doing so will concentrate our efforts and be a bit simpler for everyone to pay attention to.

+1

[spacedmonkey]

Duplicate of #25344.

[spacedmonkey]

In 37181.2.diff are the following changes

• wp_load_core_site_options now uses get_current_network_id to bring into line with _network_option functions.
• serialize functions returned to _network_option functions which was stopping tests from passing.
• delete_network_option uses delete_metadata and passes 4 param as an empty string, as per meta api.
• meta.php and formatting.php moved to earlier in bootstrap process because network options loaded ealier in bootstrap.
• Test added to prove that first value in meta store is honored.
• New update_network_meta_cache param added to wp_network_query. Network options are primed in cache, like other query classes.
• Removed test_site_notoptions test, as no longer valid.
• Documentation. Poke @DrewAPicture for feedback.

[spacedmonkey]

In 37181.4.diff

• Introduce the cache group of site_meta as a global group
• Include site_meta in clean_network_cache

[spacedmonkey]

37181.6.diff

Fixes some styling issues with the comments and the code. It now is WP code standards. Also there has been changed to variables since original patch.

One thing to note about this change how the meta api handles caching. The meta api loads all the meta for an object into one cache key. This means, when you request one option, it then loads all options in memory / object cache. Personally, I think this is a good thing, you are effectively autoloading all options, similar to how options works in core. But it may have effects on performance in different use cases. In WP_Network_Query, the network option (Meta) cache is primed. It is reasonable to guess, that if you are query networks that you will likely want the meta as well. As Network query is used in the bootstrap, the network option will be cache will be primed much easier.

[spacedmonkey]

Related: #27935

[spacedmonkey]

In the last patch, is a small performance improvement.

Unsetting the following
update_network_cache
update_network_meta_cache

Means cache key is unchanged on these query variable

[jeremyfelt]

Thanks for your work on this, @spacedmonkey.

37181.8.diff​ is updated against current trunk to account for the changes in [42343].

A couple notes so far:

• We're switching from site-options to site_meta as the cache group, but still using site-options for a handful of other things—current_network, networks_have_paths, etc... This is probably okay because we weren't ever storing those "options" in the DB.
• We'll want to make sure once this is in that external object cache drop-ins are aware/updated with the new global cache group.
• In general, I think our network option tests could use some work. For something like this we should write some more tests that rely on an external object cache.
• Is it possible that anyone is relying on notoptions right now for some reason? May need to do some digging through plugins briefly to see if anything stands out.

I'll spend some more time with this and think through the testing a bit more.

[jeremyfelt]

One more - it'd also be helpful to know if anyone is relying on the site-options cache group for one of these values in their sunrise.php or elsewhere with a direct call to cache.

[spacedmonkey]

Replying to jeremyfelt:

• We're switching from site-options to site_meta as the cache group, but still using site-options for a handful of other things—current_network, networks_have_paths, etc... This is probably okay because we weren't ever storing those "options" in the DB.

I think we need to support the site-options cache group forever more. It is used else where in core and I am personally use it for some plugin. I see no harm in leaving it.

• We'll want to make sure once this is in that external object cache drop-ins are aware/updated with the new global cache group.

Most modern object cache drops implement the wp_cache_add_global_groups, which should do this for us. Examples, can found, ​here, ​here, ​here, ​here and ​here. These are the largest plugins that I know of. Older ones may not have it implement it, but that will break other things and is not a core problem.

• In general, I think our network option tests could use some work. For something like this we should write some more tests that rely on an external object cache.

Any ideas of this @jeremyfelt ?

• Is it possible that anyone is relying on notoptions right now for some reason? May need to do some digging through plugins briefly to see if anything stands out.

A quick look of github, shows all the refrences to it, are people checking core into GH. I havent found a plugin in the wild that uses

[spacedmonkey]

Some things to not

• All options will be loaded in one cache key. This may effect people with lots of network options, however, not many people use network options, it is unlike to be a problem.
• Even through all options are loaded at once, it is not effected by the alloptions issue, detailed in #31245 . This because the meta api, does a delete instead of update. See ​here
• Network options will now be primed in WP_Network_Query, meaning network options will be loaded

in the bootstrap, which I think is is a good thing.

• In my patch, I run maybe_serialize before passing to the meta api. Howeve the meta api also serialize. I dont think there is a problem with double serializing something?
• Many of the existing filters could be moved out of the filters / actions in these could be moved out to there own functions. Consider the following example.

  add_filter('get_site_metadata', function($value, $network_id, $option){
     $pre = apply_filters( "pre_site_option_{$option}", false, $option, $network_id );
     if ( false !== $pre ) {
          $value = $pre;
     }
     return $value;
  }, 5, 3);
  
  

This allows for backward compat, but also if to remove those filters if you don't wish to use them anymore. This may over complex things.

• clean_network_cache now clears network options, which I think it should be doing already. This means that the ​delete_network function in wp-multi-network, it work better. ( Good work @johnjamesjacoby on that one ).

[spacedmonkey]

Related: #43506 #31147

[spacedmonkey]

In 37181.10.diff I updated the patch after coding standard changes.

There are now, 3 tests that pass. If you could take a look, @jeremyfelt that would be great :D

[spacedmonkey]

As part of the slack chat on this ticket, I have added a related ticket #43941 for meta data default values.

[pento]

This ticket needs reviewing and a decision.

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

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

Closing in favour of ​https://github.com/WordPress/wordpress-develop/pull/2478

[spacedmonkey]

I spent some time on this ticket refreshing it so that is applies clearly to core. There is a new ready for review at ​#2478.

[spacedmonkey]

Before patch applied

[spacedmonkey]

After patch is applied

[spacedmonkey]

Before patch ( 14 queries to site table )

[spacedmonkey]

After patch ( 14 queries replaced with 1 query )

[spacedmonkey]

Spent some more time on this ticket lately. I think this patch should go into core for the following reasons.

• Priming caches for all network options in one query, reduces page load time and number of queries run on the page. See attached screenshots, that removed up to 14 queries per page ( in network admin ). This is a massive performance win.
• With improvement to the register_meta function in earlier versions of WP, it now means that default values for a meta key can defined globally. Meaning that a network option could be register with a default value and type. There are a series of filter in the meta api that would be extremely useful for handling network options ( meta ). This opens up option in the future to have functions like register_network_setting ( a wrapper around register_meta ). This will bring network options more inline with options, see register_setting.
• Remove the need for the 'site-options' cache group.
• Remove need for notoptions cache.
• Improve consistency with how other meta types handle meta data.
• Removal of lots of code.
• Open door to reuse meta REST API controller, to handle CRUD in network options.

It is worth noting that is change does not close the door to #25344 in the future and is a good first step towards its.

[peterwilsoncc]

This may seem tangentially related but I'm going to mention it anyway...

Something I've noticed with using the *_network_option() functions on a single site is that they fall back to the *_option() function. Which makes some sense.

Where it becomes problematic on a single site install is that there is no way to set an option to be autoloaded via these functions. As a result anything using this shortcut ends up requiring a database query per option (again, on a single site install).

A good example of this is the salting values when stored in the database rather than within wp-config.php: this results in four extra database queries on each page load.

My question is, does making this change lock this problem in as unable to be solved?

@peterwilsoncc Will confused what what you are getting out there. One mentions blogmeta. There is network options, which is a little different. That difference is important here.

I understand not want to put everything in once cache key. But network options are a little more controlled than options and alloptions. Autoloading options on sites is a performance issue. But that is when you have 500+ options on a site. See this ​link for more information. As network options are not used as much as they only for network options, network options do not have the same problems as per site options. I think the many benefits here, overcall, this small pitfall.

@spacedmonkey Did you see my note clarifying the backward compatibility break?

On trunk get_site_option() always* returns a string but on this branch numeric values are always returned as an integer.

Notes:
\* Not actually always, see ​my comment above for the full details
\ always always

[mukesh27]

@spacedmonkey I left nit-pick comments on docblocks that needs to address.

tyvm @mukeshpanchal27 💪

(I could've batched these commits, but I wanted to review them manually for my own sake while using the GitHub.com UI to view the diffs.)

[spacedmonkey]

In 54080:

Networks and Sites: Use metadata api in *_network_options` functions.

Replace logic found in get_network_option, update_network_option and delete_network_option to use the metadata api. Using the metadata api has a number of benefits, such as consistency, default values and useful filters. This change also improves performance by priming the caches of all network options in a single database request.

Props spacedmonkey, swissspidy, sc0ttkclark, johnjamesjacoby, flixos90, jeremyfelt, pento, peterwilsoncc, mukesh27, desrosj.
Fixes #37181

​https://github.com/WordPress/wordpress-develop/commit/bfbbf4928fc7f075ef4489ed97adbfd9c948d015 Committed

[spacedmonkey]

Made start on the dev note ​here.

[milana_cap]

Dev note: ​https://make.wordpress.org/core/2022/10/10/multisite-improvements-in-wordpress-6-1/

[johnbillion]

Reopening based on #56845.

[davidbaumwald]

After some discussion, prepping a full revert for now. For the full discussion in Slack, see ​https://wordpress.slack.com/archives/C03LZ88NX6G/p1666104060129079

[davidbaumwald]

In 54637:

Networks and Sites: Revert the use of the metadata API for *_network_options functions.

[54080] refactored the logic in get_network_option(), update_network_option() and delete_network_option() to use the metadata API. However, this change resulted in issues with large multisite installs that utilize memcached having network options > 1MB in size.

This change reverts [54080] and all related follow-up changes.

Reverts [54080], [54081], and [54082]. Partially reverts [54267] and [54402].

Props pavelschoffer, rebasaurus, johnbillion, spacedmonkey, desrosj, rinatkhaziev.
Fixes #56845.
See #37181.

[pbiron]

Replying to milana_cap:

Dev note: ​https://make.wordpress.org/core/2022/10/10/multisite-improvements-in-wordpress-6-1/

I think the Dev note should be updated to mention the revert.

[desrosj]

The dev note has been updated to include a callout at the top.

Since the 6.1 aspects of this have been addressed, I'm going to leave this open punt to 6.2. There was agreement that this could still be beneficial and deserves further exploration, but would need other related problems to be solved at the same time to be feasible.

Just to bring some thoughts @johnjamesjacoby shared that summarize a great path forward over from Slack for easier reference:

Hello! I’m onboard with reverting for 6.1, and suggest:

• keeping it open
• moving to the 6.2 milestone
• landing it simultaneously with some future meta data cache key improvement to address the forever-old 1MB memcached issue

The major (future) benefit of getting network options using the meta data API is it inherits all improvements from it (in addition to removing the bespoke cache-key implementation of options)

Would love to see (and participate in) a collaborative effort from cache, options, meta, multisite, & performance contributors (of which there is some overlap) to identify, document, and make progress on a roadmap.

[mehulkaklotar]

As per discussion in monthly bug scrub for performance team, we need to rethink the solution for this.

[pbearne]

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