Use flyway to perform database migrations on startup.

[posulliv]

Description

Add support for running mirations automatically with flyway on server startup. Since users may not want this, I added a config option to opt out of having the gateway process perform migrations. Migrations will only be performed by the gateway process if this config option is enabled.

Additional context and related issues

This change will allow us to add backend DB support for Oracle in the future.

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( X) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text:

* Fix some things. ({issue}`issuenumber`)

[mosabua]

Awesome.. we just talked about this recently in the dev sync - fyi @willmostly

[willmostly]

Just a few minor comments - this looks good to me overall and should be a nice improvement!

[mosabua]

Don't have time to review but here are some thoughts:

• Can we check database permissions before scripts are run
• I agree that we want to have this migration on by default, but be able to deactivate it
• Must be part of the app start so it works for any deployment
• If migration fails or some problem is found the app startup should probably fail hard so the app is not used at all in some weird state
• have to figure out how this works in a cluster where we have multiple instance, maybe first instance gets a lock and then all other instances are prevented from starting (or fail to start) until the migration is done

[posulliv]

thanks @mosabua Replying to your comments:

Can we check database permissions before scripts are run

flyway will fail if database permissions are incorrect and the gateway process will not start up. trino has the same behavior for database backed resource groups.

I agree that we want to have this migration on by default, but be able to deactivate it

Got it. This is implemented in the PR now with an option to disable it.

Must be part of the app start so it works for any deployment

This is in the PR. Migrations occur on startup unless they are explitly disabled in the data store config.

If migration fails or some problem is found the app startup should probably fail hard so the app is not used at all in some weird state

Yep, this is behavior as implemented in this PR. If a migration fails the process will not start and the logs will contain the migration error.

have to figure out how this works in a cluster where we have multiple instance,

Are you saying we need to handle this in this PR? This state indicating one gateway process is performed migrations would need to be maintained somewhere. Seems like backing DB is the only only place we could store it when there are multiple gateway processes?

[mosabua]

Awesome.. thanks for answering @posulliv

The only thing I think we need to figure out is how to deal with multiple nodes. Can you find out how flyway deals with this. Feels like it should support that out of the box somehow.

Maybe the first node that starts up locks the db for the upgrade and any additional nodes connecting just fail or have to wait until migration is completed. We just need to avoid having multiple nodes connecting and all of them starting to migrate the database and ending up with a mess.

And yeah.. maybe if we document it properly we can get this PR merged with it only supporting single node usage and just explain that people have to take that into account in their upgrade procedure (which we probably have to document soon as well).

[posulliv]

Yeah @willmostly suggested flyway might take care of this as well. It does in fact use locks - https://documentation.red-gate.com/fd/postgresql-database-235241807.html#

By default Flyway uses a transactional lock with PostgreSQL

So the behavior should be the first gateway instance to start will get the lock and run the migrations. Other instances might fail during startup but if this is a k8s deployment, once they are automatically restarted they should be good as migrations will be completed.

[mosabua]

Sweet.. lets just add that to the docs then.