Generate database structure via migration #3436
Conversation
| @@ -1,6 +1,9 @@ | |||
| const should = require('should') // eslint-disable-line | |||
| const setup = require('../setup') | |||
|
|
|||
| let objectCount = 0 | |||
| const generateName = (root = 'object') => `${root}-${objectCount++}` | |||
There was a problem hiding this comment.
The TeamType sequelize model doesn't have 'unique: true' set on the name field. This means, for a clean install, the constraint doesn't get applied and the tests pass.
However, the migration that was added to introduce TeamType does set the unique constraint - so that is what we have in production.
I chose to match production with our baseline - so the constraint is applied. So I had to update the tests to use unique names below.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #3436 +/- ##
==========================================
+ Coverage 41.31% 41.54% +0.22%
==========================================
Files 610 611 +1
Lines 22881 22944 +63
Branches 5524 5531 +7
==========================================
+ Hits 9453 9531 +78
+ Misses 13428 13413 -15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
I've given everything but the 1000 line migration the once over and looks ok so far. |
There was a problem hiding this comment.
I've done a full pass of the migration file and forge/db/migrations/index.js, nothing stood out of the ordinary including a comparison with my local PostgreSQL tables. That being said it's a super long migration, and not particularly human readable.
If this works as expected on temporary staging (create new), and real staging (no-op upgrade existing), I'd expect it to behave in production too, thus 👍
Closes #3426
Description
As detailed in #3426, to enable scalability of the app we need to change how we handle migrations and the initialisation of the database.
When running on Postgres, the database migration logic that is applied when the app starts is now wrapped in a transaction that holds a lock on the
MetaVersionstable. This ensures two concurrently started apps do not both try to apply migrations.With that fixed, the next issue is that we use
sequelize.sync()to initialise the database. We have seen this taking 10+s in staging. In a scaled-out environment, having two instances running sync at the same time causes errors.To fix that, this PR introduces a new 'baseline' migration that, if no tables exist, will create the entire database structure as expected today.
For a new install, the migration logic will skip 'old' migrations (as it does today), but then apply the baseline migration and any ones that follow.
For an existing install, the baseline migration will no-op as the tables already exist.
Creating this baseline migration has been quite the task. I'm describing the process here for future reference, but also to document what testing I've done.
I wrote some code to generate the bulk of it based on our models. I then used the DDL of each table from an sqlite instance to check all of the generated fields and fill in anything missing - including indexes.
I then ran the migration to generate a new database (sqlite), and manually compared the constraints, indexes and DDL of each table against an existing database.
I then ran against postgres.
Finally I ensured our unit tests passed cleanly against both sqlite and postgres.
Whoever reviews this, please do a test of running this up with a new sqlite and/org pg database and verify some basic ff functionality.