HomePhabricator

Guarantee the `key_position` key is created properly

Description

Guarantee the key_position key is created properly

Summary:
Ref T12987. I was focused on the RefCursor table and overlooked that we need some care on this key.

It's currently possible to run bin/storage upgrade --no-adjust, then start Phabricator, and end up with duplicate records in this table. If you try to run bin/storage adjust later, it will try to add the unique key but fail. This is unusual for normal installs (they usually do not use --no-adjust) but we do it in the cluster and I did this exact thing on secure.

Normally, to avoid this, when a new table with a unique key is introduced, we also add a migration to explicitly add that key.

This is mostly harmless in this case. Fix this mistake (force the table to contain only unique rows; add the key) and try using LOCK TABLES to make this atomic. If this doesn't cause problems we can use this in similar situations in the future.

The "alter table may unlock things" warning comes from here:

https://dev.mysql.com/doc/refman/5.7/en/lock-tables.html

It seems like it's fine to issue UNLOCK TABLES even if you don't have any locks, so I think this script should always do the right thing now, regardless of ALTER TABLE unlocking or not unlocking tables.

Test Plan: Ran bin/storage upgrade -f, saw table end up in the right state. I'll also check this on secure, where the starting state is a little messier.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12987

Differential Revision: https://secure.phabricator.com/D18623

Details