wp-graphql/wp-graphql-smart-cache

Cache Maps should not be stored if "Use Object Cache" setting is not enabled

jasonbahl opened this issue · 0 comments

WPGraphQL Smart Cache currently collects and stores cache maps, intended for local cache (object/transient) invalidation. These maps are being stored always, not just when "Use Object Cache" is enabled.

Folks using Network Cache (WP Engine's EverCache, Varnish, Litespeed, etc) have no use for these cache maps, but they're being stored regardless.

This is problematic and appears to be causing "out of memory" issues.

Here's a summary of the problem:

This means that anytime a unique GraphQL Query (+variables) is executed, this map grows.

Since the $request_key is based on the queryId, query document, variables and current user, this leads to a wide array of permutations of query keys.

i.e. { "query": 'query getPost( $id: ID! ) { post( id: $id ) { title } }', "variables": { id: 123 } }, and { "query": 'query getPost( $id: ID! ) { post( id: $id ) { title } }', "variables": { id: 456 } } generate different keys and are added to the map.

For higher-traffic / higher-content sites, this can lead to a build-up of the map quite quickly and ultimately leads to memory exhaustion.


Proposal:

Disable storing the maps if "Use Object Cache" is not enabled

Since the map that is stored is only used for purging cached responses from object cache/transient cache (and not network cache) we should disable the map storage for users that do not have "Use Object Cache" enabled (similar to: https://github.com/wp-graphql/wp-graphql-smart-cache/blob/main/src/Cache/Results.php#L114-L116). (Note, the mapping is also used for our tests, at least as of now, so tests will need to enable this feature)

By default, if "Use Object Cache" is disabled, storing the maps should also be disabled. However there should be a filter to allow the maps to be stored, even if "Use Object Cache" is disabled. This will allow our tests to work and will allow other plugins to enable the maps. For example, if someone was using GET requests on a host that cannot tag the caches with the X-GraphQL-Keys headers, they could take advantage of the maps and purge that way (like we did in the early days of this plugin)

Reduce the expiration of the local cache (address this at a later time)

Currently the expiration is default at DAY_IN_SECONDS. This means all the queries for a site will be added to the map for 24 hours. Even if we solve the first problem and disable mapping for sites that have not opted in to "Use Object Cache", this is still a problem for sites that have opted-in (they're using POST requests, for example).

For sites like clutchpoints that publish content constantly (200+ new articles each day), there's a high likelihood that the maps will grow significantly throughout a 24 hour period.

If we reduce the expiration to something like 1 Hour, this will likely lead to smaller maps being produced and persisted.

We should make the default expiration time configurable via a Setting (similar to Max Age for network cache) but have a lower default.