Page MenuHomePhabricator

Re-enable profiling for stashed edits
Closed, ResolvedPublic


As I pointed out in T191032, profiling was performed both for 'stash' and 'execute' modes, which meant that we got corrupted results: the percentage of matches was totally wrong and actions were recorded at least twice. However, by limiting profile to 'execute', we introduced another problem: a stashed edit isn't profiled. For "stashed edit" I mean that you edit the page and you save the same version which was stashed; check T176291#3667054 and T176291#4100964 for a bit of further explanation. From my POV it's much, much better to have stashed actions not profiled than to get corrupted statistics, but an optimal situation would have exactly one profiling record for each action, wherever it comes from.

Event Timeline

Daimona triaged this task as High priority.Apr 4 2018, 4:25 PM

Well, stats are still corrupted, although in a different way. This is the scheme of the situation, supposing to edit with a catch-all filter (where every value should be +1).
Legend: T = total actions, H = hits (the numbers in "of the last T actions, H matched...")

old versioncurrent version
stashed editT:+1, H: +1T:+0, H: +1
fresh editT:+2, H: +1T:+1, H: +1

There's no need to revert and go back to the previous version, but this needs to be fixed anyway.

Change 423978 had a related patch set uploaded (by Daimona Eaytoy; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] Re-execute checkAllFilters if the edit was stashed

Change 423978 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Re-execute checkAllFilters if the edit was stashed

Daimona removed a project: Patch-For-Review.

Profiling is now back working and will add tests to ensure it won't break again.