Page MenuHomePhabricator

[betalabs-regression] Any edit throws "AbuseFilter.php: The specified row must be a full abuse_filter row"
Closed, ResolvedPublic

Description

Attempting to save any edit in betalabs will trigger Internal error page:

Visual Editor:

Source editor:

1 in bet[XR5xh6wQBHcAAGDRUZgAAAAB] /w/index.php?title=Wikipedia:Mentors&action=submit UnexpectedValueException from line 1358 of /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilter.php: The specified row must be a full abuse_filter row.
2
3Backtrace:
4
5#0 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilter.php(675): AbuseFilter::cacheFilter(string, stdClass)
6#1 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilter.php(625): AbuseFilter::checkFilter(stdClass, AbuseFilterCachingParser, Title, string, string)
7#2 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilter.php(1195): AbuseFilter::checkAllFilters(AbuseFilterVariableHolder, Title, string, string, AbuseFilterCachingParser)
8#3 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilterHooks.php(138): AbuseFilter::filterAction(AbuseFilterVariableHolder, Title, string, User)
9#4 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilterHooks.php(60): AbuseFilterHooks::filterEdit(DerivativeContext, WikitextContent, string, Status, string, string)
10#5 /srv/mediawiki/php-master/includes/Hooks.php(174): AbuseFilterHooks::onEditFilterMergedContent(DerivativeContext, WikitextContent, Status, string, User, boolean)
11#6 /srv/mediawiki/php-master/includes/Hooks.php(202): Hooks::callHook(string, array, array, NULL)
12#7 /srv/mediawiki/php-master/includes/EditPage.php(1766): Hooks::run(string, array)
13#8 /srv/mediawiki/php-master/includes/EditPage.php(2221): EditPage->runPostMergeFilters(WikitextContent, Status, User)
14#9 /srv/mediawiki/php-master/includes/EditPage.php(1596): EditPage->internalAttemptSave(NULL, boolean)
15#10 /srv/mediawiki/php-master/includes/EditPage.php(681): EditPage->attemptSave(NULL)
16#11 /srv/mediawiki/php-master/includes/actions/EditAction.php(60): EditPage->edit()
17#12 /srv/mediawiki/php-master/includes/actions/SubmitAction.php(38): EditAction->show()
18#13 /srv/mediawiki/php-master/includes/MediaWiki.php(499): SubmitAction->show()
19#14 /srv/mediawiki/php-master/includes/MediaWiki.php(294): MediaWiki->performAction(Article, Title)
20#15 /srv/mediawiki/php-master/includes/MediaWiki.php(884): MediaWiki->performRequest()
21#16 /srv/mediawiki/php-master/includes/MediaWiki.php(515): MediaWiki->main()
22#17 /srv/mediawiki/php-master/index.php(42): MediaWiki->run()
23#18 /srv/mediawiki/w/index.php(3): include(string)
24#19 {main}

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 4 2019, 9:49 PM
Etonkovidova updated the task description. (Show Details)Jul 4 2019, 9:49 PM
Etonkovidova updated the task description. (Show Details)
Daimona added a subscriber: Daimona.Jul 4 2019, 9:59 PM

https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/486726/ Added validation, but I can't understand why it's not getting a complete row.
A couple of notes:

  • Only happens for global filters
  • Effectively prevents any action

Tomorrow morning (UTC+2) I'll investigate what's wrong here.

If a quick fix is needed, it should be enough to comment out the line throwing the exception.

Daimona added a comment.EditedJul 4 2019, 10:04 PM

Ah, actually I got it. The global rows are saved in cache with TTL indefinite (????), and previously (before the patch above) we didn't select all fields.
So we have cached stuff without all the rows, and that fails.
I'd say to wait for the cache to be deleted, but I don't know how much it could take. And indefinite TTL doesn't look right here.

@Krinkle what TTL would you suggest for caching the result of SELECT on a foreign DB? I was thinking of something between 1 hour and 1 day. The idea being to take into account global filters modification, but keep a relatively long TTL to avoid performance bottlenecks.

@Daimona It looks like it uses the touch feature of WANObjectCache to purge when there is a change. So in terms of TTL for filter edits to apply, that will be fine with an infinite TTL (although anything longer than a week is unlikely to survive, so perhaps limit it to that using $wan::TTL_WEEK).

The issue here is not about filter edits, but software deployments resulting in a backwards-incompatible change to the format of the cached value. For that, one should use a versioned cache key. usually like so:

$wan->makeKey( 'abusefilter', thingything, self::THINGY_CACHE_VERSION );

or

$wan->getWithSet…(
  $wan->makeKey( 'abusefilter', thingything ),
  callable,
  [ 'version' => self::THINGY_CACHE_VERSION ]
);

The latter must be used if you rely on being able to 'touch' or 'purge' the key so that it works even mid-deployment and across wikis, when different servers temporarily have different versions of the software.

Then, if you make a schema change, it would go like this:

  • … schema patch written, reviewed, and merged.
  • … schema patch applied to all wikis.
  • … code first uses the new fields, including a bump to the cache version.

Right, it's done upon saving. I'm going to send a patch now.

Change 520846 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Fix global caching

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

Daimona claimed this task.Jul 5 2019, 7:19 AM

Does this actually block the train? If so, it should be UBN! (and how can I help?).

Daimona triaged this task as Unbreak Now! priority.Jul 8 2019, 4:45 PM

Yes, it does; I think all actions will be prevented on every wiki with global filters enabled, until the cache is purged (which could take a long time). r520846 should be enough.

Restricted Application added a subscriber: Liuxinyu970226. · View Herald TranscriptJul 8 2019, 4:45 PM

Change 520846 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Fix global caching

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

Daimona closed this task as Resolved.Jul 9 2019, 7:09 AM
Restricted Application added a project: User-Ryasmeen. · View Herald TranscriptJul 9 2019, 7:09 AM
Ladsgroup reopened this task as Open.Jul 9 2019, 10:44 PM
Ladsgroup added a subscriber: Ladsgroup.

This needs to be backported

This needs to be backported

To which branch? b2af2f0 is in wmf.13 and didn't affect wmf.11.

Ladsgroup closed this task as Resolved.Jul 9 2019, 10:48 PM

Oh sorry for the noise then.