Page MenuHomePhabricator

ExternalLinksChange Logging instrumentation is completely broken
Closed, InvalidPublic

Description

The way the logging was built into SpamBlacklist is overly complicated and broken. To maintain it, someone is required to understand the interaction of several hooks with a state variable that may or may not have race conditions. As I'm completely unfamiliar with mediawiki development, I'm not sure if there's a simple solution. But the current solution is un-maintainable by anyone other than the original author. If they refuse to maintain it, I think the best decision would be to scrap it and start over with a much simpler approach.

Question to anyone who's listening: it seems to me the complexity is mostly due to premature optimization, the desire to re-use "links added" and "links removed" arrays that are computed as part of SpamBlacklist. These could be costly to re-compute in a separate extension, so I understand the idea. But how do hooks work? Can't I just subscribe to a hook, do that processing, and fire the logging event without slowing down the actual save? Does the hook have to complete processing before the edit can be saved? Is that what the job queue is for? Is there good documentation on this somewhere?

Details

Related Gerrit Patches:
mediawiki/extensions/SpamBlacklist : masterAbandon EventLogging instrumentation
mediawiki/extensions/SpamBlacklist : master[WIP] Simplify and fix EventLogging instrumentation

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 6 2017, 2:46 PM
Milimetric renamed this task from Logging instrumentation is completely broken to ExternalLinksChange Logging instrumentation is completely broken.Apr 6 2017, 2:46 PM
Milimetric moved this task from Next Up to In Progress on the Analytics-Kanban board.

Change 346915 had a related patch set uploaded (by Milimetric):
[mediawiki/extensions/SpamBlacklist@master] Simplify and fix EventLogging instrumentation

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

Milimetric moved this task from Paused to In Progress on the Analytics-Kanban board.
Milimetric moved this task from Paused to In Code Review on the Analytics-Kanban board.
Nuria moved this task from In Code Review to Paused on the Analytics-Kanban board.May 5 2017, 3:14 PM
Nuria edited projects, added Analytics; removed Analytics-Kanban.Sep 14 2017, 4:11 PM
fdans moved this task from Incoming to Deprioritized on the Analytics board.Sep 21 2017, 4:39 PM
Samwalton9 closed this task as Invalid.Jan 25 2019, 11:54 AM

We're starting again on this work, so this task is no longer needed.

Change 492039 had a related patch set uploaded (by Milimetric; owner: Milimetric):
[mediawiki/extensions/SpamBlacklist@master] Abandon EventLogging instrumentation

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

This latest change I pushed removes any EventLogging instrumentation from the SpamBlacklist extension. Needs some review.

Change 346915 abandoned by Milimetric:
[WIP] Simplify and fix EventLogging instrumentation

Reason:
https://phabricator.wikimedia.org/T214706 implements this in a more elegant way, I'm going to remove EventLogging instrumentation from SpamBlacklist in a different change.

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

Change 492039 had a related patch set uploaded (by Milimetric; owner: Milimetric):
[mediawiki/extensions/SpamBlacklist@master] Abandon EventLogging instrumentation

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

Change 492039 merged by jenkins-bot:
[mediawiki/extensions/SpamBlacklist@master] Abandon EventLogging instrumentation

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