Re: Cluster name in ps output

Hi,

On 2014-05-05 10:00:34 +0000, Thomas Munro wrote:
> When running more than one cluster I often find myself looking at
> the output of 'iotop' or other tools wondering which
> cluster's "wal receiver process" or "checkpointer process" etc
> I'm seeing.

I wonder about that pretty regularly. To the point that I've a hacky
version of this locally. So +1 for me for the idea in general.

> If cluster_name is not set, it defaults to the empty string and
> the ps output is unchanged. If it's set to 'foox' the ps output
> includes that string in square brackets:
>
> postgres: [foox] checkpointer process
> postgres: [foox] writer process
> postgres: [foox] wal writer process
> postgres: [foox] autovacuum launcher process
> postgres: [foox] stats collector process
> postgres: [foox] munro foodb [local] idle

"postgres: [foox] ..." should rather be "postgres[foox]: ..." imo ;)

I guess the question is where this should be available as well. At the
very least I'd want to reference it in log_line_prefix as well?

> diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
> index 15020c4..7f7fd52 100644
> --- a/src/backend/utils/misc/guc.c
> +++ b/src/backend/utils/misc/guc.c
> @@ -449,6 +449,7 @@ int temp_file_limit = -1;
>
> int num_temp_buffers = 1024;
>
> +char *cluster_name;
> char *data_directory;
> char *ConfigFileName;
> char *HbaFileName;
> @@ -3091,6 +3092,17 @@ static struct config_string ConfigureNamesString[] =
> },
>
> {
> + {"cluster_name", PGC_POSTMASTER, CUSTOM_OPTIONS,
> + gettext_noop("Sets the name of the cluster that appears in 'ps' output."),
> + NULL,
> + GUC_IS_NAME
> + },
> + &cluster_name,
> + "",
> + NULL, NULL, NULL
> + },
> +
> + {
> {"data_directory", PGC_POSTMASTER, FILE_LOCATIONS,
> gettext_noop("Sets the server's data directory."),
> NULL,
> diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
> index 6294ca3..ead7ea4 100644
> --- a/src/backend/utils/misc/ps_status.c
> +++ b/src/backend/utils/misc/ps_status.c
> @@ -29,6 +29,7 @@
> #include "libpq/libpq.h"
> #include "miscadmin.h"
> #include "utils/ps_status.h"
> +#include "utils/guc.h"
>
> extern char **environ;
> bool update_process_title = true;
> @@ -264,15 +265,24 @@ init_ps_display(const char *username, const char *dbname,
> * apparently setproctitle() already adds a `progname:' prefix to the ps
> * line
> */
> - snprintf(ps_buffer, ps_buffer_size,
> - "%s %s %s ",
> - username, dbname, host_info);
> +#define PROGRAM_NAME_PREFIX ""
> #else
> - snprintf(ps_buffer, ps_buffer_size,
> - "postgres: %s %s %s ",
> - username, dbname, host_info);
> +#define PROGRAM_NAME_PREFIX "postgres: "
> #endif
>
> + if (*cluster_name == '\0')
> + {
> + snprintf(ps_buffer, ps_buffer_size,
> + PROGRAM_NAME_PREFIX "%s %s %s ",
> + username, dbname, host_info);
> + }
> + else
> + {
> + snprintf(ps_buffer, ps_buffer_size,
> + PROGRAM_NAME_PREFIX "[%s] %s %s %s ",
> + cluster_name, username, dbname, host_info);
> + }
> +
> ps_buffer_cur_len = ps_buffer_fixed_size = strlen(ps_buffer);

Aren't you potentially dereferencing a NULL pointer here?

Greetings,

Andres Freund