RevisionStore::checkDatabaseWikiId is broken in some cases
Closed, ResolvedPublic

Description

After an update to the current master, something seems broken.

See https://tools.wmflabs.org/wmde-uca-test/core/

[6bc29040c8e2a5cfb96e36aa] /wmde-uca-test/core/ MWException from line 1480 of /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/Storage/RevisionStore.php: RevisionStore for s53402__wmde-uca-test cannot be used with a DB connection for s53402__wmde?huca?htest

Backtrace:

#0 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/Storage/RevisionStore.php(1498): MediaWiki\Storage\RevisionStore->checkDatabaseWikiId(Wikimedia\Rdbms\DBConnRef)
#1 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/Storage/RevisionStore.php(1899): MediaWiki\Storage\RevisionStore->fetchRevisionRowFromConds(Wikimedia\Rdbms\DBConnRef, array)
#2 [internal function]: MediaWiki\Storage\RevisionStore->MediaWiki\Storage\{closure}(boolean, integer, array, NULL)
#3 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/libs/objectcache/WANObjectCache.php(1202): call_user_func_array(Closure, array)
#4 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/libs/objectcache/WANObjectCache.php(1076): WANObjectCache->doGetWithSetCallback(string, integer, Closure, array)
#5 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/Storage/RevisionStore.php(1902): WANObjectCache->getWithSetCallback(string, integer, Closure)
#6 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/Revision.php(1186): MediaWiki\Storage\RevisionStore->getKnownCurrentRevision(Title, integer)
#7 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/page/WikiPage.php(675): Revision::newKnownCurrent(Wikimedia\Rdbms\DatabaseMysqli, Title, integer)
#8 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/page/WikiPage.php(697): WikiPage->loadLastEdit()
#9 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/page/WikiPage.php(569): WikiPage->getRevision()
#10 [internal function]: WikiPage->{closure}(boolean, integer, array, NULL)
#11 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/libs/objectcache/WANObjectCache.php(1202): call_user_func_array(Closure, array)
#12 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/libs/objectcache/WANObjectCache.php(1076): WANObjectCache->doGetWithSetCallback(string, integer, Closure, array)
#13 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/page/WikiPage.php(579): WANObjectCache->getWithSetCallback(string, integer, Closure)
#14 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/page/WikiPage.php(230): WikiPage->getContentModel()
#15 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/page/WikiPage.php(217): WikiPage->getContentHandler()
#16 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/actions/Action.php(96): WikiPage->getActionOverrides()
#17 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/actions/Action.php(154): Action::factory(string, WikiPage, RequestContext)
#18 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/MediaWiki.php(156): Action::getActionName(RequestContext)
#19 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/MediaWiki.php(769): MediaWiki->getAction()
#20 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/includes/MediaWiki.php(524): MediaWiki->main()
#21 /mnt/nfs/labstore-secondary-tools-project/wmde-uca-test/public_html/core/index.php(43): MediaWiki->run()
#22 {main}
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptTue, Jan 2, 12:37 PM
Addshore renamed this task from wmde-uca-test wiki throws DB error to wmde-uca-test wiki throws DB error "RevisionStore for s53402__wmde-uca-test cannot be used with a DB connection for s53402__wmde?huca?htest".
Addshore moved this task from Backlog to Adam on the Multi-Content-Revisions board.

This looks like some sort of encoding issue for the comparison.

		if ( $dbWiki !== $storeWiki ) {
			throw new MWException( "RevisionStore for $storeWiki "
				. "cannot be used with a DB connection for $dbWiki" );
		}

With storeWiki and dbWiki being:

		// storeWiki s53402__wmde-uca-test
		// dbWiki    s53402__wmde?huca?htest

'-' -> '?h'

Mhhhm, Locally while trying to setup a wiki to test on with a dash in the name on my test setup I get the following while running the updater with the dbname 'test-dash':

[2b057575c45f3d9192c49fa1] [no req]   Wikimedia\Rdbms\DBConnectionError from line 811 of /var/www/mediawiki/includes/libs/rdbms/database/Database.php: Cannot access the database: Unknown database 'test' (db-master)
Backtrace:
#0 /var/www/mediawiki/includes/libs/rdbms/loadbalancer/LoadBalancer.php(1038): Wikimedia\Rdbms\Database->reportConnectionError(string)
#1 /var/www/mediawiki/includes/libs/rdbms/loadbalancer/LoadBalancer.php(680): Wikimedia\Rdbms\LoadBalancer->reportConnectionError()
#2 /var/www/mediawiki/includes/GlobalFunctions.php(2877): Wikimedia\Rdbms\LoadBalancer->getConnection(integer, array, boolean)
#3 /var/www/mediawiki/maintenance/Maintenance.php(1285): wfGetDB(integer, array, boolean)
#4 /var/www/mediawiki/maintenance/update.php(144): Maintenance->getDB(integer)
#5 /var/www/mediawiki/maintenance/doMaintenance.php(94): UpdateMediaWiki->execute()
#6 /var/www/mediawiki/maintenance/update.php(245): require_once(string)
#7 {main}
Addshore claimed this task.Wed, Jan 3, 2:32 PM
Addshore moved this task from Backlog to In Progress on the User-Addshore board.

So RevisionStore currently only takes in a wikiId. The checkDatabaseWikiId method then compares this wikIId with the Database Domainid. These two are not directly comparable.

In the example of this ticket the comparison can quite easily be seen with the encoding of the - in the dbname for the domain ID, but not in the wikid.

$wikiId = "s53402__wmde-uca-test";
$domainId = "s53402__wmde?huca?htest";

Should we actually just be comparing the wikiId? What is the wikIId? the dbname?
We should probably also be looking at the table prefix here?
We could likely just use a DatabaseDomain object throughout the RevisionStore and new Revision classes, which includes both the dbname and the prefix (and schema which i don't think we will be using)

Change 401746 had a related patch set uploaded (by Addshore; owner: Addshore):
[mediawiki/core@master] DNM WIP MCR switch from wikiId to DatabaseDomain

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

Addshore added a subscriber: daniel.Wed, Jan 3, 4:03 PM

Also tried something else in https://gerrit.wikimedia.org/r/#/c/401736/ but not got a working solution yet...

I'll wait for @daniel to take a look before I spend more time on this.

Addshore removed Addshore as the assignee of this task.Wed, Jan 3, 4:03 PM
Addshore moved this task from Adam to Backlog on the Multi-Content-Revisions board.

@aaron @Catrope any idea why dashes in the DB name get encoded as "?h"? Dashes are used to include table prefixes in DB names, right? Are they supposed to be included in the new "DomainID" as well?

Anomie added a subscriber: Anomie.Wed, Jan 3, 6:59 PM

T145840: 1.28-alpha with DBConnectionError: Cannot access the database: Unknown database ..., apparently, according to rMW847b91bf1fdf: Make database classes handle hyphens in $wgDBname where it was introduced.

The trick, I suppose, is to figure out how exactly the "wikiId" you have corresponds to the "DomainID". Should it compare to $domainId->getDatabase(), or does something more tricky need to happen?

Addshore renamed this task from wmde-uca-test wiki throws DB error "RevisionStore for s53402__wmde-uca-test cannot be used with a DB connection for s53402__wmde?huca?htest" to RevisionStore::checkDatabaseWikiId is broken in some cases.Thu, Jan 4, 10:06 AM
daniel added a comment.Thu, Jan 4, 1:15 PM

In s53402__wmde-uca-test, is "uca-test" genuenly part of the database name, or is it a table prefix?

Addshore added a comment.EditedThu, Jan 4, 1:17 PM

In s53402__wmde-uca-test, is "uca-test" genuenly part of the database name, or is it a table prefix?

That is all the database name.
We can get you access to the tool this is running on if you would like :)

tools.wmde-uca-test@tools-bastion-03:~$ cat ./public_html/core/LocalSettings.php |grep name
$wgSitename = "wmde-uca-test";
## The protocol and server name to use in fully-qualified URLs
$wgDBname = "s53402__wmde-uca-test";

tools.wmde-uca-test@tools-bastion-03:~$ cat ./public_html/core/LocalSettings.php |grep prefix
$wgDBprefix = "";
daniel added a comment.Thu, Jan 4, 4:46 PM

So, there's a hacky fix at https://gerrit.wikimedia.org/r/#/c/401736. The proper solution needs a little more work: https://gerrit.wikimedia.org/r/#/c/401746>

Change 401736 merged by jenkins-bot:
[mediawiki/core@master] [MCR] fix RevisionStore::checkDatabaseWikiId for DB names with dashes.

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

Addshore closed this task as Resolved.Sat, Jan 6, 12:47 PM
Addshore claimed this task.

Change 402903 had a related patch set uploaded (by Daniel Kinzler; owner: Daniel Kinzler):
[mediawiki/core@master] Specify encoding expected by DatabaseDomain::newFromId.

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