-
Notifications
You must be signed in to change notification settings - Fork 67
Fix running multiple migration process in parallel #5329
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
base: main
Are you sure you want to change the base?
Conversation
This rewrites the database migration code to: - avoid deadlocks when running multiple migration processes at the same time with a `CREATE INDEX CONCURRENTLY` statement - allow us to remove some migrations from the code base and mark them as intentionally removed - allow us to modify some migrations and declare alternate checksums for previous versions of the migration
Deploying matrix-authentication-service-docs with
|
| Latest commit: |
21f1b8a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://008dd77d.matrix-authentication-service-docs.pages.dev |
| Branch Preview URL: | https://quenting-migration-lock.matrix-authentication-service-docs.pages.dev |
This allows us to roll back to older versions of MAS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR rewrites the database migration system to fix deadlocks when running multiple migration processes in parallel, particularly with CREATE INDEX CONCURRENTLY statements. The new implementation replaces SQLx's default migration behavior with a custom solution that provides better concurrency handling, validation, logging, and observability.
Key changes:
- Custom migration locking with exponential backoff using
try_acquireto prevent deadlocks with concurrent index creation - Migration validation system supporting intentionally removed migrations and alternate checksums for modified migrations
- Enhanced observability with dedicated logging and tracing spans for each migration
Reviewed changes
Copilot reviewed 7 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
crates/storage-pg/src/lib.rs |
Implements new custom migration system with advisory lock handling, validation logic, and migration tracking arrays |
docs/development/database.md |
Documents procedures for removing and modifying migrations with examples |
crates/cli/src/commands/server.rs |
Updates to use new mas_storage_pg::migrate() and pending_migrations() functions |
crates/cli/src/commands/syn2mas.rs |
Replaces MIGRATOR.run() with mas_storage_pg::migrate() call |
crates/cli/src/commands/database.rs |
Replaces MIGRATOR.run() with mas_storage_pg::migrate() call |
crates/cli/src/commands/config.rs |
Replaces MIGRATOR.run() with mas_storage_pg::migrate() call |
crates/storage-pg/Cargo.toml |
Adds crc and tokio dependencies for lock ID generation and async sleep |
Cargo.lock |
Updates lock file with new dependencies |
crates/storage-pg/.sqlx/*.json |
Adds SQLx query metadata for new database queries |
Files not reviewed (2)
- crates/storage-pg/.sqlx/query-2f66991d7b9ba58f011d9aef0eb6a38f3b244c2f46444c0ab345de7feff54aba.json: Language not supported
- crates/storage-pg/.sqlx/query-fbf926f630df5d588df4f1c9c0dc0f594332be5829d5d7c6b66183ac25b3d166.json: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #5298
This rewrites the database migration code to:
time with a
CREATE INDEX CONCURRENTLYstatementintentionally removed
for previous versions of the migration
We also get a few bonuses from this:
relation "_sqlx_migrations" already exists, skippingnotice on startup