Database migrations are performed wrapped in transactions that require single connection with database.
This diff fixes problem with deadlocks, that appeared when trying to update permissions for all threads, which was caused by using connection pool.
After the change, until all migrations are performed, only one connection to database is established.
Details
Details
Run multiple times migration from D4071, that caused problems with locks. The issue no longer appears.
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
Comment Actions
Run multiple times migration from D4071, that caused problems with locks. The issue no longer appears.
WOO!!! :)
| keyserver/src/database/database.js | ||
|---|---|---|
| 20–22 ↗ | (On Diff #14430) | Let's move this to the top so we can skip the await if migrationConnection is already set |
| 26 ↗ | (On Diff #14430) | We can probably skip this and the scriptContext stuff, since the migrations won't ever be running in a script context |
| 105 ↗ | (On Diff #14430) | One big change this will cause is that any script will be using just a single connection. To keep the scope of the change narrowly focused on resolving the issue at hand, it would probably be best to default to using the standard loadPool mechanism. We can replace setMigrationsConcluded() with setConnectionContext() that takes something like: type ConnectionContext = { +migrationsActive?: boolean, }; Then we can set it to { migrationsActive: true } when starting the migrations, and to { migrationsActive: false } after they finish. What do you think? |
| keyserver/src/database/database.js | ||
|---|---|---|
| 105 ↗ | (On Diff #14430) | Yes, it seems to be better solution |
| keyserver/src/database/database.js | ||
|---|---|---|
| 110 ↗ | (On Diff #14457) | Looks great!! One last thing – can we add this? if (!connectionContext.migrationsActive) { migrationConnection.end(); migrationConnection = undefined; } |