Page MenuHomePhabricator

User::pingLimiter always fails to limit when $wgMainCacheType is CACHE_NONE
Open, LowPublic

Description

Per https://lists.wikimedia.org/pipermail/wikitech-l/2017-April/087887.html:

I have been trying to add certain rate limiting actions for my extension's
API and noticed that it doesn't work with $wgMainCacheType set to CACHE_NONE

This is because  User::pingLimiter()
uses ObjectCache::getLocalClusterInstance() to store the current rate limit
counts.

As I recall the ping limiter originally used the main object cache instance and would have had this problem since the earliest days, but it never comes up in production since we always use a cache on Wikimedia sites and workalike configs.

This seems a poor behavior though, and should be fixed.

@aaron do you forsee any problems with changing ObjectCache::getLocalClusterInstance() to ObjectCache::getInstance(CACHE_ANYTHING) for this case? The commends in ObjectCache explicitly mention rate limiting as a good use for getLocalClusterInstance() but if this returns CACHE_NONE on default config then it's not really suitable for rate limiting as it doesn't work. :)

Alternately, should we change ObjectCache::getLocalClusterInstance() to consistently return something? Would that change any other usages?

Event Timeline

Maybe something like:

$cache = ObjectCache::getLocalClusterInstance();
if ( $cache instanceof EmptyBagOStuff ) {
    $cache = ObjectCache::getInstance( CACHE_DB );
}

Changing getLocalClusterInstance() might be viable now as well, since most true cache-aside stuff has been moved to WAN cache.

There should perhaps be another config variable like $wgSessionCacheType etc. ($wgRateLimiterCacheType?), defaulting to CACHE_ANYTHING, that would be used in that function?

Worth mentioning: T147161, T160519 which resulted in Gerrit change #342959.

Really: should we support CACHE_NONE outside of development?

@brion @demon Also note that I have plans to deprecate CACHE_ANYTHING in favour of making CACHE_DB the default fallback during installation. See T115890 for details.

Krinkle moved this task from General to libs/objectcache on the MediaWiki-libs-BagOStuff board.

Update - I currently see two paths forward:

  1. Always have a cache by default, adopting CACHE_DB as standard fallback, and making sure that stuff not worth caching under those conditions use something other than the main/wan cache interface. This is T186673.
  2. Or, a possible interim/alternative solution might be to use MainStash as the fallback, e.g. instanceof EmptyBag ? $services->getMainObjectStash(), where MainStash is currently the only "has to be on" storage not affected by main=CACHE_NONE.