pgagroal icon indicating copy to clipboard operation
pgagroal copied to clipboard

Refactor command names in `pgagroal-cli`

Open fluca1978 opened this issue 2 years ago • 6 comments

The pgagroal-cli command has, according to me, some inconsistency within the commands. For example:

  • gracefully does a graceful shutdown;
  • cancel-shutdown does cancel the above request (but no "gracefully" appears in the name)
  • stop does a not graceful halt of the system.

I propose to choose a common word, e.g., shutdown and add a subcommand to specify the shutdown method:

  • shutdown will result to be the same as gracefully
  • shutdown immediately will result the same as stop
  • cancel-shutdown will remain the same and now it is clear which commands are related to the shutdown.

Same thing for the flush-xxx commands that can become flush with a subcommand related to the mode, e.g., flush gracefully.

The reset command is tied to the Prometheus info, and reset-server is the one that does reset a configured server. Appears to me it is better to have the reset command to be tied to the working of pgagroal, not of Prometheus, because it is more important and the metrics could be an optional feature (meaning it is not enabled).

Old commands should remain, to avoid breaking working clients and batches, and should report on stderr that the command has been deprecated.

fluca1978 avatar May 30 '22 14:05 fluca1978

This is what I would imagine:

% pgagroal-cli help
...
Commands:
  flush [mode] [database]  Flush connections according to <mode>.
                           Allowed modes are:
                           - 'gracefully' (default) to flush all connections gracefully
                           - 'idle' to flush only idle connections
                           - 'all' to flush all connections. USE WITH CAUTION!
                           If no <database> name is specified, applies to all databases.
  is-alive                 Is pgagroal alive?
  enable      [database]   Enables the specified databases (or all databases)
  disable     [database]   Disables the specified databases (or all databases)
  shutdown [mode]          Stops pgagroal pooler. The <mode> can be:
                           - 'gracefully' (default) waits for active connections to quit
                           - 'immediate' forces connections to close and terminate
                           - 'cancel' avoid a previously issued 'shutdown gracefully'
  status                   Status of pgagroal
  details                  Detailed status of pgagroal
  switch-to <server>       Switches to the specified primary server
  reload                   Reload the configuration
  clear <what>             Resets either the Prometheus statistics or the specified server.
                           <what> can be
                           - 'server' (default) followed by a server name
                           - a server name on its own
                           - 'prometheus' to reset the Prometheus metrics

note that I don't think reset can be used since it has to be both a deprecated command and a new one, so I've chosen clear instead. I imagine the program doing something like this:

% pgagroal-cli reset
Command <reset> is deprecated by <clear prometheus>

but the command works as before. On the other hand:

% pgagroal-cli reset prometheus

works too.

fluca1978 avatar May 31 '22 09:05 fluca1978

Yeah, I agree.

But, this would be a breaking change, and hence material for pgagroal 2.0.

jesperpedersen avatar May 31 '22 13:05 jesperpedersen

I do have a working prototype. I need some time to finish up a few things and clean up the ode, but essentially reflect what I've already proposed.

The version bumping is clearly up to you, since my idea is not to break the existing interface, rather to make it deprecated. Mind to pass to version 2 once the deprecated commands disappear? But I'm fine with any decision.

fluca1978 avatar May 31 '22 14:05 fluca1978

The 1.x version should be default, but the 2.x could be optional.

Something else that would be nice is command line completion for the major shells

jesperpedersen avatar May 31 '22 14:05 jesperpedersen

Before going on this work, a little set of doubts and ideas:

  • command shutdown or halt?
  • subommand gracefully or smart (as PostgreSQL's pg_ctl does for a stop)?
  • implement a start command to fork()+exec() somehow pgagroal?

I don't have any particular opinion on any of the above, so feel free to suggest as you like. The start command could be somehow difficult (need to find pgagroal in PATH, I need to re-study exec() and friends) but could pone the basis for another interesting command: restart. However, we could postpone this to another piece of work...

fluca1978 avatar Jun 01 '22 07:06 fluca1978

I would just leave as is -- shutdown and gracefully.

I don't think we need a start command, as most systems (likely) will do a service start of pgagroal in daemon mode

jesperpedersen avatar Jun 01 '22 11:06 jesperpedersen