Page MenuHomePhabricator
Create Task
Maniphest T183914

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

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}

Details

SubjectRepoBranchLines +/-
Specify encoding expected by DatabaseDomain::newFromIdmediawiki/coremaster+57 -35
DNM WIP MCR switch from wikiId to DatabaseDomainmediawiki/coremaster+94 -82
[MCR] fix RevisionStore::checkDatabaseWikiId for DB names with dashes.mediawiki/coremaster+123 -3
Customize query in gerrit

Related Objects

Event Timeline

WMDE-Fisch created this task.Jan 2 2018, 12:37 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 2 2018, 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".Jan 2 2018, 8:59 PM
Addshore added projects: User-Addshore, Multi-Content-Revisions.
Addshore moved this task from Inbox to Next on the Multi-Content-Revisions board.
Addshore added a comment.Jan 3 2018, 11:16 AM

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'

Addshore added a comment.Jan 3 2018, 11:22 AM

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.Jan 3 2018, 2:32 PM
Addshore moved this task from Unsorted 💣 to Back Burner 🏛️ on the User-Addshore board.
Addshore added a comment.Jan 3 2018, 3:05 PM

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)

gerritbot subscribed.Jan 3 2018, 3:44 PM

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

gerritbot added a project: Patch-For-Review.Jan 3 2018, 3:44 PM
Addshore added a subscriber: daniel.Jan 3 2018, 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.Jan 3 2018, 4:03 PM
Addshore moved this task from Next to Inbox on the Multi-Content-Revisions board.
daniel added subscribers: aaron, Catrope.Jan 3 2018, 4:26 PM

@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 subscribed.Jan 3 2018, 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?

Anomie merged a task: T184122: MCR refactoring broke tests/parser/parserTests.php.Jan 3 2018, 9:36 PM
Anomie added subscribers: cscott, Jdforrester-WMF, Sbailey and 2 others.
• MZMcBride subscribed.Jan 4 2018, 4:56 AM
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.Jan 4 2018, 10:06 AM
daniel added a project: User-Daniel.Jan 4 2018, 12:36 PM
daniel added a comment.Jan 4 2018, 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.EditedJan 4 2018, 1:17 PM
In T183914#3875044, @daniel wrote:

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.Jan 4 2018, 1:20 PM

ugh...

daniel added a comment.Jan 4 2018, 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>

gerritbot added a comment.Jan 4 2018, 5:27 PM

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

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-01-09 (1.31.0-wmf.16)).Jan 4 2018, 6:00 PM
Addshore closed this task as Resolved.Jan 6 2018, 12:47 PM
Addshore claimed this task.
gerritbot added a comment.Jan 8 2018, 8:14 PM

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

Tobi_WMDE_SW moved this task from Sprint Backlog to Done on the WMDE-QWERTY-XMAS-Sprint-2017-12-20 board.Jan 9 2018, 12:48 PM
Tobi_WMDE_SW moved this task from Proposed to Done on the WMDE-TechWish board.Jan 9 2018, 12:52 PM
Addshore moved this task from Back Burner 🏛️ to Closing ✔️ on the User-Addshore board.Jan 12 2018, 11:59 AM
WMDE-leszek mentioned this in T186017: Wikibase sqlite Jenkins builds failing due to obscure exception in RevisionStore.Jan 30 2018, 5:26 PM
Tobi_WMDE_SW moved this task from Done to Demoed on the WMDE-TechWish board.Feb 20 2018, 4:55 PM
CCicalese_WMF moved this task from Inbox to MCR-SDC Storage Layer - phase 1 on the Multi-Content-Revisions board.May 10 2018, 4:55 PM
CCicalese_WMF edited projects, added Multi-Content-Revisions (MCR-SDC Storage Layer - phase 1); removed Multi-Content-Revisions.
CCicalese_WMF moved this task from Backlog to Done on the Multi-Content-Revisions (MCR-SDC Storage Layer - phase 1) board.May 10 2018, 5:07 PM
CCicalese_WMF removed a project: Patch-For-Review.Jul 10 2018, 3:26 AM
gerritbot added a comment.Nov 15 2018, 3:38 PM

Change 401746 abandoned by Addshore:
DNM WIP MCR switch from wikiId to DatabaseDomain

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

Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:39 PM
gerritbot added a comment.Jun 19 2023, 6:33 PM

Change 402903 abandoned by Aklapper:

[mediawiki/core@master] Specify encoding expected by DatabaseDomain::newFromId

Reason:

No reply; boldly abandoning patch

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

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL