search

WordPoints / top-users-in-period

Display the top points earners within a given period of time
GNU General Public License v2.0
0 stars 0 forks source link

Caching #6

Closed JDGrimes closed 7 years ago

JDGrimes commented 7 years ago

While working on #4, I've build the query so that a caching class will be separate, so that the caching method can be changed, even on a per-query basis.

By default, I was thinking of using WordPress's caching API, but I always figured we'd probably need to do something special for sites that aren't using any form of persistent caching. We could use the transients API, but the drawback there is that we have to truncate the query signatures. We only have 40 characters, because we have to use the site transients.

JDGrimes commented 7 years ago

Actually, after researching this, it seems that the docs are wrong, and the max length is 167 as of WordPress 4.4. I've created WP#40455 to get the docs fixed.

JDGrimes commented 7 years ago

This means that we could use the transients API without any issues. The question is though, whether we actually need to use the regular transients or the site transients. I guess we do need to use the site transients for any network-wide queries, but we could use regular transients for any non-network queries, on multisite.

Detecting that isn't as simple as one might think though. I guess really we'd have to check if blog_id__in, etc., was set. Although, just checking if blog_id was set would be enough even in that case, because the blog_id wouldn't be set when blog_id__in is set. We would have to make sure that blog_id__compare is = though, or else we'd be in trouble.

So perhaps it would be simpler just to use site transients for all queries. The potential downside there is that the transients wouldn't be managed per-site anymore, they'd all be in the site-wide bucket. Although, in WordPress's default object cache implementation (which is non-persistent, of course), everything basically goes in the same bucket it is just prefixed with the blog prefix if the caching group isn't global. Not sure what the drop ins actually do though.

JDGrimes commented 7 years ago

A look at the code of W3TC indicates that it does place the cache in separate buckets on multisite, at least for the file-based caching.

In general then, I think it makes sense not to place all of the queries for every site in a single cache that might have to be loaded just to get the query from one site. It would be better to cache per-site queries per-site, since that is usually the only place that they will be used.

JDGrimes commented 7 years ago

Should that logic be a part of each cache type's object, or should it be abstracted out to a higher level? I guess it makes sense to have it be per class for now. For some cache types there might not even be a concept of site vs network, etc.

JDGrimes commented 7 years ago

The question then is whether we should just use the transients API, which will automatically use the object cache when persistent caching is enabled, or whether we should manually switch between the two cache types. I guess at this time there doesn't appear to be any need for us to add our own layer of complexity, so we may as well just provide a cache that uses the transients for now.

JDGrimes commented 7 years ago

When the results are for a different site on the network, I guess we should go ahead and use the network transients. That will take care of results from a particular site being shown on other sites around the network. It will meant that when the query runs on the site that it is from, it will put the results in its own cache, leading to cache duplication between the network and per-site cache. But at the moment this really isn't a concern, since we're not going to be providing that use-case directly in the initial version.