Page MenuHomePhabricator

Installer fails in RevisionStore
Closed, ResolvedPublic

Description

While trying out the base installer on a current git checkout, I found that it fails while trying to add the main page:

Could not insert main page: RevisionStore for my_wiki cannot be used with a DB connection for devwiki

(devwiki is the database name I selected for the target wiki; my_wiki is the default.)

Also showed these errors afterwards:

Notice: Uncommitted DB writes (transaction from WikiPage::doCreate). in /usr/local/var/www/w/includes/libs/rdbms/database/Database.php on line 3798

Notice: DB transaction writes or callbacks still pending (WikiPage::insertOn). in /usr/local/var/www/w/includes/libs/rdbms/database/Database.php on line 3804

Fatal error: Uncaught Wikimedia\Rdbms\DBTransactionError: Explicit transaction still active. A caller may have caught an error. in /usr/local/var/www/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php:1245 Stack trace: #0 [internal function]: Wikimedia\Rdbms\LoadBalancer->Wikimedia\Rdbms\{closure}(Object(Wikimedia\Rdbms\DatabaseMysqli)) #1 /usr/local/var/www/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1591): call_user_func_array(Object(Closure), Array) #2 /usr/local/var/www/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1268): Wikimedia\Rdbms\LoadBalancer->forEachOpenMasterConnection(Object(Closure)) #3 [internal function]: Wikimedia\Rdbms\LoadBalancer->approveMasterChanges(Array) #4 /usr/local/var/www/w/includes/libs/rdbms/lbfactory/LBFactory.php(184): call_user_func_array(Array, Array) #5 [internal function]: Wikimedia\Rdbms\LBFactory->Wikimedia\Rdbms\{closure}(Object(Wikimedia\Rdbms\LoadBalancerSingle), 'approveMasterCh...', Array) #6 /usr/local/var/www/w/includes/libs/rdbms/lbfactory/LBFactorySingle.php( in /usr/local/var/www/w/includes/libs/rdbms/loadbalancer/LoadBalancer.php on line 1245

Details

Related Gerrit Patches:

Related Objects

View Standalone Graph
This task is connected to more than 200 other tasks. Only direct parents and subtasks are shown here. Use View Standalone Graph to show more of the graph.
StatusAssignedTask
Resolveddaniel
ResolvedTgr

Event Timeline

brion created this task.Feb 16 2018, 10:46 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 16 2018, 10:46 PM

I also got this error at my install time.

@brion what did you have checked out? master?

brion added a comment.Feb 28 2018, 3:43 PM

Yep, master. Note that it may not happen if you accept the default database name of "my_wiki".

Hmm, I just tried the CLI installer (what I usually use) and everything succeeded.

I guess this ticket is about the GUI installer?

brion added a comment.Mar 5 2018, 5:02 PM

Yep. I forgot there even was a CLI installer. :)

But this does not seem to affect the web installation. Try the second time, I am successfully install.

Addshore added a subscriber: Tgr.Mar 8 2018, 5:59 PM
Tgr added a comment.Mar 14 2018, 12:47 AM

If RevisionStore is not explicitly bound to a wiki, it will fall back to wfWikiID() which does not work during installation (it checks the dbname global and there is no LocalSettings.php yet). As Brion said this does not happen if you leave the DB name on the default setting (since that comes from DefaultSettings.php).

Tgr added a comment.Mar 14 2018, 1:05 AM

RevisionStore comes from the service container which is blind to the installer and is initialized from globals. So I suppose options are:

  • Fix MediaWikiServices::getInstance() so that it does not try to configure itself; make setting up the services the responsibility of the entry point and fail if that does not happen. Have the installer set up services using the session overrides it holds for DB name etc. Nicest approach IMO.
  • Make the installer set the globals (probably at the beginning of WebInstaller::execute where it reads the settings from the session). Probably the easiest.
  • Optionally inject RevisionStore into WikiPage and then Revision instead of having the latter take it from the service container directly. Have the installer construct a custom RevisionStore using the wiki id it has.
daniel added a subscriber: daniel.Mar 15 2018, 6:06 PM

@Tgr, you are probably right that the easiest fix is to let WebInstaller::execute set the relevant globals. So we should go for that first.

But I also agree that it would be better to have the installer explicitly (re)configure the service container. It actually already does this by calling MediaWikiServices::resetGlobalInstance() in two places: The constructor of the installer class (oh my) and in DatabaseInstaller::enableLB(). In the former call, it explicitly passes in the "installer config", while in the second case, it does not. Perhaps it should, I have not investigated this closely... But it seems to me like the proper fix would involve calling resetGlobalInstance() with the correct settings in the right places.

daniel added a subscriber: aaron.Mar 15 2018, 6:19 PM

@Tgr I have two experimental patches open that touch the relevant code. They may be useful as context, and maybe even offer a solution:

I talked quite a bit about this with @aaron, maybe he has an idea.

daniel triaged this task as High priority.Mar 15 2018, 6:19 PM
Tgr added a comment.Mar 15 2018, 6:36 PM

Those patches don't help here. The issue is that the Database object that gets passed to RevisionStore::insertRevisionOn is directly generated by DatabaseInstaller and uses the DB name given by the user; everything else (including wfWikiID, the load balancer or anything you could get from the service container) uses globals, which at this point are just DefaultSettings.php values.

@Tgr yes, the idea is that we should avoid falling back to wfWikiID. I may be totally wrong, but I poked at this for a while when I was in SF, and I seem to recall that this, unlikely as it may seem, should fix the issue: https://gerrit.wikimedia.org/r/c/406793/2..3/includes/libs/rdbms/lbfactory/LBFactorySingle.php

Tgr added a comment.Mar 16 2018, 2:02 AM

That would work if the load balancer would be initialized from the DB object created by the installer, but as far as I can tell it's not. It will be created by the service container via LBFactory::getMainLB, and the LBFactory object itself will be created from globals.

The initialization process of LBFactory is rather confusing... I *think* it should be constructed based on the Config object that comes from Installer::getInstallerConfig(). Perhaps WebInstaller could override that and take the DB config from the session?

Anyway, I think @aaron is the person who best understands how all this works.

Change 420627 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/core@master] Set WebInstaller session variables as globals

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

Change 420627 merged by jenkins-bot:
[mediawiki/core@master] Set WebInstaller session variables as globals

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

Tgr closed this task as Resolved.Mar 20 2018, 6:31 AM
Tgr claimed this task.

The problem with service initialization is filed as T190124: MediaWikiServices should not rely on globals. For now, the problem is fixed by the installer setting globals. The CLI installer did that so at least it's a bit more consistent now, and as long as wfWikiID uses globals it's needed anyway.