mozilla / fxa-auth-db-mysql

DEPRECATED - Migrated to https://github.com/mozilla/fxa
Mozilla Public License 2.0
12 stars 26 forks source link

Remove the FK relationships of devices with deviceCapabilities, deviceCommands? #485

Closed jrgm closed 5 years ago

jrgm commented 5 years ago

I was looking what the database migration required by https://github.com/mozilla/fxa-auth-db-mysql/pull/483 would look like.

The pt-online-schema-migration tool has some support for foreign keys, but there are big caveats.

To summarize the --alter-foreign-keys-method option, there are basically two strategies it can use: rebuild_constraints and drop_swap (and a meta strategy auto which will use the rebuild_constraints if it thinks it can get away with minimal blocking time. Otherwise, use drop_swap).

rebuild_constraints: it does the atomic RENAME TABLE as usual, and then uses ALTER TABLE on the child tables to drop the FK (which will now be referencing the _device_old table), and adds the FK back (under a new name (prepends a _ to the original name)).

drop_swap: Do SET foreign_key_checks=0, drop the devices table, and then rename the mirror table to devices.

In production, the deviceCommands table has about 20% of the number of rows in the devices table, so rebuild_constraints cannot be used.

With the drop_swap, there are two major drawbacks. First, for a brief period of time, the devices table will NOT exist. Second, the original devices table is dropped, so there is no way to reverse this change if something is wrong.

So...

Can we remove the foreign key relationships with the devices table and use specific "DELETE JOIN"'s instead?


The current constraints are:

deviceCommands: CONSTRAINT devicecommands_ibfk_2 FOREIGN KEY (uid, deviceId) REFERENCES devices (uid, id) ON DELETE CASCADE

deviceCapabilities: CONSTRAINT devicecapabilities_ibfk_1 FOREIGN KEY (uid, deviceId) REFERENCES devices (uid, id) ON DELETE CASCADE

blocks mozilla/fxa-auth-server#2547

jrgm commented 5 years ago

For completeness, these are the FK relationships in all our databases, after dropping and recreating locally:

mysql> select constraint_schema as db, constraint_name as name, table_name, referenced_table_name, delete_rule, match_option, update_rule from information_schema.referential_constraints where constraint_schema in ('fxa', 'fxa_profile', 'fxa_oauth');

+-------------+---------------------------+--------------------+--------------------------+-------------+--------------+-------------+
| db          | name                      | table_name         | referenced_table_name    | delete_rule | match_option | update_rule |
+-------------+---------------------------+--------------------+--------------------------+-------------+--------------+-------------+
| fxa         | devicecapabilities_ibfk_1 | deviceCapabilities | devices                  | CASCADE     | NONE         | RESTRICT    |
| fxa         | devicecommands_ibfk_1     | deviceCommands     | deviceCommandIdentifiers | CASCADE     | NONE         | RESTRICT    |
| fxa         | devicecommands_ibfk_2     | deviceCommands     | devices                  | CASCADE     | NONE         | RESTRICT    |
| fxa         | securityevents_ibfk_1     | securityEvents     | securityEventNames       | CASCADE     | NONE         | RESTRICT    |
| fxa_profile | avatar_selected_ibfk_1    | avatar_selected    | avatars                  | CASCADE     | NONE         | RESTRICT    |
| fxa_profile | avatars_ibfk_1            | avatars            | avatar_providers         | CASCADE     | NONE         | RESTRICT    |
+-------------+---------------------------+--------------------+--------------------------+-------------+--------------+-------------+
6 rows in set (0.01 sec)

mysql>
rfk commented 5 years ago

Oh, huh, I actually didn't realize we had these, and filed [1] under the assumption that we didn't.

Can we remove the foreign key relationships with the devices table and use specific "DELETE JOIN"'s instead?

+1 to this FWIW, both because it makes migrations easier, and because it's more consistent with how we manage foreign keys on our main tables (that is, we do it by hand). @shane-tomlinson thoughts?

[1] https://github.com/mozilla/fxa-auth-db-mysql/issues/479

shane-tomlinson commented 5 years ago

+1 to this FWIW, both because it makes migrations easier, and because it's more consistent with how we manage foreign keys on our main tables (that is, we do it by hand). @shane-tomlinson thoughts?

This seems reasonable to me, other than DELETE, are there any other operations or edge cases we need to worry about?

jrgm commented 5 years ago

This seems reasonable to me, other than DELETE, are there any other operations or edge cases we need to worry about?

Is that question with respect to the migration tool, or to what the change needs to be to relace ON CASCADE DELETE for these tables?

shane-tomlinson commented 5 years ago

Is that question with respect to the migration tool, or to what the change needs to be to relace ON CASCADE DELETE for these tables?

For the CASCADE DELETE

jrgm commented 5 years ago

I can't think of an edge case in moving to "delete from ... join", except maybe if there's an issue with doing an explicit delete while a cascade delete is still set on the table.