Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(migrations): improve migration system #299

Merged
merged 31 commits into from
Jan 13, 2022

Conversation

samuelmasse
Copy link
Contributor

Assures that the database version is persisted in a transaction that also contains any changes to the database schema. Tables are now created initially in a transaction all at the same time. Changes the status migration to create the table in the correct state after the migration has run (this is necessary to be future proof, in case a migration in the future needs to modify the schema again).

When adding a table, it is now required to add a migration (not only when modifying an existing table). This is required because not doing so might lead to problems in the future where running multiple migrations in a row could lead to corrupt database state, since a table that is needed does not exist in the expected format. We also can't allow the server to create its tables before running the migrations, because doing so could also lead to to problems in the future where a migration might expect to migrate a table as it was in a specific version but the server creates a table of a newer format that the migration doesn't expect.

Closes DEV-2216

@linear
Copy link

linear bot commented Jan 12, 2022

DEV-2216 Improve migration system

Comes from doing the migration in DEV-2205. We need to have a initial migration that creates all tables from the initial version of messaging. A migration needs to be added not only when a table is modified, but also when a table is added. In that case the migration creates the table as it was at that server version, and the down migration deletes the table.

When services register tables to be created, these should all be created afterwards at the same time using a transaction, and then the database version can be set (otherwise can lead to strange edge cases).

Copy link
Contributor

@laurentlp laurentlp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small suggestions otherwise looks fine to me!

packages/engine/src/database/service.ts Outdated Show resolved Hide resolved
packages/engine/src/meta/service.ts Outdated Show resolved Hide resolved
packages/engine/src/meta/service.ts Outdated Show resolved Hide resolved
packages/engine/src/meta/service.ts Show resolved Hide resolved
Base automatically changed from sm-client-tokens to master January 13, 2022 17:47
@samuelmasse samuelmasse merged commit c8ae63e into master Jan 13, 2022
@samuelmasse samuelmasse deleted the sm-migration-improvements branch January 13, 2022 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants