Aerie Database Migrations

[goetzrrGit]

Discuss ideas on how to handle database migrations.

Summary:
https://docs.google.com/document/d/1OXe1zvkchmK4OungzU0H6ehgmVg2Ra7T/edit

Tung's Document:
https://docs.google.com/document/d/19IMkMyVQcuavkcf9hAwG2GnzoMajAyqLd72InSoV9i0/edit

[goetzrrGit]

Dylan:

So I am looking into structured migrations today and have a few thoughts:

• the Hasura CLI for migrations puts them all in the deployment directory, rather than co-located with their services + our existing sql definition files
• hasura naturally wants to control all DB changes, so it wants to put the initial DB setup into a first migration - which makes sense to me, but perhaps doesn’t fit with how we currently manage our SQL

would love to have a meeting to discuss this more

[goetzrrGit]

Dylan D Stewart
43 minutes ago
Actually, I think https://github.com/diesel-rs/diesel/tree/master/diesel_cli does only what we want more-or-less.

Dylan D Stewart
42 minutes ago
Luke sent me another link that is very similar https://github.com/launchbadge/sqlx/tree/main/sqlx-cli

Dylan D Stewart
42 minutes ago
seems like the timestamping is a common pattern on migrations tooling, so might be best for us to just adapt to that? (edited)

Luke Jurgella
41 minutes ago
Kinda crude but maybe just in deployment/schema_version we could increment an integer so we get file based merge conflicts?

Luke Jurgella
40 minutes ago
I suppose that only works if there’s only one schema change that you’re rebasing on-top of

Matthew L Dailis
34 minutes ago
Here’s a discussion about switching from integers to timestamps: go-pg/migrations#65
I guess the grass is always greener on the other side 😅

Dylan D Stewart
31 minutes ago
I’ll take migration conflicts every damn time

Dylan D Stewart
30 minutes ago
The suggestion of a single person managing migrations is laughable for non-trivial systems

Dylan D Stewart
27 minutes ago
IFF we looked at custom CLI tooling https://atextor.de/2020/07/27/building-a-decent-java-cli.html - still very not hot on this idea

Matthew L Dailis
22 minutes ago
We could potentially add some sort of scripted check (perhaps as a PR guard) that makes sure that any new migrations come “after” all the migrations already on develop

Matthew L Dailis
22 minutes ago
Still custom code, but a bit more limited in scope, I would hope

Dylan D Stewart
21 minutes ago
agreed in the more limited scope being good there

Ryan R Goetz
15 minutes ago
Could we have another field in the PR template saying if this migration is order specific? It might help a little.

Ryan R Goetz
13 minutes ago
Maybe we might be okay with the timestamps, because in the PR you will rebase and get a migration folder from develop. You can test the PR locally and see if your migration gets messed up by another migration folder. If it does then your migration should be applied after then you can modify the timestamp of your folder.
👍
1

Dylan D Stewart
13 minutes ago
It think thats difficult to determine

Dylan D Stewart
12 minutes ago
But yeah, I think the timestamps thing isn’t something worth working around if there is good tooling surrounding the timestamp format

Matthew L Dailis
12 minutes ago
@Ryan R Goetz
I’m thinking along those same lines - that’ll work fine during development, but there’s an edge case that arises around releases

Dylan D Stewart
12 minutes ago
We can ensure our CI system protects us

Matthew L Dailis
12 minutes ago
E.g. I’m working on a feature that didn’t make it into a release - so the customer has now upgraded to a future version, so my migration won’t be run

Ryan R Goetz
10 minutes ago
@Matthew L Dailis
wouldn’t that be fine as you PR with the migration specific for the PR be isolated. When that branch gets merged in at a later point would that migration be applied correctly?

Ryan R Goetz
9 minutes ago
because you rebased and will be migrating off the latest database?

Ryan R Goetz
9 minutes ago
You might have to update you migration up.sql a bit in the PR if it fails (edited)

Ryan R Goetz
8 minutes ago
but you do that before merging back into develop

Matthew L Dailis
8 minutes ago
If it has a timestamp that is smaller than the latest migration on develop - systems that have already applied the latest migration will ignore your new migration

Ryan R Goetz
6 minutes ago
That is why in the PR you test it locally and update the folder’s name with a later time value.
18498298492_create_tag_table -> 18498568492_create_tag_table (edited)
👍
1

Matthew L Dailis
6 minutes ago
Yes - completely agree - I’m just saying it would be nice to have an automated way to catch that

Matthew L Dailis
6 minutes ago
Depending on how you do your rebase you may or may not notice

Ryan R Goetz
5 minutes ago
That is true…

[skovati]

We could definitely enforce the timestamp (or ID) ordering in CI on PR synchronization with a tiny shell script.

[mattdailis]

Kind of related/maybe goes without saying - but once a PR has been merged, I suppose we want to forbid editing past migration scripts.

[goetzrrGit]

I agree with that. Would it be a good idea to have PR for any possible edits to a past migration or lock them forever?

[dyst5422]

I think once they are a part of a release, they should forever be read only.

[mattdailis]

Reading up more on migrations, I found a couple sources that I liked:

• https://postgres.ai/blog/20220525-common-db-schema-change-mistakes
• https://docs.gitlab.com/ee/development/migration_style_guide.html

The first source is primarily focused on writing efficient migrations, but it has a lot of links to other good sources. One of those links is to the GitLab contributor documentation (the second source above). This is definitely worth a read - GitLab uses Postgres as well (though they use it via the Ruby/Rails interface), and, unlike many software-as-a-service companies, they need to support user-managed deployments just like we do.

They also mention that they used to have a "DOWNTIME" variable set per migration - but that they transitioned from that to zero (ish) downtime migrations. While it would be very cool for Aerie to be able to do zero-downtime migrations, I am not yet convinced that the benefit to users would be enough to outweigh the engineering time we'd need to put in.