Installer fails in RevisionStore
Closed, ResolvedPublic
Actions

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

	Subject	Repo	Branch	Lines +/-
	Set WebInstaller session variables as globals	mediawiki/core	master	+4 -0

Customize query in gerrit

Related Objects
Search...

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.

Status	Assigned	Task
		· · ·
Resolved	daniel	T174024 Implement MCR revision retrieval interface
Resolved	Tgr	T187586 Installer fails in RevisionStore
		· · ·

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?

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?

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 project: MW-1.31-release.Mar 8 2018, 5:56 PM

Addshore added a subscriber: Tgr.Mar 8 2018, 5:59 PM

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).

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.

pmiazga subscribed.Mar 15 2018, 5:47 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.

@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

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

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

gerritbot added a project: Patch-For-Review.Mar 20 2018, 1:39 AM

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

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-03-20 (1.31.0-wmf.26)).Mar 20 2018, 5:00 AM

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.