Fix the bugs in Special:GlobalUserRename
Description
Status | Subtype | Assigned | Task | |
---|---|---|---|---|
· · · | ||||
Declined | Legoktm | T49918 Rename of global (attached) users to existing global usernames | ||
Resolved | Legoktm | T961 Code review CentralAuth's GlobalUserMerge and related UserMerge code before SUL finalization mass usage | ||
· · · |
Event Timeline
This code needs review by a senior architect, and it must be tested and deployed in the first week or two of May, at the latest.
Given that we're at the end of March, I'd like to ensure that the resources needed to check on this are available. This needs to be checked before SUL finalization begins (still slated for April 15th) - we're a little under 3 weeks away. @RobLa or @Brion, pinging to draw your attention. Possible to give a timeframe?
The code, as written, works in theory but not in practice and breaks in all kinds of interesting ways with only slight pushes.
Not that we don't know that it's needed, but here's just the onwiki backlog waiting on this tool: https://meta.wikimedia.org/wiki/Steward_requests/Username_changes#Requests_with_usurps_and_complex_renames
After finalization it will likely be a hundred fold for the first couple of weeks.
Is there any more information about the circumstances under which it fails, and the failure mode?
Absolutely. Here's a couple of scenarios that occurred when I was testing it in November:
- Account 1 is registered and makes an edit
- Account 2 is registered and makes no edit
- Account 1 is merged into Account 2
- The edit vanishes from attribution but sits in the database
- Account 1 is registered, no edits
- Account 2 is registered, no edits
- Account 3 is registered, an edit
- All accounts merged into Account 1
- The edit shows up in Account 1's contributions, but Account 3 was deleted from the database, corrupting the move log as though the account never existed
...and other variations of the sort. GUM is having trouble staying in sync between attribution, logging, and account storage*.
*edit: in that, it will delete the local account that is being merged instead of just renaming it and attaching it to the global account
The code is not too bad. The LocalUserMergeJob potentially has a lot of work to do, which it does in a lot of separate transactions, so the work could easily be incomplete if an exception is encountered, e.g. a deadlock. LocalUserMergeJob::allowRetries() returns true (the default is not changed), which may possibly mitigate the issue depending on where the exception occurs.
I think a good way to proceed would be to test it on beta and to file detailed bugs if any issues occur. By detailed, I mean including the relevant entries from the runJobs log and the exception log.
I don't know if there will be job queue exceptions in practice. Maybe we can get away with running it as it is, i.e. without rearchitecting for robustness. Pilot deployment may help to nail down how much work we need to do here.
I can't reproduce this. I checked the revision table on beta's metawiki for referential integrity, but the only issues I could find were caused by T96117. I tried doing it myself, but everything seems fine afterwards: http://meta.wikimedia.beta.wmflabs.org/w/index.php?title=User:Timtest2&action=history
I don't recall the circumstances at this point, I just recall talking to @Legoktm about it as it happened. It was in his very first release of the code to betalabs, so it could have very well have been a one-off bug or fixed.