Page MenuHomePhabricator

Add 'actor' to default $wgSharedTables
Open, HighPublic

Description

The default for $wgSharedTables is set up for the shared user table setup (https://www.mediawiki.org/wiki/Manual:Shared_database#The_simplest_setup:_A_shared_user_table), however these days that also requires sharing the actor table, as @kostajh and I discovered today while debugging in #mediawiki.

This should be fine for all new installs with no problem, do we need to do anything else for wikis that were sharing user tables and now potentially have actor tables that are out of sync? (Is that even possible?)

Event Timeline

Legoktm created this task.Jan 21 2020, 10:59 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 21 2020, 10:59 AM

and now potentially have actor tables that are out of sync? (Is that even possible?)

Yes, it's actually very likely.

Chances are that the upgrade process will have assigned the same actor IDs for all users that were registered at the time of the upgrade to an actor-using version of MediaWiki. But the actor IDs assigned for revisions/uploads/log entries by unregistered users are unlikely to match.

And since there were probably varying numbers of these unregistered users, the actor IDs for users registered since probably also don't match. And even if that "varying numbers of unregistered users" isn't the case, they still might not match because the actor ID would be assigned on creation on the wiki where the creation was done, but on first edit/upload/logged action for each other sharing wiki.

Uhoh :(

So I guess we're going to need some kind of maintenance script to reconcile all of these somehow...?

Yeah, probably.

Doing this would probably have to involve a list of tables with actor fields, including those from extensions. We'll probably want such a list for cleanup like T2323#5425236 and T225469 too.

At this point I think we should do this now so we don't continue to keep creating misconfigured wikis with a warning that people upgrading will need to remove it until we have the maintenance script.

eprodromou raised the priority of this task from Low to High.Sep 15 2020, 8:29 PM
eprodromou added a subscriber: eprodromou.

We think this is a high priority issue. The key thing is to identify if this remains a blocker. If not, it should drop back to low.

Change 630646 had a related patch set uploaded (by Kevin Bazira; owner: Kevin Bazira):
[mediawiki/core@master] Add 'actor' to default

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

With @daniel's guidance, I pushed a patch for this task here. Below are the steps I followed to test this patch that was added to MW 1.35;

  1. Created first_wiki by installing MW v1.31.10.
  1. Added some content to first_wiki. Created some users then viewed the contents of the 'actor' table - it was empty.
  1. Created second_wiki by installing MW v1.31.10 and adding the following configurations to LocalSettings.php.
$wgSharedDB = 'first_wiki_db';  # The $wgDBname for the first_wiki database holding the main user table
  1. Logged into second_wiki using credentials of a user I created in step2 for first_wiki. This confirmed that the first_wiki DB 'user' table was being shared successfully.
  1. Updated first_wiki from 1.31 to 1.35 and I was still able to perform step4 successfully.
  1. Viewed the contents of the first_wiki_db 'actor' table and this time it was not empty - it had all the usernames I had registered in step2.
  1. Updated second_wiki from 1.31 to 1.35 and I was still able to perform step4 successfully.
  1. I repeated the steps above and upgraded MW 1.33 to 1.35 and 1.34 to 1.35 successfully. The exception was that on step2, the 'actor' table wasn't empty for both 1.33 and 1.34.

So with this test, I can confirm that updating from 1.31^ to 1.35 works as expected, when multiple wikis are sharing the 'actor' table.

Change 631467 had a related patch set uploaded (by Daniel Kinzler; owner: Kevin Bazira):
[mediawiki/core@REL1_35] Add 'actor' to default $wgSharedTables

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

Change 630646 merged by jenkins-bot:
[mediawiki/core@master] Add 'actor' to default $wgSharedTables

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

daniel added a comment.EditedOct 5 2020, 7:56 PM

I just realized that the patch as merged has a potential to break wikis that are updated from a version that was already using an actor table (e.g. 1.34 or 1.35). When I told @kevinbazira which cases to test, I failed to identify the most critical one:

The problem is: No actor related DB updates will run if an actor table was already in use, but the wiki will start reading from the shared actor table - which has entirely incompatible data. @Legoktm even mentioned this case in the description ("wikis that were sharing user tables and now potentially have actor tables that are out of sync?) and @Anomie discussed it in his comment. I failed to properly consider the case.

I see the following options:

  1. Just revert this patch and tell people they should set $wgSharedTables manually when setting $wgSharedDB
  2. Implement a DB migration step that consolidates the different actor tables, and replaces actor IDs in all tables that reference them. That's the ideal solution, but quite a bit of work, and may be prone to obscure bugs that cause data corruption.
  3. All the updater to change the default: if it detects that an actor table has already been used, it would set $wgSharedTables to a value without the actor table. This is simple and safe, but asaif, we do not have precedent for the installer writing config. One way to do this would be a DefaultSettingsOverrides.php file that the updater can append to. But introducing this mechanism would probably need broader consensus, possibly an RFC.

Tagging this for the 1.36 release, because this now needs to be resolved before the release, one way or the other. If we have no proper solution by then, we'd at least have to do the revert.

daniel added a comment.Oct 7 2020, 8:22 AM

In addition to the three options outlined above, here is another one:

  1. Don't add 'actor' to $wgSharedTables in DefaultSettings.php, but add it in the LocalSettings.php generated by the installer, always. This would make sharing the actor table the default for new wikis, but not for existing wikis. This isn't great, since the default in DefaultSettings.php is no longer the actual default. But if we document it properly in that file and on MediaWiki.org, it should be OK.

I personally think that giving the updater a way to write config (option 3) might come in handy for other use cases as well. But if we can't get agreement on this quickly, option 4 would be preferable.

Change 633955 had a related patch set uploaded (by Kevin Bazira; owner: Kevin Bazira):
[mediawiki/core@master] Move 'actor' SharedTable to LocalSettings

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

Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:01 PM

Change 633955 merged by jenkins-bot:
[mediawiki/core@master] Move 'actor' SharedTable to LocalSettings

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

@kevinbazira @daniel Is this done or does the updated change need to be backported to 1.35?

daniel added a comment.Nov 9 2020, 5:33 PM

@kevinbazira @daniel Is this done or does the updated change need to be backported to 1.35?

This should indeed be backporting!

Change 640191 had a related patch set uploaded (by Clarakosi; owner: Kevin Bazira):
[mediawiki/core@REL1_35] Move 'actor' SharedTable to LocalSettings

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

Change 631467 abandoned by Clarakosi:
[mediawiki/core@REL1_35] Add 'actor' to default $wgSharedTables

Reason:
Abandoned in favor of the updated patch: I06d1e56d7a9314d42170a0e2e060712965371fc9

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

So, I just noticed this issue while debugging why I cannot log in to a newly-added wiki to WMCZ's wikifarm. The reason is that the actor table at the new wiki is empty, which makes User::newFromName() return an invalid user object.

This means no wikifarm with shared database setup can easily add a new wiki (ie. without fiddling with their actor tables).

Even it's not in default sharing config anymore, it should remain high IMO, because it causes an issue without a workaround that can be done by a sysadmin without knowledge about MW internals (which most of the wiki sysadmins probably are).

Change 640191 merged by jenkins-bot:
[mediawiki/core@REL1_35] Add 'actor' to $wgSharedTables in LocalSettings for new installs

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