Skip to content

Commit

Permalink
[agroal#335] Disambiguate connection errors from data errors in commands
Browse files Browse the repository at this point in the history
This commit introduces a few constants to indicate if the
communication over the management socket has failed due to a
connection error or because the data received is not valid.
So far, only `config-get` returns a different value to indicate
invalid data over the socket, but other commands can take advanatge of
this too.

The `pgagroal-cli` main loop has been refactored to distinguish the
new cases, and now it prints the "connection error" message only when
the low level function reports such an error.

All the functions now return a specific value among those defined.

Close agroal#335
  • Loading branch information
fluca1978 committed Dec 13, 2022
1 parent b5516c8 commit 0179387
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 37 deletions.
85 changes: 49 additions & 36 deletions src/cli.c
Original file line number Diff line number Diff line change
Expand Up @@ -621,9 +621,18 @@ main(int argc, char** argv)

if (configuration_path != NULL)
{
if (action != ACTION_UNKNOWN && exit_code != 0)
if (action != ACTION_UNKNOWN)
{
printf("Connection error on %s\n", config->unix_socket_dir);
switch (exit_code)
{
case EXIT_STATUS_CONNECTION_ERROR:
printf("Connection error on %s\n", config->unix_socket_dir);
break;
case EXIT_STATUS_DATA_ERROR:
case EXIT_STATUS_OK:
break;
}

}
}

Expand All @@ -637,7 +646,7 @@ main(int argc, char** argv)

if (verbose)
{
warnx("%s (%d)\n", exit_code == 0 ? "Success" : "Error", exit_code);
warnx("%s (%d)\n", exit_code == EXIT_STATUS_OK ? "Success" : "Error", exit_code);
}

return exit_code;
Expand All @@ -648,65 +657,65 @@ flush(SSL* ssl, int socket, int32_t mode, char* database)
{
if (pgagroal_management_flush(ssl, socket, mode, database))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
enabledb(SSL* ssl, int socket, char* database)
{
if (pgagroal_management_enabledb(ssl, socket, database))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
disabledb(SSL* ssl, int socket, char* database)
{
if (pgagroal_management_disabledb(ssl, socket, database))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
gracefully(SSL* ssl, int socket)
{
if (pgagroal_management_gracefully(ssl, socket))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
stop(SSL* ssl, int socket)
{
if (pgagroal_management_stop(ssl, socket))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
cancel_shutdown(SSL* ssl, int socket)
{
if (pgagroal_management_cancel_shutdown(ssl, socket))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
Expand All @@ -718,7 +727,7 @@ status(SSL* ssl, int socket)
}
else
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}
}

Expand All @@ -734,7 +743,7 @@ details(SSL* ssl, int socket)
}

// if here, an error occurred
return 1;
return EXIT_STATUS_CONNECTION_ERROR;

}

Expand All @@ -747,64 +756,64 @@ isalive(SSL* ssl, int socket)
{
if (pgagroal_management_read_isalive(ssl, socket, &status))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

if (status != 1 && status != 2)
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}
}
else
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
reset(SSL* ssl, int socket)
{
if (pgagroal_management_reset(ssl, socket))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
reset_server(SSL* ssl, int socket, char* server)
{
if (pgagroal_management_reset_server(ssl, socket, server))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
switch_to(SSL* ssl, int socket, char* server)
{
if (pgagroal_management_switch_to(ssl, socket, server))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

static int
reload(SSL* ssl, int socket)
{
if (pgagroal_management_reload(ssl, socket))
{
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}

return 0;
return EXIT_STATUS_OK;
}

/**
Expand All @@ -818,7 +827,7 @@ reload(SSL* ssl, int socket)
* @param config_key the key of the configuration parameter, that is the name
* of the configuration parameter to read.
* @param verbose if true the function will print on STDOUT also the config key
* @returns 0 on success, 1 on failure
* @returns 0 on success, 1 on network failure, 2 on data failure
*/
static int
config_get(SSL* ssl, int socket, char* config_key, bool verbose)
Expand All @@ -844,9 +853,9 @@ config_get(SSL* ssl, int socket, char* config_key, bool verbose)
goto error;
}

// assume an empty response is ok,
// do not throw an error to indicate no configuration
// setting with such name as been found
// an empty response means that the
// requested configuration parameter has not been
// found, so throw an error
if (buffer && strlen(buffer))
{
if (verbose)
Expand All @@ -858,13 +867,17 @@ config_get(SSL* ssl, int socket, char* config_key, bool verbose)
printf("%s\n", buffer);
}
}
else
{
free(buffer);
return EXIT_STATUS_DATA_ERROR;
}

free(buffer);
return 0;
}

return 0;
return EXIT_STATUS_OK;

error:
return 1;
return EXIT_STATUS_CONNECTION_ERROR;
}
9 changes: 9 additions & 0 deletions src/include/pgagroal.h
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,15 @@ extern "C" {
#define UPDATE_PROCESS_TITLE_MINIMAL 2
#define UPDATE_PROCESS_TITLE_VERBOSE 3

/**
* Constants used to manage the exit code
* of a command sent over the socket in the
* management stuff, e.g., `pgagroal-cli`.
*/
#define EXIT_STATUS_OK 0
#define EXIT_STATUS_CONNECTION_ERROR 1
#define EXIT_STATUS_DATA_ERROR 2

#define likely(x) __builtin_expect (!!(x), 1)
#define unlikely(x) __builtin_expect (!!(x), 0)

Expand Down
2 changes: 1 addition & 1 deletion src/libpgagroal/management.c
Original file line number Diff line number Diff line change
Expand Up @@ -1552,7 +1552,7 @@ pgagroal_management_write_config_get(int socket, char* config_key)
if (pgagroal_write_config_value(&data[0], config_key))
{
pgagroal_log_warn("pgagroal_management_write_config_get: unknwon configuration key <%s>", config_key);
goto error;
// leave the payload empty, so a zero filled payload will be sent
}

// send the size of the payload
Expand Down

0 comments on commit 0179387

Please sign in to comment.