It would be nice to have a debug option in MediaWikiServices where for each service it stores the stack trace of when it was initialized the first time, and prints that on conflict.

In T341102#9021451, @Tgr wrote:

It would be nice to have a debug option in MediaWikiServices where for each service it stores the stack trace of when it was initialized the first time, and prints that on conflict.

Good idea, we are doing something similar to detect cyclic dependencies.

The offending code is:

		// T212881: Redefine the RevisionStore service to explicitly use the new DB name.
		// Otherwise, ExternalStoreDB would be instantiated with an implicit database domain,
		// causing it to use the DB name of the wiki the script is running on due to T200471.
		$services->redefineService(
			'RevisionStore',
			static function ( MediaWikiServices $services ) use ( $dbName ) : RevisionStore {
				return $services->getRevisionStoreFactory()->getRevisionStore( $dbName );
			}
		);

Calling redefineService() is generally unsafe. It would be much better to call MediaWikiServices::resetGlobalInstance() and provide it with a configuration that matches the new wiki, in particular with respect to the database name.

The installer and MediaWikiIntegrationTestCase perform this kind of reset. We could get inspiration there.

Working on this is made difficult by the fact that this script is very WMF specific and it'S impossible to test it locally.

Pinging @Ladsgroup.

Change 939350 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/extensions/WikimediaMaintenance@master] DRAFT: Reset services after creating the database

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

I made a patch that shows how I think this should be approached.
I have no way to safely test this. @Ladsgroup do you have thoughts on how to figure out if this works, without breaking stuff?

In T341102#9024546, @daniel wrote:

I made a patch that shows how I think this should be approached.
I have no way to safely test this. @Ladsgroup do you have thoughts on how to figure out if this works, without breaking stuff?

The general way is to reserve a window, pull this patch onto an mwmaint server, and try to do the next one on the list (so probably T335216?). Which is… not ideal.

I wonder if it's possible to just split out "create the main MediaWiki DB" into a separate script, so the main script can be called with the actual wiki as its --wiki parameter.

In T341102#9026363, @Jdforrester-WMF wrote:

The general way is to reserve a window, pull this patch onto an mwmaint server, and try to do the next one on the list (so probably T335216?). Which is… not ideal.

Especially not ideal since this just might screw up the database of an existing wiki (the one the script "runs on")... Probably not in a catastrophic way, but still.

In T341102#9026593, @Tgr wrote:

I wonder if it's possible to just split out "create the main MediaWiki DB" into a separate script, so the main script can be called with the actual wiki as its --wiki parameter.

After all the work we did on overhaulting the config system in MW, it would probably be possible to run the entire script with the actual wiki in the --wiki parameter... Though I'm not so sure, given the way we generate config on the fly in production. We are not really benefitting from the SettingsBuilder system yet.

Generally, this script should work much more like the installer does.

In T341102#9026593, @Tgr wrote:

I wonder if it's possible to just split out "create the main MediaWiki DB" into a separate script, so the main script can be called with the actual wiki as its --wiki parameter.

I have been asking and basically begging for years to split the addWiki.php script or at least run it multiple times with stages to make more resilient towards issues like this.

In T341102#9026713, @daniel wrote:

Generally, this script should work much more like the installer does.

Theoretically yes but installer works for a small localhost MW installation. This script creates databases, sets the external storage, sets swift container, sets elastic search, ... This is extremely fragile.

Managed to reproduce this in beta:

ladsgroup@deployment-deploy03:~$ mwscript eval.php --wiki=aawiki
> $services = \MediaWiki\MediaWikiServices::getInstance();

> $languageNames = $services->getLanguageNameUtils()->getLanguageNames();

> $lbFactory = $services->getDBLoadBalancerFactory();

> $localLb = $lbFactory->getMainLB();

> $services->redefineService( 'RevisionStore', static function ( $services ) {return $services->getRevisionStoreFactory()->getRevisionStore( 'gpewiki' );}  );

Caught exception Wikimedia\Services\CannotReplaceActiveServiceException: Cannot replace an active service: RevisionStore
#0 /srv/mediawiki-staging/php-master/maintenance/eval.php(126) : eval()'d code(1): Wikimedia\Services\ServiceContainer->redefineService('RevisionStore', Object(Closure))
#1 /srv/mediawiki-staging/php-master/maintenance/eval.php(126): eval()
#2 /srv/mediawiki-staging/php-master/maintenance/includes/MaintenanceRunner.php(681): MWEval->execute()
#3 /srv/mediawiki-staging/php-master/maintenance/run.php(51): MediaWiki\Maintenance\MaintenanceRunner->run()
#4 /srv/mediawiki-staging/multiversion/MWScript.php(159): require_once('/srv/mediawiki-...')
#5 {main}

Let me see what I can do now. Locally it works, which means probably a service in an extension is being naughty.

(I always looked for an excuse to use this macro)

Change 939749 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[mediawiki/extensions/Wikibase@master] client: Avoid dynmaically registering hook handlers

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

Loaded the patch and in deploy03 and that seems to make it work

ladsgroup@deployment-deploy03:/srv/mediawiki/php-master$ mwscript eval.php --wiki=aawiki
> $services = \MediaWiki\MediaWikiServices::getInstance();
> $languageNames = $services->getLanguageNameUtils()->getLanguageNames();
> $lbFactory = $services->getDBLoadBalancerFactory();
> $localLb = $lbFactory->getMainLB();
> $services->redefineService( 'RevisionStore', static function ( $services ) {return $services->getRevisionStoreFactory()->getRevisionStore( 'gpewiki' );}  );

Change 939749 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@master] client: Avoid dynamically registering hook handlers

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

Change 939764 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] HookContainer: avoid instantiation of handlers when calling register()

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

Change 939764 merged by jenkins-bot:

[mediawiki/core@master] HookContainer: avoid instantiation of handlers when calling register()

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

Change 940918 had a related patch set uploaded (by Zabe; author: Daniel Kinzler):

[mediawiki/core@wmf/1.41.0-wmf.18] HookContainer: avoid instantiation of handlers when calling register()

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

Change 940919 had a related patch set uploaded (by Zabe; author: Amir Sarabadani):

[mediawiki/extensions/Wikibase@wmf/1.41.0-wmf.18] client: Avoid dynamically registering hook handlers

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

Change 940919 merged by jenkins-bot:

[mediawiki/extensions/Wikibase@wmf/1.41.0-wmf.18] client: Avoid dynamically registering hook handlers

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

Change 940918 merged by jenkins-bot:

[mediawiki/core@wmf/1.41.0-wmf.18] HookContainer: avoid instantiation of handlers when calling register()

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

Mentioned in SAL (#wikimedia-operations) [2023-07-24T22:36:21Z] <zabe@deploy1002> Started scap: Backport for [[gerrit:940919|client: Avoid dynamically registering hook handlers (T341102)]], [[gerrit:940918|HookContainer: avoid instantiation of handlers when calling register() (T341102 T340113 T339834)]]

Mentioned in SAL (#wikimedia-operations) [2023-07-24T22:37:46Z] <zabe@deploy1002> zabe: Backport for [[gerrit:940919|client: Avoid dynamically registering hook handlers (T341102)]], [[gerrit:940918|HookContainer: avoid instantiation of handlers when calling register() (T341102 T340113 T339834)]] synced to the testservers mwdebug2002.codfw.wmnet, mwdebug2001.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug1002.eqiad.wmnet, and mw-debug kubernetes deployment (accessible via k8s-experime

Mentioned in SAL (#wikimedia-operations) [2023-07-24T22:46:20Z] <zabe@deploy1002> Finished scap: Backport for [[gerrit:940919|client: Avoid dynamically registering hook handlers (T341102)]], [[gerrit:940918|HookContainer: avoid instantiation of handlers when calling register() (T341102 T340113 T339834)]] (duration: 09m 59s)