Automigrate & Autoupdate should use async.eachSeries

[klassicd]

Suggestion

I'm running into issues with Loopback 4's migrateSchema() where it doesn't create tables in a predictable order. The docs indicate you can specify the order tables are created for Postgres: https://loopback.io/doc/en/lb4/todo-list-tutorial-sqldb.html

In my testing both automigrate() & autoupdate() use async.each which runs queries in parallel. I believe these should be rewritten to use async.eachSeries.

https://github.com/strongloop/loopback-connector/blob/master/lib/sql.js#L78

https://github.com/strongloop/loopback-connector/blob/master/lib/sql.js#L1607

Use Cases

Run table migrations in predictable order to eliminate missing relationship errors like relation "public.relationalTableName" does not exist. I believe this could be as simple as rewriting async.each to be async.eachSeries.

I'm happy to submit a PR if the maintainers thought this was an acceptable solution.

Acceptance criteria

TBD - will be filled by the team.

[dhmlau]

Related to: loopbackio/loopback-next#2831

@bajtos, do you think the proposal from @klassicd is something we could try? Thanks.

[lotterfriends]

Please replace async.each with async.eachSeries! It took me days to find out why I got deadlocks at every automigration. If the order is not kept, complex FOREIGN KEY constructs with postgres are impossible. :(

[klassicd]

I've submitted a patch here #170

I would also like to bring forth another issue I'm running into with the models option in migrateSchema.

Imagine two tables with a foreign key constraint.
A -> Has Many -> B
B -> Belongs To -> A

If you want to preserve existing data, or the tables don't exist yet, you need need supply A before B in the models option, otherwise you will get a foreign key constraint error trying to create B first.

app.migrateSchema({existingSchema: 'alter', models: ['A', 'B']})

If you want to drop all existing data, then you need to supply B before A, otherwise you will get a foreign key constraint error trying to drop A first.

app.migrateSchema({existingSchema: 'drop', models: ['B', 'A']})

This may be a good case requirement for the migration framework in planning loopbackio/loopback-next#487

My current solution is to look for {existingSchema: 'drop'} and then run a custom method that drops all foreign keys and tables using cascade. After that I run migrationSchema({existingSchema: 'alter'}).

[ataft]

It should be noted that this fixes two of the .each() bugs, but that there are many more throughout the individuals connectors. Many of the individual connectors override autoupdate/automigrate and there are also many other functions that use each() instead of eachSeries().

One example is a PR I have to fix the order of MSSQL alterTable:
loopbackio/loopback-connector-mssql#228

Rather than waiting for the community to gradually fix these one-by-one, I'd recommend the loopback team seek them out and fix them all.