wp_suspend_cache_addition should also disable cache setting?

[malthert]

Right now wp_suspend_cache_addition is only checked when "add" to cache.

However most plugins/developers only use wp_cache_set for updating/adding to the cache.
This means in most plugins/cases, wp_suspend_cache_addition has no use, in fact it's rather unexpected behavior that only some functions can add to cache (namely: set, incr, decr)

I propose we also check wp_suspend_cache_addition before setting the cache.
This would also be in line with its function description:

Stops more data being added to the cache, but still allows cache retrieval.

As "set" also "adds" (or at least modifies if already exists therefore adds/subtracts) data in cache.

Fixes: https://core.trac.wordpress.org/ticket/55999

[johnbillion]

This is a good idea but it is a breaking change. During a large import for example, suspending cache addition makes sense to avoid consuming a large amount of memory (particularly when inserting terms and post meta), but updating existing cache entries may well be desirable and expected.

I'll see if I can find some concrete examples.

I think to avoid backwards compatibility issues this needs to be implemented via a new function such as wp_suspend_cache_changes().

Either way, it would be good to get test coverage on this too.

[kkmuffme]

During a large import for example, suspending cache addition makes sense to avoid consuming a large amount of memory (particularly when inserting terms and post meta), but updating existing cache entries may well be desirable and expected.

• for incr/decr it will end up with wrong values if only "add" is suspended but you then call incr/decr on it. For these 2 it's breaking but the original behavior was unintended and there isn't a use case I can think of where this is expected.

• for set: it's breaking what is currently broken = making it work as expected :-) when using a persistent cache backend. The current behavior is a massive source of race conditions causing invalid cache states.

Additionally, from the function description of the suspend function it reads as it will suspend all "adding" to cache - that it previously only stopped "add" is mostly unexpected I'd say.

I'm also not really able to come up with any example where you want to disable "add" but still be able to "set"?

---

Added unit tests now

[swissspidy]

Adding the early keyword as this kind of change should be potentially done earlier in a cycle.