Page MenuHomePhabricator

Enable MariaDB/MySQL strict mode for MW during development and Jenkins jobs
Open, NormalPublic

Description

The tests jobs should fail under MariaDB/MySQL strict mode on CI db hosts. We need to populate the configuration bit under /etc/mysql/conf.d/ and announce it. Some extensions tests will most probably start falling magically.

There are 2 options to add:

sql_mode = 'TRADITIONAL'

which is really an alias to:

sql_mode = 'STRICT_TRANS_TABLES,STRICT_ALL_TABLES,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_ENGINE_SUBSTITUTION'

Ideally, we would set too:

sql_mode = 'TRADITIONAL, ONLY_FULL_GROUP_BY';

However, this last one only works properly on MySQL >= 5.7, and on lower versions of MySQL and MariaDB it doesn't detect functional dependencies properly (https://stackoverflow.com/questions/40084093/does-mariadb-not-support-functional-dependencies-in-select-statements-when-group ) , so it could create false positives (it would be nice to try it however, as it will likely detect true issues, but I agree it shouldn't be pussed hard, unlike the previous one, until all supported versions do the right thing.

Event Timeline

hashar created this task.Nov 23 2015, 10:56 AM
hashar raised the priority of this task from to Normal.
hashar updated the task description. (Show Details)
hashar added subscribers: zeljkofilipin, hashar, wpmirrordev and 15 others.

The technical parts of these are just adding the config to beta.my.cnf, it may be a bit more complex because need proper announcing, feedback, etc. (which are the non technical parts).

jcrespo updated the task description. (Show Details)Apr 27 2018, 9:00 AM

@hashar Updated with actual configuration to add to my.cnf, and a bit of comments.

Change 429386 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/core@master] tests: enable MariaDB/MySQL strict mode

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

hashar claimed this task.Apr 27 2018, 12:05 PM
hashar added a project: Quibble.

Can be done in the new Quibble containers

hashar added a subscriber: Stashbot.

I have used the wrong task number (T118371). Logs:

Change 429406 merged by jenkins-bot:
[integration/config@master] docker: enable MariaDB strict mode for Quibble
https://gerrit.wikimedia.org/r/429406

Mentioned in SAL (#wikimedia-releng) [2018-05-02T10:43:34Z] <hashar> Building quibble docker iamges 0.0.11-2 T118371 T193222

Change 430330 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/config@master] Bump Quibble jobs to 0.0.11-2

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

jcrespo renamed this task from Enable MariaDB/MySQL strict mode on CI slaves to Enable MariaDB/MySQL strict mode on CI db hosts.May 2 2018, 10:50 AM
jcrespo updated the task description. (Show Details)
jcrespo awarded a token.

@jcrespo we had MediaWiki tests running with sql_mode = 'TRADITIONAL' . I clearly remember at least a couple issues got caught thanks to that new setting!

I guess we next want to add ONLY_FULL_GROUP_BY ?

jcrespo added a comment.EditedJun 29 2018, 5:29 AM

ONLY_FULL_GROUP_BY on MariaDB (see bug https://jira.mariadb.org/browse/MDEV-11588 ) or MySQL <5.7 may give false positives, as on those versions it does not correctly detect functional dependencies. I would like to enable it, because there is likely hidden issues that can be detected by enabling ONLY_FULL_GROUP_BY -I know at least a couple of bugs that would have been avoided if it has been tested with it- , but I conceal that in some cases it could be problematic because those older versions.

Alternatively, I wonder if we could test with MySQL 8.0 instead, or as a tester configuration alternative, (which would have avoided most of the issues you hit by using an older MariaDB version).

Maybe we can ask the community, as long as we do not accept the simple response "testing more is bad".

hashar removed hashar as the assignee of this task.Sep 24 2018, 4:51 PM

For now CI has sql_mode = 'TRADITIONAL' unfortunately I lack time to think/babysit the addition of ONLY_FULL_GROUP_BY.

That should be doable via a change in mediawiki/core https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/429386/ with the culprit that as soon as the change merge some extensions/skins might start failing randomly. At least we are used to those breakages and can deal with them.

Note: we run MediaWiki tests with:

  • HHVM provided by Jessie which comes with mariadb 10.0.36-0+deb8u1
  • php7.0 provided by Stretch which comes with mariadb 10.1.26-0+deb9u1

Change 429386 had a related patch set uploaded (by Aaron Schulz; owner: Hashar):
[mediawiki/core@master] tests: enable MariaDB/MySQL strict mode

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

@hashar Can you confirm all of CI is running TRADITIONAL (including sql_mode configuration on mediawiki)? It is ok if not, but if it is, I would consider this resolved.

hashar added a comment.Apr 4 2019, 7:17 AM

TLDR: I thought enabling the setting in the server config was enough, but MediaWiki explicitly set it.

As part of this task, the MariaDB configuration for the CI containers have sql_mode = 'TRADITIONAL'.

Then there is https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/429386/ which set $wgSQLMode = 'TRADITIONAL;. I think I have send it simply to check whether any tests were failing.

It seems the server configuration can thus be overridden by setting that $wgSQLMode, and the doc states so:

includes/DefaultSettings.php
/**
 * SQL Mode - default is turning off all modes, including strict, if set.
 * null can be used to skip the setting for performance reasons and assume
 * DBA has done his best job.
 * String override can be used for some additional fun :-)
 */
$wgSQLMode = '';

So it is not quite set sorry :-\

Thanks for clarifying, I have commented on https://gerrit.wikimedia.org/r/429386

Change 429386 merged by jenkins-bot:
[mediawiki/core@master] tests: enable MariaDB/MySQL strict mode

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

hashar added a comment.Apr 4 2019, 7:47 AM

That should now be enabled for the master branch. We might then backport the change to other branches, notably REL1_31/REL1_32.

Then I guess after a few weeks we can add ONLY_FULL_GROUP_BY as well?

I have sent an email to wikitech-l to raise awareness about this change. https://lists.wikimedia.org/pipermail/wikitech-l/2019-April/091877.html

ONLY_FULL_GROUP_BY may be delayed until we can test on a mariadb version that supports functional dependency checking, like MySQL 5.7. I don't think that is possible at the moment. Let's keep this open to detect immediate and important regressions, then consider it as resolved.

Krinkle renamed this task from Enable MariaDB/MySQL strict mode on CI db hosts to Enable MariaDB/MySQL strict mode for MW during development and Jenkins jobs.Apr 6 2019, 7:57 PM