sastraxi/pgsh

Feature: destroy/drop could wait for connections to close

nafg opened this issue · 3 comments

nafg commented

Currently it will first make me type in the name, then if other clients are connected I'll get a verbose error:

Type the database name to drop it: lrbcol
Dropping lrbcol...
pgsh destroy <target>

Destroys the given database. This cannot be undone!

Positionals:
  target  The database to drop. You can maintain a blacklist of databases to
          protect from this command in your .pgshrc          [string] [required]

Options:
  --version      Show version number                                   [boolean]
  -i, --iso      show timestamps in ISO-8601 format   [boolean] [default: false]
  --verbose, -a  introspect databases and show their latest migrations
                                                      [boolean] [default: false]
  --help         Show help                                             [boolean]

{ error: database "lrbcol" is being accessed by other users
    at Connection.parseE (/usr/lib/node_modules/pgsh/node_modules/pg/lib/connection.js:555:11)
    at Connection.parseMessage (/usr/lib/node_modules/pgsh/node_modules/pg/lib/connection.js:380:19)
    at Socket.<anonymous> (/usr/lib/node_modules/pgsh/node_modules/pg/lib/connection.js:120:22)
    at Socket.emit (events.js:193:13)
    at addChunk (_stream_readable.js:295:12)
    at readableAddChunk (_stream_readable.js:276:11)
    at Socket.Readable.push (_stream_readable.js:231:10)
    at TCP.onStreamRead (internal/stream_base_commons.js:154:17)
  name: 'error',
  length: 151,
  severity: 'ERROR',
  code: '55006',
  detail: 'There is 1 other session using the database.',
  hint: undefined,
  position: undefined,
  internalPosition: undefined,
  internalQuery: undefined,
  where: undefined,
  schema: undefined,
  table: undefined,
  column: undefined,
  dataType: undefined,
  constraint: undefined,
  file: 'dbcommands.c',
  line: '862',
  routine: 'dropdb' }

Instead, it would be nice if it could check for exclusive access before making me type in the name. Even better, if it is in use, instead of quitting it could tell me, wait, and try again in 10 seconds or so. Or it could say "please close other connections then press enter" etc., waiting for the user rather than some amount of time. Or perhaps, since I have to type in the name anyway, it could say "db still in use, close and only then enter name."

The big advantage of a timed retry loop is that I may not know how many clients are connected. For instance, one or more testing apps, my IDE, a psql session in some other terminal tab, etc. So I'd like pgsh to tell me when it's no longer in use, that way if I stop my app I know whether I'm done, and so on.

Of course the behavior could be opt in or opt out or otherwise controlled by command line flags.

nafg commented

I guess a more minor but broader issue is that the database error seems to make it print the usage screen, unless that's intentional.

sastraxi commented

@nafg thanks for opening this issue!

The (unnecessary / incorrect) error output is because we're returning the wrong code from the subcommand -- will fix this in the next version.

As for waiting for processes to exit, we could do an incremental backoff to keep trying to drop using e.g. https://www.npmjs.com/package/backoff and do something like:

$ pgsh destroy mydb
Dropping mydb...
There is 1 other session using the database. (waiting)
^C
Did not drop mydb!
$

Exiting with code 0 if we dropped, 1 if we didn't. I'm tempted to add a flag to fail instantly (is it wrong that I want to re-use -f?)

Instead, it would be nice if it could check for exclusive access before making me type in the name.

I don't know if I agree. I'm interpreting this as "if we had bash shell completion, it should only complete databases that would be valid with the current subcommand". Is my interpretation correct?

nafg commented
  • 👍1