[keyserver] add UUID column to ids table and necessary migrations
AbandonedPublic
Actions

Authored by varun on Jul 15 2022, 1:07 PM.

Details

Reviewers

atul
• jon
• abosh
ashoat

Summary

Create the UUID column if it doesn't exist, backfill any NULL cells with a UUID, and then add a UUID for each new row with a trigger

This is the first step in moving away from auto-incremented IDs, so that we can move user data over to DynamoDB

Test Plan

Blew up my database and checked that all migrations succeeded and final DB state was correct
Checked that all the queries in the migration were idempotent by manually manipulating the table

Diff Detail

Repository

rCOMM Comm

Branch

user-id-generator (branched from master)

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

varun created this revision.Jul 15 2022, 1:07 PM

Herald added subscribers: • adrian, tomek, ashoat. · View Herald TranscriptJul 15 2022, 1:07 PM

Harbormaster completed remote builds in B10561: Diff 14536.Jul 15 2022, 1:19 PM

varun requested review of this revision.Jul 15 2022, 1:19 PM

• abosh added inline comments.Jul 16 2022, 7:57 AM

keyserver/src/database/migration-config.js
102	You can simplify this since we're just checking to see if `result` is falsey in the first part of the condition.
126	You can simplify this since we're just checking to see if `result` is falsey in the first part of the condition.

Probably a good idea to include me on all diffs in your stack that touch MySQL given the ongoing migration and that there is some undocumented context I have on the whole system

keyserver/src/database/migration-config.js
96	If we keep adding migrations into this file it feels like it will get messy fast. Should we put this migration in a separate file? (And maybe make a Backlog task to move each migration into its own file.)
112	We should only set a UUID on entries where `table_name` is `users`, since we aren't ready to migrate the rest of the tables over
115	Can you link the docs for this function on MySQL 5.7 and MariaDB 10.8? Wondering which kind of UUID they each use
130	We don't have any `TRIGGER`s today and I'm not sure we should introduce any. The more complicated your SQL usage is, the more you end up getting locked into your database engine. Instead, I wonder if we can just update `createIDs` to generate a UUID via JS. We would just need to make sure we're using the same UUID algo on JS vs. SQL. Probably would be easier to just update `setUUIDsInIdsTable` to use a JS function as well. Then we also wouldn't need to worry about potential MySQL 5.7 vs. MariaDB 10.8 issues there.
keyserver/src/database/setup-db.js
66	It's weird that all UUIDs will continue having an `auto_increment` ID as well. Is there a way we could build a schema without that confusion? It seems like either we could have two separate tables, or we could have a schema where either `id` or `uuid_id` are set. I agree we should have a `ID -> table_name` mapping for both types of IDs, though