Add migrations_priority to database.yml

[dsounded]

Summary

Basically sometimes we need to specify the order for multiple database migrations, we can do that only with consecutive calls e.g. rake db:migrate:secondary rake db:migrate:primary ... rake db:migrate:nth (since positions in the yml file are ignored I believe)
But this is not always handy.

This requests brings new option for our database.yml called migrations_priority which will determine which db should migrate in which order.

By default all the priority set to 0, so it shouldn't affect previous behavior.

[eileencodes]

Thanks for the PR @dsounded! What's a use case where you need to specify the order? What order do they run in now - is it by timestamp or by database order in the yaml configuration? I think that the feature is interesting, but if they already run in order it's defined, why not change the order of the database yaml instead of adding a new option?

[dsounded]

@eileencodes the use case is: sometimes the primary db depends on the stuff from secondary, or, we need to transfer the data from primary to secondary and then remove if from primary.

Checked it and it really depends on the order in the config file.
So maybe you are right and the order is also appropriate solution here.

Maybe it's only about some readability enhancement, having the option it will be easier to put primary above secondary but still there will be an option to set the priority. And overall the process will be more explicit.

Or at least it would be good to document order behavior somewhere if this isn't accepted :)

[ghiculescu]

should this mutate the list?

Suggested change

	end.sort_by!(&:migrations_priority)
	end.sort_by(&:migrations_priority)

[kaspth]

Seems odd to me to bake in an order, when it seems like that'll be migration dependent.

Versus having a script to do something like:

# script/migrate/particular_migration.sh
bin/rails db:migrate:secondary
bin/rails db:migrate:primary # primary depends on new column in secondary.

I'd probably also supply particular VERSIONs to these migrations too. And if there's a place where we'd improve the API, I think we should focus on that.

[eileencodes]

I don't think this is the right API to solve the problem but you are right @dsounded, there is a flaw that I never considered in how the migrations work for multiple databases. At GitHub we don't have this problem because we haven't yet ported our multi-db migration handling over from the legacy db handling to the Rails way.

If an application has 2 databases and there's 1 migration to copy data from primary to animals and then another migration to delete the table from primary and they are run at the same time then the table from primary will get deleted before the data is copied over to animals.

I think the right way to solve this is not to add a priority that you have to change if the migration priority changes but rather to run all the migrations in order as if they are in the same directory. I think that we should run all migrations in timestamp order regardless of their directory - this would prevent the above issue. I'm not sure how easy this kind of change would be - the way connections work with the rake tasks get a little complex. I'll have to think on this a bit.

[dsounded]

@eileencodes makes sense, I guess this change also won't be hard to implement

[dsounded]

@eileencodes Hey

Just wanted to tweak the stuff locally, I created a commit which is 100% not production ready or something but shares the idea that it can be done like you described.

https://github.com/dsounded/rails/commit/012779a3ea20a7fb84e466bf18c940304722b09b

I am not sure if there is more efficient way since we do need to flip connections time to time (tried to decrease the needed amount of flips).
Definitely it will affect some other places, but maybe this one will help you in future :)

[dsounded]

The console output of example:

== 20210224105581 Test1: migrating ============================================
secondary
== 20210224105581 Test1: migrated (0.0002s) ===================================

== 20210224105591 Test1: migrating ============================================
primary
== 20210224105591 Test1: migrated (0.0000s) ===================================

== 20210224105601 Test2: migrating ============================================
secondary - 2
== 20210224105601 Test2: migrated (0.0002s) ===================================

[eileencodes]

Can you replace this PR with that commit or open a new PR? That new approach is going in the right direction and is close to what I was thinking. I do think it can be simplified a bit but will be easier to comment on in a PR 😄

[dsounded]

@eileencodes yeah, sure, will do

[dsounded]

#41664