Add an option to terminate all existing connections to the database when resetting a database
Description
This PR introduces a new --force option for the graphile-migrate reset command, which allows terminating existing connections to the database when dropping it.
I often face a case during end-to-end testing when before each test case I want to reset the database. Unfortunately due to some lingering connections from previous tests, the graphile-migrate reset command sometimes throws an error as expected. This change allows to bypass that limitation.
Performance impact
Unknown.
Security impact
Unknown.
Checklist
- [x] My code matches the project's code style and
yarn lint:fixpasses. - [x] I've added tests for the new feature, and
yarn testpasses. - [x] I have detailed the new feature in the relevant documentation.
- [ ] I have added this feature to 'Pending' in the
RELEASE_NOTES.mdfile (if one exists). - [ ] If this is a breaking change I've explained why.
Thanks for the PR... I shall search my feelings :thinking:
Sorry for the delay... Even now, as I write this comment, I'm going back and forth on how I feel about it.
It makes me feel uncomfortable... I don't really like the idea behind it. But also, it's for the reset command, so it's not a huge deal since you'd never do this in production. But also, all it effectively does is call pg_terminate_backend immediately before resetting the DB (I think?), which is something you could do yourself with a lifecycle hook (I think?). But also, most software's response to having the connection to the DB terminated is to reconnect, and reconnecting to a DB that's been dropped and recreated before the migrations have completed executing is not ideal and could lead to uncomfortable outcomes (e.g. code generation might run on an earlier state, PostGraphile watch mode would get inundated with DDL changes, etc causing high CPU but more importantly potentially high disk churn).
I guess my main concern is that this opens up a can of worms and may result in people raising further PRs after this (e.g. "lock the DB after reset to prevent X, Y and Z", "create the database with a different name and then rename it", etc) when really they shouldn't be running their application against a DB that's being reset - and if they weren't, they wouldn't need this PR.
Given your use case, I'd just recommend that you teach your test suite to be better at closing lingering connections (or you use pg_terminate_backend() yourself in the tests).
A technique that works really well for PostGraphile's test suite is to create a template test DB, then each test creates a clone of this template using a random name to use for that test and drops it at the end. The best part of this is it allows all my tests to run in parallel, the only downside of which is that the fans on my PC suddenly sound like a hurricane as all 32 cores are maxed out! (Cloning a template DB, assuming it doesn't have loads of data, is generally a sub-second operation.)