911: Setup migrations

[sarahsporck]

So this PR is basically the counterpart to #841. I did not use an external migration tool, but only created a migration table, as it was also suggested by @maxammann .

Pros:

• we do not need an external closed source dependency
• missing features such as rollback can be added later
• migrations can be written in *.java

Cons:

• missing experience on how migrations work and best practices
• potentially additional maintenance
• migrations cannot be written in sql (only indirect by passing the sql string to exposed)

While originally, I was pretty convinced that flyway is the better solution I now think that using a custom migration implementation has the same value, and is probably also implemented very similar.

Please, leave you're opinions on this in a timely manner so we can decide, which way we want to go :)

[f1sh1918]

Well i checked both PRs now and i feel good with flyway to be honest i just created some basic migration scripts in sql and kotlin and everything worked fine. For our concerns it should be fine. Since i have no real experience in database migration i would tend to use a library which is for my opinion easy to use and has a good documentation and you can find some easy examples

[michael-markl]

Personally (and somewhat surprisingly for me :D), I think I like this approach better as it has quite a little footprint, is easy to understand (no need to fight through flyway docs), and super flexible.

I think in both PRs you are missing one important aspect:
V1 should always create all tables at some baseline (I would suggest baseline=current tables in production).

This means, we'll have to create a lot of duplication in the V1 migration file (as we should not use the schema definitions in the "production code", as they can change in the future), but that's fine.
That being said, consequently, we should remove all other setupDatabase functions. We never want files other than migration files to alter the DB schema.

[michael-markl]

In a next step / issue, we could set up our CI to run all migrations starting from an empty table, just to have some confidence on push.

[f1sh1918]

In a next step / issue, we could set up our CI to run all migrations starting from an empty table, just to have some confidence on push.

But does this migration apply @michael-markl i couldn't get it work. Still having the email index

[f1sh1918]

Personally (and somewhat surprisingly for me :D), I think I like this approach better as it has quite a little footprint, is easy to understand (no need to fight through flyway docs), and super flexible.

I think in both PRs you are missing one important aspect: V1 should always create all tables at some baseline (I would suggest baseline=current tables in production).

This means, we'll have to create a lot of duplication in the V1 migration file (as we should not use the schema definitions in the "production code", as they can change in the future), but that's fine. That being said, consequently, we should remove all other setupDatabase functions. We never want files other than migration files to alter the DB schema.

OK so as i got that right, for every adjustment i make for the schema like adding a column, i have to write a migration.
Hm so it would make sense also to be able to execute sql files?!
Maybe a stupid question but can we not just run the setupDatabase functions once in the first migration file to create the baseline?

[michael-markl]

OK so as i got that right, for every adjustment i make for the schema like adding a column, i have to write a migration.

That's my understanding of DB migrations, yes.

Hm so it would make sense also to be able to execute sql files?!

I'm not entirely sure what your questions refers to, but you can also execute arbitrary SQL statements through exposed.

Maybe a stupid question but can we not just run the setupDatabase functions once in the first migration file to create the baseline?

Not a stupid question. But: I think, what DB migrations usually want to achieve is, that if you start with an empty DB and apply all migrations, you end up in a deterministic state. This means, that once you have written a migration (and executed it on the prod system), you should usually never change it. Otherwise, you cannot replicate the production schema by simply running all migrations again on an empty DB.

So let's assume, we would just use the setupDatabase functions in the V1 migration step. At some point we want to add/drop a column from a table. Then you would adjust the schema in the production code (that we call from our webservice). But with this adjustment, you also change the V1 migration step.

What we could do, I think, is redefine our "first" schema inside the V1 migration file (basically copy pasting every Schema we have into the migration file) and call exposed's SchemaUtils.create on these schemas.
Edit1: Another possibility would be to use pg_dump --schema-only on our current prod DB and to use that as our V1 script.

Edit2:

But does this migration apply @michael-markl i couldn't get it work. Still having the email index

I haven't tested it.

[timobrembeck]

Looks very promising 👍

I don't have a strong preference towards one of the two solutions, but I'd give this one a slight edge since having the possibility to use the exposed ORM in migrations can be a crucial advantage, especially when dealing with data migrations where you not only alter the schema of a table, but also its contents.

I noticed there is a fork of the initial code base, Suwayomi/exposed-migrations, which seems to be more or less maintained and has a few improvements, so I guess we could also import that library and provide all our suggestions as upstream contributions.

Other than that, I agree with everything @michael-markl said 😁

[sarahsporck]

@michael-markl did you create the baseline migration from the production db?

[michael-markl]

@michael-markl did you create the baseline migration from the production db?

No, that still needs to be done (see Todo in V2 step). I don't have access to the backend so I couldn't do it 😅

[sarahsporck]

@maxammann or @f1sh1918 could one of you send me the current production sql schemes? I think you just need to run pg_dump --schema-only. Correct me if I'm wrong @michael-markl 😅

[f1sh1918]

@maxammann or @f1sh1918 could one of you send me the current production sql schemes? I think you just need to run pg_dump --schema-only. Correct me if I'm wrong @michael-markl 😅

I've tried several times now. No idea why it didn't work. No error and nothing.. Just couldn't find any created schema sql file on the server. Gonna try to get it tommorow again...

Update:
Okay had to use postgres instead of backend user. Some error messages would be nice :)
I commited the baseline.sql file

[timobrembeck]

In my opinion, the migration table should be created by the migration framework and not by a migration itself.
It just does not make sense design-wise, since if you change the schema of the migration table and its usage in MigrationUtils, these changes need to be present from the first migration on (e.g. when setting up a new system), but this first migration is supposed to be immutable. So if you define your migration schema inside a migration, you will never be able to change it, which kind of defies the point of a migration.
So I would suggest to use SchemaUtils.create() somewhere inside the migration utils in this specific case. Most likely you will never change the migration schema anyways, but just in case you do, you are flexible enough to handle the transition in the code.

[michael-markl]

I think I agree :D So the changes to the migrations table should probably be handled separately.

So I would suggest to use SchemaUtils.create() somewhere inside the migration utils in this specific case. Most likely you will never change the migration schema anyways, but just in case you do, you are flexible enough to handle the transition in the code.

I think it is reasonable to create the migrations table and do any other Migrations-Table-Migrations (:D) just before we apply any actual migrations (but still inside the transaction, so if migration fails, we can safely go back to the old backend code (with the old migrations table))

[timobrembeck]

OK, but I would not handle migration-schema changes inside separate migrations, because then we would have to create a migrations-migration table to keep track of which migration-migrations are already applied 😂

[michael-markl]

Yep! I guess, changes to the migration schema will happen rarely enough to handle them in the code directly without keeping track of them in another table :D
Also, once these changes have been applied in prod, we can drop the code for migrating to new new migration schema :D

[michael-markl]

Once this gets deployed (I guess once it's merged), we should not forget to run migrateSkipBaseline on prod.

[sarahsporck]

@michael-markl is there anything holding this back to be merged? I also tested this locally and did not encounter any issues.

[michael-markl]

No, imo it can be merged. I am excited for this😻

[f1sh1918]

@sarahsporck
yes we should have waited for deactivation of auto update on production
i wanted to test it now and i get this error with an empty database

Exception in thread "main" app.ehrenamtskarte.backend.migration.DatabaseOutOfSyncException: 
Latest migration versions do not match: Version on DB null - Code Version 2

[sarahsporck]

@sarahsporck
yes we should have waited for deactivation of auto update on production
i wanted to test it now and i get with an empty database

Exception in thread "main" app.ehrenamtskarte.backend.migration.DatabaseOutOfSyncException: 
Latest migration versions do not match: Version on DB null - Code Version 2

Locally? And you did execute just the migrate command?

[f1sh1918]

@sarahsporck
yes we should have waited for deactivation of auto update on production
i wanted to test it now and i get with an empty database

Exception in thread "main" app.ehrenamtskarte.backend.migration.DatabaseOutOfSyncException: 
Latest migration versions do not match: Version on DB null - Code Version 2

Locally? And you did execute just the migrate command?

Ok got it working. I didn't know how to drop these functions in the beginning of the V0001_Baseline
I thought we just wait for staging server for migration but its fine :)

[sarahsporck]

Currently migratuons are not yet run in the ci. But can only executed locally. So as long as you don't accidentally migrate on the live system everything should be fine.

[michael-markl]

Since #916 is merged, migrations are run in PRs and on merge.