Page MenuHomePhabricator

Read from and write to `actor` table in AbuseFilter
Open, LowPublic

Description

rMW27c61fb1e94d: Add `actor` table and code to start using it changed the database schema throughout MediaWiki. Tables to migrate:

  • abuse_filter: af_user & af_user_text -> af_actor
  • abuse_filter_log: afl_user & afl_user_text -> afl_actor
  • abuse_filter_history: afh_user & afh_user_text -> afh_actor

Additionally, there are several reads from recentchanges and revision.

To do:

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Will it be fixed for renamings before patch? Still not fixed for me, see T193504

Change 445185 had a related patch set uploaded (by Matěj Suchánek; owner: Matěj Suchánek):
[mediawiki/extensions/AbuseFilter@master] Use actor table in AbuseFilter

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

Reconsidering after the architecture review, in case it will make it easier to migrate abuse_filter_log. Low priority otherwise.

Change 681867 had a related patch set uploaded (by Daniel Kinzler; author: Tim Starling):

[mediawiki/core@master] Split a base class out of ActorMigration

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

abuse_filter_log saves non-existing users when AbuseFilter prevents account creation. This now make problems with the actor migration in checkuser, see T309737. Any ideas what we should do about that? It also basically makes actor migration for that table impossible in the current situation.

abuse_filter_log saves non-existing users when AbuseFilter prevents account creation. This now make problems with the actor migration in checkuser, see T309737. Any ideas what we should do about that? It also basically makes actor migration for that table impossible in the current situation.

I know... Aside from issues with the actor migration here and in other extensions, this has caused other issues in the past, like T280413 and related tickets. I honestly don't know what to do about it.

abuse_filter_log saves non-existing users when AbuseFilter prevents account creation. This now make problems with the actor migration in checkuser, see T309737. Any ideas what we should do about that? It also basically makes actor migration for that table impossible in the current situation.

Proposal: https://gerrit.wikimedia.org/r/814125 (T233004#8080557).

$wgAbuseFilterNotifications = 'rc' or 'rcandudp' is broken as well and throws CannotCreateActorException (it's post send, not visible to the wiki), the proposal does not fix that part (because CheckUser would be called from RecentChanges for that configs)

Even AbuseFilter extension is fixed, this needs to wait 90 day to be sure there is no such data in checkuser (or needs to check if such filter exists and how old the last log is for that within the 90 days)

I have also no idea. Creating an hidden/locked account with invalid password/token for this purpose allows to create the actor entry, but could have other issues for oversighters to see this, reporting if someone tries to create such the name a second time or other issues. Using an interwiki user also looks not nice.
Using the ip as actor seems not wanted and is unexpected when a user with AGF is filtered as false positive, if I understand the hint to some private bugs above correctly.

Indeed, AbuseFilter is broken with these two options for $wgAbuseFilterNotifications, and it was unnoticed because Wikimedia uses $wgAbuseFilterNotifications = 'udp' (which should still work).
I've created T314272: AbuseFilter throws exception when account creation is logged to recent changes for it because it is not related to this task.

the proposal does not fix that part

But it could help unblock CheckUser at least.

Repeating my Gerrit comment: it should be OK for the commit I just approved to go out with the train, but manual testing should be done prior to each subsequent production deployment step, specifically:

  • Test MigrateActorsAF.php
  • Test READ_NEW
  • Test WRITE_NEW

This should be done on a test wiki and a comment should be written somewhere indicating that it was done.

If these tests have already been done, that's fine, but someone should confirm that they were done.

Change 445185 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Use actor table in AbuseFilter

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

@matej_suchanek Let me know if you need help in any step of the way. You can also start testing first with beta cluster and then test wikis.

Thank you. I think most of the steps will be done by others anyway since I'm not a sysadmin. (Like the first step, to get the patch deployed to production this week.) By coincidence, I do have rights for filter management on wikis in each group, so testing is the least I can do. The next step is probably T333332.

I wanted to highlight one important thing to consider during migration. Small and medium-sized wikis and some others, including test wikis, use global abuse filters, i.e., they also read from abuse_filter on metawiki. There is only one config variable, so each wiki reads the local and global databases the same way.

For example, it could cause problems if testwiki was set to READ_NEW mode before WRITE_BOTH has been set on metawiki and the migration script has been run there, too. Or to drop the legacy fields on metawiki if some wikis are still in READ_OLD mode.

So as you mentioned "step," the steps need to be arranged a bit more carefully than usual.

Thank you for the note, it's actually not too unusual. For rev_comment migration we had the similar thing that a client wiki would read with its own config from wikidata's revision table (and *fireworks* ensues). I keep that in mind.

@matej_suchanek Is anything else here needed before we can start reading from af_actor and afh_actor?

As SCHEMA_COMPAT_WRITE_BOTH is active everywhere, I suppose no.

Change #1034804 had a related patch set uploaded (by Matěj Suchánek; author: Matěj Suchánek):

[mediawiki/extensions/AbuseFilter@master] Drop af_user(_text) and afh_user(_text) fields

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

Change #1039564 had a related patch set uploaded (by Matěj Suchánek; author: Matěj Suchánek):

[mediawiki/extensions/AbuseFilter@master] Drop $wgAbuseFilterActorTableSchemaMigrationStage

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

Change #1040726 had a related patch set uploaded (by Matěj Suchánek; author: Matěj Suchánek):

[mediawiki/extensions/AbuseFilter@master] Remove usages of ActorMigration

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

Change #1039564 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Drop $wgAbuseFilterActorTableSchemaMigrationStage

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

Change #1039564 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Drop $wgAbuseFilterActorTableSchemaMigrationStage

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

This seems to have broken beta - https://integration.wikimedia.org/ci/view/Beta/job/beta-update-databases-eqiad/76633/console

22:20:04 Running /srv/mediawiki-staging/php-master/extensions/AbuseFilter/includes/Hooks/Handlers/../../../maintenance/MigrateActorsAF.php...
22:20:04 Beginning migration of abuse_filter.af_user and abuse_filter.af_user_text to abuse_filter.af_actor
22:20:04 User name "Meno25" is usable, cannot create an anonymous actor for it. Run maintenance/cleanupUsersWithNoId.php to fix this situation.
22:20:04 
22:20:04 User name "Ciphers" is usable, cannot create an anonymous actor for it. Run maintenance/cleanupUsersWithNoId.php to fix this situation.
22:20:04 
22:20:04 User name "Antime" is usable, cannot create an anonymous actor for it. Run maintenance/cleanupUsersWithNoId.php to fix this situation.
22:20:04 
22:20:04 User name "AhmadSherif" is usable, cannot create an anonymous actor for it. Run maintenance/cleanupUsersWithNoId.php to fix this situation.
22:20:04 
22:20:04 User name "شرف الدين" is usable, cannot create an anonymous actor for it. Run maintenance/cleanupUsersWithNoId.php to fix this situation.
22:20:04 
22:20:04 ... af_id=48
22:20:04 Completed migration, updated 0 row(s) with 0 new actor(s), 41 error(s)
22:20:04 Beginning migration of abuse_filter_history.afh_user and abuse_filter_history.afh_user_text to abuse_filter_history.afh_actor
22:20:04 Completed migration, updated 0 row(s) with 0 new actor(s), 0 error(s)
22:20:04 RuntimeException from line 1113 of /srv/mediawiki-staging/php-master/includes/installer/DatabaseUpdater.php: Execution of /srv/mediawiki-staging/php-master/extensions/AbuseFilter/includes/Hooks/Handlers/../../../maintenance/MigrateActorsAF.php did not complete successfully.
22:20:04 #0 /srv/mediawiki-staging/php-master/includes/installer/DatabaseUpdater.php(606): MediaWiki\Installer\DatabaseUpdater->runMaintenance('MediaWiki\\Exten...', '/srv/mediawiki-...')
22:20:04 #1 /srv/mediawiki-staging/php-master/includes/installer/DatabaseUpdater.php(563): MediaWiki\Installer\DatabaseUpdater->runUpdates(Array, true)
22:20:04 #2 /srv/mediawiki-staging/php-master/maintenance/update.php(189): MediaWiki\Installer\DatabaseUpdater->doUpdates(Array)
22:20:04 #3 /srv/mediawiki-staging/php-master/maintenance/includes/MaintenanceRunner.php(696): UpdateMediaWiki->execute()
22:20:04 #4 /srv/mediawiki-staging/php-master/maintenance/run.php(51): MediaWiki\Maintenance\MaintenanceRunner->run()
22:20:04 #5 /srv/mediawiki-staging/multiversion/MWScript.php(158): require_once('/srv/mediawiki-...')
22:20:04 #6 {main}

And of course, for added fun... If we bring back a copy of the script, and run it against arwiki in this case...

-----------------------------------------------------------------
arwiki
-----------------------------------------------------------------
arwiki:  ...Update 'CleanupUsersWithNoId' already logged as completed. Use --force to run it again.

Change #1041255 had a related patch set uploaded (by Reedy; author: Reedy):

[operations/mediawiki-config@master] Remove old wgAbuseFilterActorTableSchemaMigrationStage config

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

@Ladsgroup would report similar errors on the first run: T336224#8838909.

As for why it happens again now, maybe it is due to moving the script from addPostDatabaseUpdateMaintenance to runMaintenance?

Change #1034804 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Drop af_user(_text) and afh_user(_text) fields

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

Change #1040726 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Remove AbuseFilterActorMigration

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

Change #1078415 had a related patch set uploaded (by Zabe; author: Zabe):

[operations/mediawiki-config@master] Stop setting wgAbuseFilterActorTableSchemaMigrationStage

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

Change #1078415 merged by jenkins-bot:

[operations/mediawiki-config@master] Stop setting wgAbuseFilterActorTableSchemaMigrationStage

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

Mentioned in SAL (#wikimedia-operations) [2024-10-08T13:46:45Z] <zabe@deploy2002> Started scap sync-world: Backport for [[gerrit:1078415|Stop setting wgAbuseFilterActorTableSchemaMigrationStage (T188180)]]

Mentioned in SAL (#wikimedia-operations) [2024-10-08T13:48:53Z] <zabe@deploy2002> zabe: Backport for [[gerrit:1078415|Stop setting wgAbuseFilterActorTableSchemaMigrationStage (T188180)]] synced to the testservers (https://wikitech.wikimedia.org/wiki/Mwdebug)

Mentioned in SAL (#wikimedia-operations) [2024-10-08T13:53:49Z] <zabe@deploy2002> Finished scap sync-world: Backport for [[gerrit:1078415|Stop setting wgAbuseFilterActorTableSchemaMigrationStage (T188180)]] (duration: 07m 03s)

Change #1041255 abandoned by Reedy:

[operations/mediawiki-config@master] Remove old wgAbuseFilterActorTableSchemaMigrationStage config

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