Refactor command names in `pgagroal-cli` #253

fluca1978 · 2022-05-30T14:51:45Z

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.

The text was updated successfully, but these errors were encountered:

fluca1978 · 2022-05-31T09:42:04Z

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.

jesperpedersen · 2022-05-31T13:23:59Z

Yeah, I agree.

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

fluca1978 · 2022-05-31T14:08:22Z

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.

jesperpedersen · 2022-05-31T14:13:10Z

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

fluca1978 · 2022-06-01T07:22:23Z

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...

jesperpedersen · 2022-06-01T11:54:20Z

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

Now `pgagroal-cli` has a set of "logically" grouped commands and subcommands. For example, all the commands related to shutting down the pooler are under the `shutdown` command, that can operate with subcommands like `gracefully`, `immediate` or `cancel`. In order to provide this capability, two new functions have been introduced: - `parse_command()` accepts the command line and seek for a command, possibly its subcommand, and an optional "value" (often the database or server name). - `parse_deprecated_command()` does pretty much the same thing but against the old command. Thanks to this, old commands can still work and the user will be warned about their deprecation, but the interface of `pgagroal-cli` is not broken. Both functions requires to know the offset at which start seeking for a command, and that depends on the number of options already parsed via `getopt_long()`. Since the `&option_index` is valued only for long options, I decided to use the `optind` global value, see getopt_long(3). This value is initialized with the "next thing" to seek on the command line, i.e., the next index on `argv`. In the case the command accepts an optional database name, the database value is automatically set to '*' (all databases) in case the database name is not found on the command line. Therefore: pgagroal-cli flush idle is equivalent to pgagroal-cli flush idle '*' On the other hand, commands that require a server name get the value automatically set to "\0" (an invalid server name) in order to "block" other pieces of code. Moroever, if the server has not been specified, the command is automatically set to "unknown" so that the help screen is shown. The `pgagroal-cli` has a set of `pgagroal_log_trace()` calls whenever a command is "parsed", so that it is possible to quickly follow the command line parsing. Also, since the `pgagroal-cli` exists if no command line arguments have been specified, the safety check aboutt `argc > 0` around the command line parsing have been removed. In the case the user specified an unknown command, she is warned on stdout before printing the `usage()` help screen. Deprecated commands are notified to the user via a warning message, printed on stderr, that provides some hints about the correct usage of the new command. Documentation has been updated and improved. A specific documentation section about deprecated commands have been introduced. Close agroal#253

This commit introduces two compile time macros: - PGAGROAL_VERSION is a string and contains the application version that will be shown in various application messages. - PGAGROAL_VERSION_NUM is a numeric value that can be used to compare the running version with another number, so for example to limit features or messages. `PGAGROAL_VERSION` replaces the `VERSION` macro that was defined in the main header file. Moreover, the message printed at level INFO when booting the pooler has been enhanced to show also the version that is starting, like for instance: pgagroal: v1.5.0 started on *:5432 See agroal#253, agroal#289, agroal#290 Close agroal#292

Three new constants are introduced here: - PGAGROAL_MAJOR_VERSION (number) - PGAGROAL_MINOR_VERSION (number) - PGAGROAL_PATCH_VERSION (number) - PGAGROAL_VERSION (string) that represent, as integer literals and string composition, the parts of the version number. The values are copied from the variables in the `CmakeLists.txt` file that creates the building flow, so changing the values in a single place changes and propagates the changes all over the source code. A few utility functions have been introduced to compose the three constant into a unique number that can be used to compare the version currently running, so that features and/or message can be targeted as conditionals. Introduced functions are: - pgagroal_version_as_number() returns an unique sortable version number. For example, version 1.5.0 is returned as 15000, version 1.6.2 as 16002, and so on; - pgagroal_version_number() returns the currently running version number, that is invokes pgagroal_version_as_number() with the predefined constants. The `pgagroal_version_as_number()` accepts individual values, while `pgagroal_version_number()` provides the currently running version number. The idea is that, thanks to both the functions, it is possible to check for a feature that depends on a version in a way like the following one: if (pgagroal_version_numer() >= pgagroal_version_as_number(2,1,0)) // version is greater than 2.1.0 ! The utility function `pgagroal_version_ge()` does exactly the above: it accepts the three parts of a version number and checks if the currently running version is greater or equal (hence, 'ge') of the specified triplet, so that the above piece of code becomes: if (pgagroal_version_ge(2,1,0)) // version greater or equal 2.1.0 Close agroal#292 See agroal#253, agroal#289, agroal#290, agroal#293

…groal-admin` Now `pgagroal-cli` has a set of "logically" grouped commands and subcommands. For example, all the commands related to shutting down the pooler are under the `shutdown` command, that can operate with subcommands like `gracefully`, `immediate` or `cancel`. In order to provide this capability, two new functions have been introduced as utilities: - `parse_command()` accepts the command line and seek for a command, possibly its subcommand, and an optional "value" (often the database or server name). - `parse_deprecated_command()` does pretty much the same thing but against the old command. Thanks to this, old commands can still work and the user will be warned about their deprecation, but the interface of `pgagroal-cli` is not broken. Both functions requires to know the offset at which start seeking for a command, and that depends on the number of options already parsed via `getopt_long()`. Since the `&option_index` is valued only for long options, I decided to use the `optind` global value, see getopt_long(3). This value is initialized with the "next thing" to seek on the command line, i.e., the next index on `argv`. In the case the command accepts an optional database name, the database value is automatically set to '*' (all databases) in case the database name is not found on the command line. Therefore: pgagroal-cli flush idle is equivalent to pgagroal-cli flush idle '*' On the other hand, commands that require a server name get the value automatically set to "\0" (an invalid server name) in order to "block" other pieces of code. Moroever, if the server has not been specified, the command is automatically set to "unknown" so that the help screen is shown. The `pgagroal-cli` has a set of `pgagroal_log_trace()` calls whenever a command is "parsed", so that it is possible to quickly follow the command line parsing. Also, since the `pgagroal-cli` exists if no command line arguments have been specified, the safety check aboutt `argc > 0` around the command line parsing have been removed. In the case the user specified an unknown command, she is warned on stdout before printing the `usage()` help screen. Deprecated commands are notified to the user via a warning message, printed on stderr, that provides some hints about the correct usage of the new command. The warning about deprecated commands is shown only if the currently running version of the software is greater than the version the command has been deprecated onto. In particular these commands have been deprecated since 1.6. This commit also introduces the command refactoring for `pgagroal-admin` in a way similar to the work done for `pgagroal-cli`. New commands are available: - user <what> with <what> being <add>, <del>, <edit>, <ls>. Updated: - documentation - shell completions - help screens - examples Close agroal#290 agroal#253

…groal-admin` Now `pgagroal-cli` has a set of "logically" grouped commands and subcommands. For example, all the commands related to shutting down the pooler are under the `shutdown` command, that can operate with subcommands like `gracefully`, `immediate` or `cancel`. In order to provide this capability, new functions have been introduced as utilities: - `parse_command()` accepts the command line and seek for a command, possibly its subcommand, and an optional "value" (often the database or server name). - `parse_command_simple()` is a wrapper around the above `parse_command` that shorten the function call line because it does not require to specify the key and the value (and their defaults). - `parse_deprecated_command()` does pretty much the same thing but against the old command. Thanks to this, old commands can still work and the user will be warned about their deprecation, but the interface of `pgagroal-cli` is not broken. All the above functions require to know the offset at which start seeking for a command, and that depends on the number of options already parsed via `getopt_long()`. Since the `&option_index` is valued only for long options, I decided to use the `optind` global value, see getopt_long(3). This value is initialized with the "next thing" to seek on the command line, i.e., the next index on `argv`. In the case the command accepts an optional database name, the database value is automatically set to '*' (all databases) in case the database name is not found on the command line. Therefore: pgagroal-cli flush idle is equivalent to pgagroal-cli flush idle '*' On the other hand, commands that require a server name get the value automatically set to "\0" (an invalid server name) in order to "block" other pieces of code. Moroever, if the server has not been specified, the command is automatically set to "unknown" so that the help screen is shown. The `pgagroal-cli` has a set of `pgagroal_log_debug()` calls whenever a command is "parsed", so that it is possible to quickly follow the command line parsing. Also, since the `pgagroal-cli` exists if no command line arguments have been specified, the safety check aboutt `argc > 0` around the command line parsing has been removed. In the case the user specified an unknown command, she is warned on stdout before printing the `usage()` help screen. Deprecated commands are notified to the user via a warning message, printed on stderr, that provides some hints about the correct usage of the new command. The warning about deprecated commands is shown only if the currently running version of the software is greater than the version the command has been deprecated onto. In particular these commands have been deprecated since 1.6. This commit also introduces the command refactoring for `pgagroal-admin` in a way similar to the work done for `pgagroal-cli`. New commands are available: - user <what> with <what> being <add>, <del>, <edit>, <ls>. Updated: - documentation - shell completions - help screens - examples Close agroal#290 agroal#253

This defines how long a connection will live in seconds - Add a `max_connection_age` member to `struct configuration`. It will be checked upon returned to the pool, or during idle timeout. - Add new STATE, TRACKER, and Prometheus metric for `max_connection_age` - Add documentation for `max_connection_age` - Add a `start_time` member to `struct connection`. Its implementation is similar to `timestamp` [agroal#378] Vault Implementaion [agroal#253][agroal#209] Refactor commands in `pgagroal-cli` and `pgagroal-admin` Now `pgagroal-cli` has a set of "logically" grouped commands and subcommands. For example, all the commands related to shutting down the pooler are under the `shutdown` command, that can operate with subcommands like `gracefully`, `immediate` or `cancel`. In order to provide this capability, new functions have been introduced as utilities: - `parse_command()` accepts the command line and seek for a command, possibly its subcommand, and an optional "value" (often the database or server name). - `parse_command_simple()` is a wrapper around the above `parse_command` that shorten the function call line because it does not require to specify the key and the value (and their defaults). - `parse_deprecated_command()` does pretty much the same thing but against the old command. Thanks to this, old commands can still work and the user will be warned about their deprecation, but the interface of `pgagroal-cli` is not broken. All the above functions require to know the offset at which start seeking for a command, and that depends on the number of options already parsed via `getopt_long()`. Since the `&option_index` is valued only for long options, I decided to use the `optind` global value, see getopt_long(3). This value is initialized with the "next thing" to seek on the command line, i.e., the next index on `argv`. In the case the command accepts an optional database name, the database value is automatically set to '*' (all databases) in case the database name is not found on the command line. Therefore: pgagroal-cli flush idle is equivalent to pgagroal-cli flush idle '*' On the other hand, commands that require a server name get the value automatically set to "\0" (an invalid server name) in order to "block" other pieces of code. Moroever, if the server has not been specified, the command is automatically set to "unknown" so that the help screen is shown. The `pgagroal-cli` has a set of `pgagroal_log_debug()` calls whenever a command is "parsed", so that it is possible to quickly follow the command line parsing. Also, since the `pgagroal-cli` exists if no command line arguments have been specified, the safety check aboutt `argc > 0` around the command line parsing has been removed. In the case the user specified an unknown command, she is warned on stdout before printing the `usage()` help screen. Deprecated commands are notified to the user via a warning message, printed on stderr, that provides some hints about the correct usage of the new command. The warning about deprecated commands is shown only if the currently running version of the software is greater than the version the command has been deprecated onto. In particular these commands have been deprecated since 1.6. This commit also introduces the command refactoring for `pgagroal-admin` in a way similar to the work done for `pgagroal-cli`. New commands are available: - user <what> with <what> being <add>, <del>, <edit>, <ls>. Updated: - documentation - shell completions - help screens - examples Close agroal#290 agroal#253 [agroal#381] Changes to `pgagroal-cli` commands This commit changes two commands in `pgagroal-cli`. The `is-alive` command is deprecated by means of the `ping` command. Documentation has been modified accordingly. The `details` command is now deprecated by the `status details` one. To achieve this, the `status details` is parsed _before_ the `status` one (that has not changed at all). In order to better reflect this change, the internal constant `ACTION_DETAILS` has been renamed to `ACTION_STATUS_DETAIL`. Documentation updated accordingly. Shell completions updated accordingly. Close agroal#381 [agroal#378] Vault Implementation

fluca1978 added the feature New feature label May 30, 2022

fluca1978 mentioned this issue Jun 20, 2022

pgagroal-cli incompatible (or incomplete) options #278

Closed

This was referenced Jul 25, 2022

[#253] Refactor commands in pgagroal-cli #289

Merged

Refactor pgagroal-admin commands #290

Closed

Command completion for main shells #291

Closed

Make the version number comparable #292

Closed

fluca1978 mentioned this issue Jul 28, 2022

[#292] Provide application version as a numeric value too #293

Closed

fluca1978 mentioned this issue Jul 28, 2022

[#291] Make the version number comparable #294

Merged

jesperpedersen closed this as completed in #289 Oct 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor command names in `pgagroal-cli` #253

Refactor command names in `pgagroal-cli` #253

fluca1978 commented May 30, 2022

fluca1978 commented May 31, 2022

jesperpedersen commented May 31, 2022

fluca1978 commented May 31, 2022

jesperpedersen commented May 31, 2022

fluca1978 commented Jun 1, 2022 •

edited

Loading

jesperpedersen commented Jun 1, 2022

Refactor command names in pgagroal-cli #253

Refactor command names in pgagroal-cli #253

Comments

fluca1978 commented May 30, 2022

fluca1978 commented May 31, 2022

jesperpedersen commented May 31, 2022

fluca1978 commented May 31, 2022

jesperpedersen commented May 31, 2022

fluca1978 commented Jun 1, 2022 • edited Loading

jesperpedersen commented Jun 1, 2022

Refactor command names in `pgagroal-cli` #253

Refactor command names in `pgagroal-cli` #253

fluca1978 commented Jun 1, 2022 •

edited

Loading