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?

Event Timeline

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

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