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