Page MenuHomePhabricator

Use independent stats for emergency disable
Closed, ResolvedPublic

Description

As noted in a code comment:

// @ToDo this is an amount between 1 and AbuseFilterProfileActionsCap, which means that the
// reliability of this number may strongly vary. We should instead use a fixed one.

Event Timeline

Possible implementation:

When a filter is modified (FilterStore)

  • Create a cache key with TTL=threshold time, and value = [ 'matches' => 0, 'total' => 0 ], overwriting the value and the TTL if it already exists

When processing filter hits (FilterRunner)

  • Increase the 'total' key
  • Additionally, if a filter matched the action, bump the 'matches' key (the TTL shouldn't be changed, obviously)

In EmergencyWatcher

  • Simply lookup the key and check its value

Note that changing the TTL in the config will not result in existing keys being updated (which shouldn't be a big deal after all).


Overall, I think this should be quite simple. The main question is how to distribute this code. In particular, whether each component (esp. FilterStore and FilterRunner) should create/update the keys on its own, or whether there should be a central service for doing this (used by EmergencyWatcher as well). I think even with the first approach, some sort of "centralized knowledge" is necessary (e.g. for building the key), so we might as well just go with the second option.

When I started working on this, I realized the following. Filter updates are much less common than filter runs (which happen in real time). So the cache will usually contain only a few entries if any. If we attempted to update the key for every filter after every action, it would be a no-op in 95-99%. Therefore, we should also have a cache entry which tells for which filters it makes sense to update the cache entry (probably one key for each filter group). @Daimona What do you think?

Change 663190 had a related patch set uploaded (by Matěj Suchánek; owner: Matěj Suchánek):
[mediawiki/extensions/AbuseFilter@master] Use independent stats for emergency disable

https://gerrit.wikimedia.org/r/663190

Change 663190 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Use independent stats for emergency disable

https://gerrit.wikimedia.org/r/663190

Daimona assigned this task to matej_suchanek.