Page MenuHomePhabricator

Check minimum supported DBMS version when running update.php (and web updater)
Closed, ResolvedPublic

Description

When talking about T161232, I noticed that although we check mysqlDBMS version at install time, we don't seem to check it again.

I'm not suggesting we need to check it on every page view, like we do with PHP, but I think we should be checking it and confirming it's correct when running update.php; an action that would be done after upgrading MW php files, which could bring a bump in the near future if T161232 goes ahead

Event Timeline

Reedy created this task.Apr 3 2017, 1:53 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 3 2017, 1:53 PM
Paladox added a subscriber: Paladox.Apr 3 2017, 2:01 PM
Zppix added a subscriber: Zppix.Apr 3 2017, 2:13 PM

Shouldnt this be a blocker for T161232

Reedy added a comment.Apr 3 2017, 2:19 PM

Shouldnt this be a blocker for T161232

Indifferent, could be. Both are technically easy, T161232 is "politically" harder. This should really be done irrespective of that task

Though, this should also be done for all DBMS, not just MySQL

Reedy renamed this task from Check minimum supported MySQL version when running update.php (and web updater) to Check minimum supported DBMS version when running update.php (and web updater).Apr 3 2017, 2:20 PM
Reedy updated the task description. (Show Details)
Reedy triaged this task as Medium priority.Jul 13 2017, 4:39 PM
Reedy moved this task from Backlog to update.php on the MediaWiki-Maintenance-system board.

Ok, so the only place we store MySQL version is in MySQLInstaller::$minimumVersion...

(SQLite has a constant... Fixing that to at least be consistent in https://gerrit.wikimedia.org/r/#/c/365184/)

I want to avoid re-inventing the wheel, but instantiating an installer object/subclass just to get a version from seems wasteful. So we either need to copy the version numbers somewhere else (ugh. Unlike for the PHP version, there seems to be less of a need for this), or we make them static and pull them off that way. Then have something in the updater to do the version check like we do in the installer (be nice if we could factor this somewhere, and only have one place checking versions).

		$version = $conn->getServerVersion();
		if ( version_compare( $version, $this->minimumVersion ) < 0 ) {
			return Status::newFatal( 'config-mysql-old', $this->minimumVersion, $version );
		}

Could this make sense in the actual database classes? checkVersionIsSufficient or something less craply named

Kghbln added a subscriber: Kghbln.
Legoktm claimed this task.Oct 3 2017, 2:28 AM
Legoktm added a subscriber: Legoktm.

I guess I'll take this on.

Change 381924 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@master] Check minimum database server version when running update.php

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

Change 381924 merged by jenkins-bot:
[mediawiki/core@master] Check minimum database server version when running update.php

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

Change 385493 had a related patch set uploaded (by Legoktm; owner: Legoktm):
[mediawiki/core@REL1_30] Check minimum database server version when running update.php

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

Legoktm closed this task as Resolved.Oct 20 2017, 11:59 PM

Change 385493 merged by jenkins-bot:
[mediawiki/core@REL1_30] Check minimum database server version when running update.php

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