Page MenuHomePhabricator

Wikimedia\Rdbms\LBFactory/LoadBalancer gives wrong database connection on postgres when passing the current wiki id and $wgDBmwschema has default value
Closed, DeclinedPublicBUG REPORT

Description

ILBFactory::getMainLB and ILoadBalancer::getConnectionRef both have an argument $domain which is a "Domain ID, or false for the current domain".
In existing code the wiki id often given as this argument, because wikiid and database name are often the same.

But when the current wikiid of the running wiki is passed to both functions a wrong database connection is returned and selecting expected tables from the local/running wiki does not work.

This affects extension with shared (central/global) tables under postgres. The postgres tests are failing with "relation does not exist", because test code often sets the shared database to the local one.

This only affects postgres when $wgDBmwschema is mediawiki (installer default). Setting a custom schema in postgres makes test pass.

There is a naming difference between a "wikiid" and a "database domain" and it seems also a semantical/conceptionally different.
The DatabaseDomain class described database/schema/prefix while the wikiid is handled in WikiMap class and described the database name (when the wiki was not renamed).
The schema and prefix part of the DatabaseDomain are null under mysql/sqlite and allows to use the wikiid and domain at the same location, but that does not work under postgres correctly.

There are functions to convert domain to wikiid (WikiMap:.getWikiIdFromDbDomain) or to create a domain (WikiMap::getCurrentWikiDbDomain)

In order to fix the postgres errors a check was added to convert the local wiki id to false to allow correct handling inside of Wikimedia\Rdbms to the following extensions:

I was asked from @Krinkle to create a task to get a look at it

Related is also some wikiid/domain check for the FileBackendGroup in the wiring code

		// If the local wiki ID and local domain ID do not match, probably due to a non-default
		// schema, issue a warning. A non-default schema indicates that it might be used to
		// disambiguate different wikis.
		$legacyDomainId = strlen( $ld->getTablePrefix() )
			? "{$ld->getDatabase()}-{$ld->getTablePrefix()}"
			: $ld->getDatabase();
		if ( $ld->getSchema() !== null && $legacyDomainId !== $fallbackWikiId ) {
			wfWarn(
				"Legacy default 'domainId' is '$legacyDomainId' but wiki ID is '$fallbackWikiId'."
			);
		}

More related changes from the past:

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 815350 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/OAuthRateLimiter@master] Change wikiID to false for local database connection

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

Change 815352 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/BounceHandler@master] Change wikiID to false for local database connection

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

Change 815800 had a related patch set uploaded (by Umherirrender; author: Umherirrender):

[mediawiki/extensions/Flow@master] Change wikiID to false for local database connection

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

Krinkle triaged this task as Medium priority.

I will be -1'ing the above patches for the following reason:

On principal grounds, I believe we must not accept more debt and workarounds to make PGSQL work in this way. Patches are welcome to either convert these extension classes to accept correct dbdomain values as parameters instead. Alterternatively, we also accept patches to repair the wikiid->dbdomain mapping that Aaron developed in Rdbms, which I suspect has stopped working.

Ideally, the whole Echo DBFactory stuff would be removed due to duplication and conflict with LBFactory.

Change 815800 abandoned by Umherirrender:

[mediawiki/extensions/Flow@master] Change wikiID to false for local database connection

Reason:

In the spirit of T315396

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

Change 815352 abandoned by Umherirrender:

[mediawiki/extensions/BounceHandler@master] Change wikiID to false for local database connection

Reason:

In the spirit of T315396

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

Change 815350 abandoned by Umherirrender:

[mediawiki/extensions/OAuthRateLimiter@master] Change wikiID to false for local database connection

Reason:

In the spirit of T315396

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

Passing of old "wiki IDs" is not supported in these methods. The extensions should use WikiMap methods like getDomainFromWikid().