Page MenuHomePhabricator

Force truncation of objectcache table during database schema update
Closed, ResolvedPublic

Description

From T270032. This seems to be the most effective and least disruptive solution to me.

MariaDB [wikidb]> describe objectcache;
+---------+----------------+------+-----+---------+-------+
| Field   | Type           | Null | Key | Default | Extra |
+---------+----------------+------+-----+---------+-------+
| keyname | varbinary(255) | NO   | PRI |         |       |
| value   | mediumblob     | YES  |     | NULL    |       |
| exptime | datetime       | YES  | MUL | NULL    |       |
+---------+----------------+------+-----+---------+-------+
3 rows in set (0.01 sec)

This schema change would change exptime to binary(14). On most installations, this is a straightforward change, because during schema update the table will be truncated and so new values will be filled correctly within the limit of the new type. However, this truncation is configurable, if --nopurge option is passed to the update script, the table will not be truncated, which means old values which are in DATETIME will be cutoff to fit binary(14) (if in sql non-strict mode), and the resulting values will not only be invalid, they could also cause problems to manifest elsewhere, see T270032#6716853.

And if in sql-strict mode, and the --nopurge option is passed, the values will not be truncated but I think the script will fail with an error like "Data too long for column..." since in that case the value are not allowed to be truncated.

So I think the simple solution to this is to temporarily force truncation of the table even if --nopurge option is passed. A message should explain this is for schema change reasons. This would be better than leaving the field to hold invalid values or for the update script to fail midway during update.

Event Timeline

Ammarpad created this task.Sun, Jan 3, 3:19 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSun, Jan 3, 3:19 PM
Ammarpad renamed this task from Force truncation of objectcache table during database object to Force truncation of objectcache table during database schema update.Sun, Jan 3, 3:23 PM
Ammarpad updated the task description. (Show Details)
Ammarpad added subscribers: Ladsgroup, Krinkle.
Ammarpad updated the task description. (Show Details)Sun, Jan 3, 3:30 PM
Krinkle added a comment.EditedTue, Jan 5, 1:01 AM

I don't think we should simultanously support the --nopurge option and then have update.php --nopurge on 1.36 result in the purging of the object cache. This would be surprising and unexpected behaviour that would likely result in a bug report that I'm not sure we can decline.

Alternative proposals:

  • We can remove the --nopurge option alltogether. It was introduced in r18531 for T10016. I could not find any rationale for supporting this step being skipped. Does anyone know of a use case for not purging caches when upgrading a wiki via update.php? We could leave a small handler until the next LTS that reads this option and emits an error if it is set, so that users aren't surprised, and so that we can discover if someone intends to use it.
  • Another way would be to introduce a logged update for 1.36 that is simply a one-line method that purges the cache once for installs from 1.35 and older. The update would fail with an error message if the user specified --nopurge, asking them that they need to run it without that option to upgrade to 1.36.
Ammarpad added a comment.EditedTue, Jan 5, 2:57 AM

Either way looks good to me. I think the only bad option is to change the type and assume that everyone is running update.php without --nopurge

But I note that, even though --nopurge option description claims it only causes purging of objectcache table, in reality it does more than that (via DatabaseUpdater::purgeCache() which is not only about objectcache). So the script already is somewhat not communicating what that option does clearly.

I think we should remove it and just return an error (unrecognized parameter which I think it does by default) if someone uses --nopurge. Would that work?

Assuming that's true, yes, that sounds great.

Change 657105 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] update.php: Purge database caches unconditionally after upgrade

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

Change 657105 merged by jenkins-bot:
[mediawiki/core@master] update.php: Purge database caches unconditionally after upgrade

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

Ammarpad closed this task as Resolved.Wed, Jan 20, 12:31 PM
Ammarpad claimed this task.
Ammarpad moved this task from Blocker to Done on the MW-1.36-release board.Wed, Jan 20, 12:34 PM