Re: [HACKERS] Cluster name in ps output
On Fri, Jul 4, 2014 at 10:33 AM, Tom Lane t...@sss.pgh.pa.us wrote: Vik Fearing vik.fear...@dalibo.com writes: You mean that if synchronous_standby_names is an empty it automatically should be set to cluster_name? No, I mean that synchronous_standby_names should look at cluster_name first, and if it's not set then unfortunately look at application_name for backward compatibility. I think you're failing to explain yourself very well. What you really mean is that we should use cluster_name not application_name to determine the name that a standby server reports to the master. There's something to that, perhaps, but I think that the mechanism we use for doing the reporting involves the application_name parameter passed through libpq. It might be a bit painful to change that, and I'm not entirely sure it'd be less confusing. I actually prefer it the way it is now, I think. Arguably, application_name is not quite the right thing for identifying a synchronous standby, because it's intended to answer the question What *class* of connection is this? rather than Which *precise* connection is this?. But I don't think there's anything especially wrong with having a class that has only 1 member when synchronous replication is in use; indeed, in some ways it seems quite natural. That connection is after all unique. OTOH, AIUI, cluster_name is all about what shows up in the ps output, and is intended to distinguish multiple clusters running on the same server. If you're not doing that, you likely want cluster_name to be empty, but you might still want to use synchronous replication. If you are doing it, you may well want to set it to a value that distinguishes the server only from others running on the same box, like maybe the version number or port -- whereas for matching against synchronous_standby_names, we need a value that's meaningful across all related servers, but not necessarily distinguishing from other stuff on the same server. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Tom Lane wrote:
Also, -1 for adding another log_line_prefix escape. If you're routing
multiple clusters logging to the same place (which is already a bit
unlikely IMO), you can put distinguishing strings in log_line_prefix
already. And it's not like we've got an infinite supply of letters
for those escapes.
FWIW if we find ourselves short on log_line_prefix letters, we can
always invent more verbose notation -- for instance we could have escape
for each GUC var such as %{config:port} for the port number, and so on.
Probably this will be mostly useless for most GUC params, but it might
come in handy for a few of them. And of course we could have more
prefixes apart from config:.
Not that this affects the current patch in any way.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On Wed, Jul 2, 2014 at 3:45 AM, Vik Fearing vik.fear...@dalibo.com wrote: On 06/29/2014 02:25 PM, Andres Freund wrote: On 2014-06-29 11:11:14 +0100, Thomas Munro wrote: On 29 June 2014 10:55, Andres Freund and...@2ndquadrant.com wrote: So, I'd looked at it with an eye towards committing it and found some more things. I've now * added the restriction that the cluster name can only be ASCII. It's shown from several clusters with differing encodings, and it's shown in process titles, so that seems wise. * moved everything to the LOGGING_WHAT category * added minimal documention to monitoring.sgml * expanded the config.sgml entry to mention the restriction on the name. * Changed the GUCs short description Thank you. I also think, but haven't done so, we should add a double colon after the cluster name, so it's not: postgres: server1 stats collector process but postgres: server1: stats collector process +1 Committed with the above changes. Thanks for the contribution! Is there a reason for not using this in synchronous_standby_names, perhaps falling back to application_name if not set? You mean that if synchronous_standby_names is an empty it automatically should be set to cluster_name? Or, you mean that if application_name is not set in primary_conninfo the standby should automatically use its cluster_name as application_name in primary_conninfo? I'm afraid that those may cause the trouble such as that standby is unexpectedly treated as synchronous one even though a user want to set up it as asynchronous one by emptying synchronous_standby_names in the master. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 07/04/2014 08:50 AM, Fujii Masao wrote: On Wed, Jul 2, 2014 at 3:45 AM, Vik Fearing vik.fear...@dalibo.com wrote: Is there a reason for not using this in synchronous_standby_names, perhaps falling back to application_name if not set? You mean that if synchronous_standby_names is an empty it automatically should be set to cluster_name? Or, you mean that if application_name is not set in primary_conninfo the standby should automatically use its cluster_name as application_name in primary_conninfo? I'm afraid that those may cause the trouble such as that standby is unexpectedly treated as synchronous one even though a user want to set up it as asynchronous one by emptying synchronous_standby_names in the master. No, I mean that synchronous_standby_names should look at cluster_name first, and if it's not set then unfortunately look at application_name for backward compatibility. Using application_name for this always seems like a hack to me, and cluster_name is a much better fit. We should have created cluster_name back when we created synchronous_standby_names. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Vik Fearing vik.fear...@dalibo.com writes: You mean that if synchronous_standby_names is an empty it automatically should be set to cluster_name? No, I mean that synchronous_standby_names should look at cluster_name first, and if it's not set then unfortunately look at application_name for backward compatibility. I think you're failing to explain yourself very well. What you really mean is that we should use cluster_name not application_name to determine the name that a standby server reports to the master. There's something to that, perhaps, but I think that the mechanism we use for doing the reporting involves the application_name parameter passed through libpq. It might be a bit painful to change that, and I'm not entirely sure it'd be less confusing. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 06/29/2014 02:25 PM, Andres Freund wrote: On 2014-06-29 11:11:14 +0100, Thomas Munro wrote: On 29 June 2014 10:55, Andres Freund and...@2ndquadrant.com wrote: So, I'd looked at it with an eye towards committing it and found some more things. I've now * added the restriction that the cluster name can only be ASCII. It's shown from several clusters with differing encodings, and it's shown in process titles, so that seems wise. * moved everything to the LOGGING_WHAT category * added minimal documention to monitoring.sgml * expanded the config.sgml entry to mention the restriction on the name. * Changed the GUCs short description Thank you. I also think, but haven't done so, we should add a double colon after the cluster name, so it's not: postgres: server1 stats collector process but postgres: server1: stats collector process +1 Committed with the above changes. Thanks for the contribution! Is there a reason for not using this in synchronous_standby_names, perhaps falling back to application_name if not set? -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Abjit, all: If we're adding log_line_prefix support for cluster_name (something I think is a good idea), we need to also add it to CSVLOG. So, where do we put it in CSVLog? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 2014-06-30 12:10:53 -0700, Josh Berkus wrote: Abjit, all: If we're adding log_line_prefix support for cluster_name (something I think is a good idea), we need to also add it to CSVLOG. So, where do we put it in CSVLog? That was shot down (unfortunately imo), and I don't think anybody actively working on it. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Hi, On 2014-06-28 15:08:45 +0200, Andres Freund wrote: Otherwise it looks good to me. So, I'd looked at it with an eye towards committing it and found some more things. I've now * added the restriction that the cluster name can only be ASCII. It's shown from several clusters with differing encodings, and it's shown in process titles, so that seems wise. * moved everything to the LOGGING_WHAT category * added minimal documention to monitoring.sgml * expanded the config.sgml entry to mention the restriction on the name. * Changed the GUCs short description I'll leave the patch sit a while before actually committing it. I also think, but haven't done so, we should add a double colon after the cluster name, so it's not: postgres: server1 stats collector process but postgres: server1: stats collector process Comments? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services From cdef775befc2a770e3ca29c434ae42666375358b Mon Sep 17 00:00:00 2001 From: Andres Freund and...@anarazel.de Date: Sun, 29 Jun 2014 11:40:53 +0200 Subject: [PATCH] Add cluster_name GUC which will be included in process titles if set. When running several postgres clusters on one OS instance it's often inconveniently hard to identify which postgres process belongs to which instance. Add the cluster_name GUC which will be included as part of the process titles if set. With that processes can more easily identified using tools like 'ps'. Thomas Munro, with some adjustments by me and review by a host of people. --- doc/src/sgml/config.sgml | 23 ++ doc/src/sgml/monitoring.sgml | 16 +++ src/backend/utils/misc/guc.c | 28 +++ src/backend/utils/misc/postgresql.conf.sample | 3 ++- src/backend/utils/misc/ps_status.c| 22 +++-- src/include/utils/guc.h | 1 + 6 files changed, 86 insertions(+), 7 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index e3d1c62..11d552e 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -4131,6 +4131,29 @@ local0.*/var/log/postgresql /listitem /varlistentry + varlistentry id=guc-cluster-name xreflabel=cluster_name + termvarnamecluster_name/varname (typestring/type)/term + indexterm + primaryvarnamecluster_name/ configuration parameter/primary + /indexterm + listitem + para +Sets the cluster name that appears in the process title for all +processes in this cluster. The name can be any string of less than +symbolNAMEDATALEN/ characters (64 characters in a standard +build). Only printable ASCII characters may be used in the +varnameapplication_name/varname value. Other characters will be +replaced with question marks (literal?/literal). No name is shown +if this parameter is set to the empty string literal''/ (which is +the default). This parameter can only be set at server start. + /para + para +The process title is typically viewed using programs like +applicationps/ or, on Windows, applicationProcess Explorer/. + /para + /listitem + /varlistentry + varlistentry termvarnamedebug_print_parse/varname (typeboolean/type) indexterm diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml index 69d99e7..88f22a1 100644 --- a/doc/src/sgml/monitoring.sgml +++ b/doc/src/sgml/monitoring.sgml @@ -92,6 +92,22 @@ postgres: replaceableuser/ replaceabledatabase/ replaceablehost/ re /para para + If xref linkend=guc-cluster-name has been configured the + cluster name will also be show in commandps/ output: +screen +$ psql -c 'SHOW cluster_name' + cluster_name +-- + server1 +(1 row) + +$ ps aux|grep server1 +postgres 27093 0.0 0.0 30096 2752 ?Ss 11:34 0:00 postgres: server1 logger process +... +/screen + /para + + para If you have turned off xref linkend=guc-update-process-title then the activity indicator is not updated; the process title is set only once when a new process is launched. On some platforms this saves a measurable diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c index 6902c23..3a31a75 100644 --- a/src/backend/utils/misc/guc.c +++ b/src/backend/utils/misc/guc.c @@ -198,6 +198,7 @@ static void assign_effective_io_concurrency(int newval, void *extra); static void assign_pgstat_temp_directory(const char *newval, void *extra); static bool check_application_name(char **newval, void **extra, GucSource source); static void assign_application_name(const char *newval, void *extra); +static bool check_cluster_name(char **newval, void **extra, GucSource source); static const char *show_unix_socket_permissions(void); static const char
Re: [HACKERS] Cluster name in ps output
On 29 June 2014 10:55, Andres Freund and...@2ndquadrant.com wrote: So, I'd looked at it with an eye towards committing it and found some more things. I've now * added the restriction that the cluster name can only be ASCII. It's shown from several clusters with differing encodings, and it's shown in process titles, so that seems wise. * moved everything to the LOGGING_WHAT category * added minimal documention to monitoring.sgml * expanded the config.sgml entry to mention the restriction on the name. * Changed the GUCs short description Thank you. I also think, but haven't done so, we should add a double colon after the cluster name, so it's not: postgres: server1 stats collector process but postgres: server1: stats collector process +1 Best regards, Thomas Munro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 2014-06-29 11:11:14 +0100, Thomas Munro wrote: On 29 June 2014 10:55, Andres Freund and...@2ndquadrant.com wrote: So, I'd looked at it with an eye towards committing it and found some more things. I've now * added the restriction that the cluster name can only be ASCII. It's shown from several clusters with differing encodings, and it's shown in process titles, so that seems wise. * moved everything to the LOGGING_WHAT category * added minimal documention to monitoring.sgml * expanded the config.sgml entry to mention the restriction on the name. * Changed the GUCs short description Thank you. I also think, but haven't done so, we should add a double colon after the cluster name, so it's not: postgres: server1 stats collector process but postgres: server1: stats collector process +1 Committed with the above changes. Thanks for the contribution! Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On Sun, Jun 29, 2014 at 9:25 PM, Andres Freund and...@2ndquadrant.com wrote: On 2014-06-29 11:11:14 +0100, Thomas Munro wrote: On 29 June 2014 10:55, Andres Freund and...@2ndquadrant.com wrote: So, I'd looked at it with an eye towards committing it and found some more things. I've now * added the restriction that the cluster name can only be ASCII. It's shown from several clusters with differing encodings, and it's shown in process titles, so that seems wise. * moved everything to the LOGGING_WHAT category * added minimal documention to monitoring.sgml * expanded the config.sgml entry to mention the restriction on the name. * Changed the GUCs short description Thank you. I also think, but haven't done so, we should add a double colon after the cluster name, so it's not: postgres: server1 stats collector process but postgres: server1: stats collector process +1 Committed with the above changes. Thanks for the contribution! +build). Only printable ASCII characters may be used in the +varnameapplication_name/varname value. Other characters will be application_name should be cluster_name? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Hi,
On 2014-06-26 23:03:24 +0100, Thomas Munro wrote:
+ {cluster_name, PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ gettext_noop(Sets the name of the cluster that appears
in 'ps' output.),
+ NULL,
+ GUC_IS_NAME
+ },
+ cluster_name,
+ ,
+ NULL, NULL, NULL
+ },
+
In my opinion this should rather be LOGGING or LOGGING_WHAT rather than
CONN_AUTH_SETTINGS. I don't really see what place it it in the latter
category?
Possibly you've copied it from bonjour? But that's in the category
because it's used to advertises connections which isn't the case for
cluster_name.
I also don't see a reason for it to be marked as GUC_IS_NAME? That's
about truncating it so it fits into a sql identifer. Not that it should
ever play a role...
+#cluster_name = '' # visible in ps output if set
+ # (change requires restart)
Not sure if referring to ps is the best thing here. Maybe 'visible in
the processes name if set'? Same for the GUC's description string.
Otherwise it looks good to me.
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
At 2014-06-25 16:13:19 +0900, masao.fu...@gmail.com wrote: Categorizing this parameter to CONN_AUTH_SETTINGS looks strange to me Oh yes. Sorry, I meant to respond to this point in my original review, but forgot. Yes, CONN_AUTH_SETTINGS is just weird. But I couldn't find an obviously better answer either. LOGGING_WHAT would work with log_line_prefix support, but I don't think there was as much support for that version of this patch. I personally don't have a strong opinion about whether it's worth adding an escape. STATS_COLLECTOR also looks a bit strange not only for cluster_name but also update_process_title, though... True. Is UNGROUPED the only answer? monitoring.sgml explains PS display. Isn't it better to update monitoring.sgml so that it refers to cluster_name? Good point. -- Abhijit -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On Thu, Jun 26, 2014 at 4:05 PM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: At 2014-06-25 16:13:19 +0900, masao.fu...@gmail.com wrote: Categorizing this parameter to CONN_AUTH_SETTINGS looks strange to me Oh yes. Sorry, I meant to respond to this point in my original review, but forgot. Yes, CONN_AUTH_SETTINGS is just weird. But I couldn't find an obviously better answer either. LOGGING_WHAT would work with log_line_prefix support, but I don't think there was as much support for that version of this patch. I personally don't have a strong opinion about whether it's worth adding an escape. STATS_COLLECTOR also looks a bit strange not only for cluster_name but also update_process_title, though... True. Is UNGROUPED the only answer? Or create new group like REPORTING_WHAT? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 25 June 2014 08:13, Fujii Masao masao.fu...@gmail.com wrote:
On Wed, Jun 25, 2014 at 1:29 PM, Abhijit Menon-Sen a...@2ndquadrant.com
wrote:
The patch looks OK, and works as advertised (I tested on Linux). If we
want the feature (I like it), this patch is a good enough way to get it.
I got the following compiler warning.
guc.c:3099: warning: initialization from incompatible pointer type
Thank you both for your reviews. Please find attached an updated v5
patch (based on Abhijit's rebased and improved version) which fixes
the compiler warning. (The previous unusual and possibly incorrect
static initialization with a string literal was intended to make sure
logging would work before the GUC machinery has finished setting up
default values, but that no longer applies.)
Regards,
Thomas Munro
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e3d1c62..e9a0e10 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -695,6 +695,23 @@ include 'filename'
/listitem
/varlistentry
+ varlistentry id=guc-cluster-name xreflabel=cluster_name
+ termvarnamecluster_name/varname (typestring/type)/term
+ indexterm
+ primaryvarnamecluster_name/ configuration parameter/primary
+ /indexterm
+ listitem
+ para
+Sets the cluster name that appears in the process title for all
+processes in this cluster. No name is shown if this parameter is set
+to the empty string literal''/ (which is the default). The
+process title is typically viewed by the commandps/ command, or in
+Windows by using the applicationProcess Explorer/.
+This parameter can only be set at server start.
+ /para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-tcp-keepalives-idle xreflabel=tcp_keepalives_idle
termvarnametcp_keepalives_idle/varname (typeinteger/type)
indexterm
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6902c23..5578d44 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -443,6 +443,7 @@ int temp_file_limit = -1;
int num_temp_buffers = 1024;
+char *cluster_name;
char *data_directory;
char *ConfigFileName;
char *HbaFileName;
@@ -3090,6 +3091,17 @@ static struct config_string ConfigureNamesString[] =
},
{
+ {cluster_name, PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ gettext_noop(Sets the name of the cluster that appears in 'ps' output.),
+ NULL,
+ GUC_IS_NAME
+ },
+ cluster_name,
+ ,
+ NULL, NULL, NULL
+ },
+
+ {
/*
* Can't be set by ALTER SYSTEM as it can lead to recursive definition
* of data_directory.
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index d109394..e4e0411 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -74,6 +74,8 @@
# (change requires restart)
#bonjour_name = '' # defaults to the computer name
# (change requires restart)
+#cluster_name = '' # visible in ps output if set
+ # (change requires restart)
# - Security and Authentication -
diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
index 3aeceae..471d890 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);
set_ps_display(initial_str, true);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 1493d2c..0a729c1 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -224,6 +224,7 @@ extern int temp_file_limit;
extern int num_temp_buffers;
+extern char *cluster_name;
extern char *data_directory;
extern char *ConfigFileName;
extern char *HbaFileName;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
* cluster_name moved to config group CONN_AUTH_SETTINGS, on the basis that it's similar to bonjour_name, but it isn't really... open to suggestions for a better config_group! Categorizing this parameter to CONN_AUTH_SETTINGS looks strange to me because it's not directly related to connection and authentication. LOGGING_WHAT is better if we allow log_line_prefix to include the cluster_name. Or STATS_COLLECTOR which update_process_title is categorized to? STATS_COLLECTOR also looks a bit strange not only for cluster_name but also update_process_title, though... On Wed, Jun 25, 2014 at 1:29 PM, Abhijit Menon-Sen a...@2ndquadrant.com wrote: The patch looks OK, and works as advertised (I tested on Linux). If we want the feature (I like it), this patch is a good enough way to get it. I got the following compiler warning. guc.c:3099: warning: initialization from incompatible pointer type monitoring.sgml explains PS display. Isn't it better to update monitoring.sgml so that it refers to cluster_name? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Hi.
I reviewed the version of this patch without log_line_prefix support,
since that seemed to be generally acceptable in followup discussion.
The patch didn't apply any more because of some changes to guc.c, but it
was trivial to regenerate (fixed patch attached).
diff --git a/src/backend/utils/misc/postgresql.conf.sample
b/src/backend/utils/misc/postgresql.conf.sample
index 70e5a51..84ae5f3 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -74,6 +74,8 @@
# (change requires restart)
#bonjour_name = '' # defaults to the computer name
# (change requires restart)
+#cluster_name = '' # defaults to the computer name
+ # (change requires restart)
Cut-and-paste error (there's no default). Also fixed in the attached
patch.
The patch looks OK, and works as advertised (I tested on Linux). If we
want the feature (I like it), this patch is a good enough way to get it.
I'm marking it ready for committer.
-- Abhijit
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 09e6dfc..2426dfe 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -695,6 +695,23 @@ include 'filename'
/listitem
/varlistentry
+ varlistentry id=guc-cluster-name xreflabel=cluster_name
+ termvarnamecluster_name/varname (typestring/type)/term
+ indexterm
+ primaryvarnamecluster_name/ configuration parameter/primary
+ /indexterm
+ listitem
+ para
+Sets the cluster name that appears in the process title for all
+processes in this cluster. No name is shown if this parameter is set
+to the empty string literal''/ (which is the default). The
+process title is typically viewed by the commandps/ command, or in
+Windows by using the applicationProcess Explorer/.
+This parameter can only be set at server start.
+ /para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-tcp-keepalives-idle xreflabel=tcp_keepalives_idle
termvarnametcp_keepalives_idle/varname (typeinteger/type)
indexterm
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 6902c23..c9dbb4c 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -443,6 +443,7 @@ int temp_file_limit = -1;
int num_temp_buffers = 1024;
+const char *cluster_name = ;
char *data_directory;
char *ConfigFileName;
char *HbaFileName;
@@ -3090,6 +3091,17 @@ static struct config_string ConfigureNamesString[] =
},
{
+ {cluster_name, PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ gettext_noop(Sets the name of the cluster that appears in 'ps' output.),
+ NULL,
+ GUC_IS_NAME
+ },
+ cluster_name,
+ ,
+ NULL, NULL, NULL
+ },
+
+ {
/*
* Can't be set by ALTER SYSTEM as it can lead to recursive definition
* of data_directory.
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index d109394..e4e0411 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -74,6 +74,8 @@
# (change requires restart)
#bonjour_name = '' # defaults to the computer name
# (change requires restart)
+#cluster_name = '' # visible in ps output if set
+ # (change requires restart)
# - Security and Authentication -
diff --git a/src/backend/utils/misc/ps_status.c b/src/backend/utils/misc/ps_status.c
index 3aeceae..471d890 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);
set_ps_display(initial_str, true);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 1493d2c..d3cdf68 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -224,6 +224,7 @@ extern int temp_file_limit;
extern int num_temp_buffers;
[HACKERS] Cluster name in ps output
Hi
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. Obviously it's easy enough to find out (for example
by looking at a tree view in htop/ps that shows the -D parameter
of the parent postmaster), but I thought it could be useful to be
able to give a name to the cluster and include it in the ps
output. Here is a strawman patch that does that.
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
I would be grateful for any feedback. Thanks,
Thomas
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);
set_ps_display(initial_str, true);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index be68f35..639288a 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -223,6 +223,7 @@ extern int temp_file_limit;
extern int num_temp_buffers;
+extern char *cluster_name;
extern char *data_directory;
extern char *ConfigFileName;
extern char *HbaFileName;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Hi,
On 2014-05-05 10:00:34 +, 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
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 5 May 2014 10:10, Andres Freund and...@2ndquadrant.com wrote:
Hi,
On 2014-05-05 10:00:34 +, 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.
Thanks! (Do you have a write up/diff somewhere showing the local
modifications you're running?)
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 ;)
Hah -- I agree, but on systems using setproctitle, the program name and :
are provided already, so the end result would have to be different on
those systems and I figured it should be the same everywhere if possible.
(BTW I also tried to tidy up the way that is handled, so that instead of a
different snprintf statement being selected by the preprocessor, a macro
PROGRAM_NAME_PREFIX is defined to be empty on those systems).
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?
Good idea, I will look into that.
+ 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?
Hmm -- I thought the GUC machinery would make sure cluster_name either
pointed to the default I provided, an empty string, or a string read from
the configuration file. Perhaps I need to go and read up on how GUCs work.
Thomas Munro
Re: [HACKERS] Cluster name in ps output
On 05/05/2014 06:00 PM, Thomas Munro wrote: Hi 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. Obviously it's easy enough to find out (for example by looking at a tree view in htop/ps that shows the -D parameter of the parent postmaster), but I thought it could be useful to be able to give a name to the cluster and include it in the ps output. Here is a strawman patch that does that. 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 I'd find something like that very useful. I'd even be tempted to tack on the port number, though that might be overkill. Perhaps just use the port number instead of a new cluster name? Most people won't use/set something like a cluster name, though I guess distros that use pg_wrapper would probably auto-set it via pg_wrapper's configuration. Personally I'd find: postgres[5433]: checkpointer process at least as useful. The only time that's not unique is in a BSD jail or lxc container, and in those cases IIRC ps can show you the jail/container anyway. Showing the port would help new-ish users a lot; many seem to be very confused by which PostgreSQL instance(s) they're connecting to and which are running. Especially on Mac OS X, where people often land up with Apple's PostgreSQL, Homebrew, Postgres.app, and who knows what else running at the same time. -- Craig Ringer http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 2014-05-05 10:49:19 +, Thomas Munro wrote: On 5 May 2014 10:10, Andres Freund and...@2ndquadrant.com wrote: Hi, On 2014-05-05 10:00:34 +, 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. Thanks! (Do you have a write up/diff somewhere showing the local modifications you're running?) I've just hacked in the port into the ps display since that was sufficient for my case. 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 ;) Hah -- I agree, but on systems using setproctitle, the program name and : are provided already, so the end result would have to be different on those systems and I figured it should be the same everywhere if possible. Fair point. Aren't you potentially dereferencing a NULL pointer here? Hmm -- I thought the GUC machinery would make sure cluster_name either pointed to the default I provided, an empty string, or a string read from the configuration file. Perhaps I need to go and read up on how GUCs work. That's true - but I am not sure you can guarantee it's only called after the GUC machinery has started up. Particularly on windows. I guess just initializing the global variable to should do the trick. Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-05-05 10:00:34 +, 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.
Ditto.
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?
I'm not entirely sure that I see the point of having it in
log_line_prefix- each cluster logs to its own log file which includes
the cluster name (at least on Debian/Ubuntu and friends). The only use
case I can imagine here would be for syslog, but you could just *set*
the cluster name in the log_line_prefix, as it'd be (by definition..)
configurable per cluster.
I'd much rather see other things added as log_line_prefix options.. An
interesting thought that just occured to me would be to allow any GUC to
be added to log_line_prefix using some kind of extended % support (eg:
'%{my_guc_here}' or something...). Would also be useful for extensions
which add GUCs then? Not sure about specifics, but does seem like an
interesting idea.
Oh, and I know people will shoot me for bringing it up again, but I'd
still like to see the CSV format be configurable ala log_line_prefix,
and the same for any kind of logging (or auditing) to a table which we
might eventually support. Yes, we need to work out how to do file
changes when it's updated and stick a header on each new file with the
columns included.
Thanks,
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] Cluster name in ps output
Craig, * Craig Ringer (cr...@2ndquadrant.com) wrote: postgres[5433]: checkpointer process at least as useful. The only time that's not unique is in a BSD jail or lxc container, and in those cases IIRC ps can show you the jail/container anyway. Uhh, that's not at all true. You can trivially have multiple IPs on a box w/o jails or containers (aliased interfaces) and then run PG on the default port- which I find to be *far* more convenient than having the same IP and a bunch of different ports. What you *can't* have is two clusters with the same name and same major version, at least on the Debian/Ubuntu distributions, and as such, I would argue to also include the major version rather than include the port, which you could get from pg_lsclusters. Showing the port would help new-ish users a lot; many seem to be very confused by which PostgreSQL instance(s) they're connecting to and which are running. Especially on Mac OS X, where people often land up with Apple's PostgreSQL, Homebrew, Postgres.app, and who knows what else running at the same time. I'm far from convinced that showing the port in the ps output will really help these users.. Not to mention that you can get that from 'netstat -anp' anyway. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Cluster name in ps output
On 2014-05-05 07:58:30 -0400, Stephen Frost wrote: 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? I'm not entirely sure that I see the point of having it in log_line_prefix- each cluster logs to its own log file which includes the cluster name (at least on Debian/Ubuntu and friends). The only use case I can imagine here would be for syslog, but you could just *set* the cluster name in the log_line_prefix, as it'd be (by definition..) configurable per cluster. So I've to configure it in multiple locations? I don't see the point. I usually try to configure as much in common/template config files that are included. Everything that doesn't have to be overwritten is good. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
* Andres Freund (and...@2ndquadrant.com) wrote:
On 2014-05-05 07:58:30 -0400, Stephen Frost wrote:
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?
I'm not entirely sure that I see the point of having it in
log_line_prefix- each cluster logs to its own log file which includes
the cluster name (at least on Debian/Ubuntu and friends). The only use
case I can imagine here would be for syslog, but you could just *set*
the cluster name in the log_line_prefix, as it'd be (by definition..)
configurable per cluster.
So I've to configure it in multiple locations? I don't see the point. I
usually try to configure as much in common/template config files that
are included. Everything that doesn't have to be overwritten is good.
I see the point- we've already got quite a few %whatever's and adding
mostly useless ones may just add confusion and unnecessary complexity.
If you've already got your postgresql.conf templated through
puppet/chef/salt/whatever then having to add another '%{ CLUSTER_NAME }%'
or whatever shouldn't be a terribly difficult thing.
Thanks,
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] Cluster name in ps output
On 5 May 2014 10:49, Thomas Munro mu...@ip9.org wrote:
On 5 May 2014 10:10, Andres Freund and...@2ndquadrant.com wrote:
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?
Good idea, I will look into that.
See v2 patch attached which lets you use %C for cluster name in the log
prefix.
Maybe it would be overkill, but seeing the various responses about which
information belongs in the ps string, I guess we could also use a format
string with %blah fields for that. Then the Debian/Ubuntu package users
who tend to think of the major version + name as the complete cluster
identifier could use [%V/%C] ... to get postgres: [9.3/main] ..., and
others could throw in a %P to see a port number.
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 977fc66..0adfee7 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2345,6 +2345,12 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
else
appendStringInfo(buf, %lx.%x, (long) (MyStartTime), MyProcPid);
break;
+ case 'C':
+if (padding != 0)
+ appendStringInfo(buf, %*s, padding, cluster_name);
+else
+ appendStringInfoString(buf, cluster_name);
+break;
case 'p':
if (padding != 0)
appendStringInfo(buf, %*d, padding, MyProcPid);
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);
set_ps_display(initial_str, true);
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index be68f35..639288a 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -223,6 +223,7 @@ extern int temp_file_limit;
extern int num_temp_buffers;
+extern char *cluster_name;
extern char *data_directory;
extern char *ConfigFileName;
extern char *HbaFileName;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
Andres Freund and...@2ndquadrant.com writes: On 2014-05-05 10:49:19 +, Thomas Munro wrote: Hah -- I agree, but on systems using setproctitle, the program name and : are provided already, so the end result would have to be different on those systems and I figured it should be the same everywhere if possible. Fair point. How about dropping the brackets, and the cluster-name concept, and just doing postgres: 5432 checkpointer process Aren't you potentially dereferencing a NULL pointer here? Hmm -- I thought the GUC machinery would make sure cluster_name either pointed to the default I provided, an empty string, or a string read from the configuration file. Perhaps I need to go and read up on how GUCs work. That's true - but I am not sure you can guarantee it's only called after the GUC machinery has started up. The elog code MUST be able to work before GUC initialization is done. What do you think will happen if we fail to open postgresql.conf, for example? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
* Tom Lane (t...@sss.pgh.pa.us) wrote: How about dropping the brackets, and the cluster-name concept, and just doing postgres: 5432 checkpointer process -1 for my part, as I'd just end up with a bunch of those and no distinction between the various processes. In other words, without a cluster distinction, it's useless. Including the value of listen_addresses along w/ the port would make it useful. If we really don't want the cluster-name concept (which, personally, I like quite a bit), how about including the listen_address value if it isn't '*'? I could see that also helping users who installed from a distro and got '127.0.0.1' and don't understand why they can't connect... Of course, these are users who can use 'ps' but not 'netstat'. Not sure how big that set really is. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Cluster name in ps output
Stephen Frost sfr...@snowman.net writes: * Tom Lane (t...@sss.pgh.pa.us) wrote: How about dropping the brackets, and the cluster-name concept, and just doing postgres: 5432 checkpointer process -1 for my part, as I'd just end up with a bunch of those and no distinction between the various processes. In other words, without a cluster distinction, it's useless. Yeah, after I sent that I got to the bit about running multiple postmasters with different IP-address bindings. I agree the port number alone wouldn't be enough in that scenario. Including the value of listen_addresses along w/ the port would make it useful. If we really don't want the cluster-name concept (which, personally, I like quite a bit), how about including the listen_address value if it isn't '*'? Nah, let's do cluster name. That way, somebody who's only got one postmaster isn't suddenly going to see a lot of useless clutter, ie the user gets to decide what to add to ps output. SHOW cluster_name might be useful at the application level as well, I suspect. I still think the brackets are unnecessary though. Also, -1 for adding another log_line_prefix escape. If you're routing multiple clusters logging to the same place (which is already a bit unlikely IMO), you can put distinguishing strings in log_line_prefix already. And it's not like we've got an infinite supply of letters for those escapes. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
* Tom Lane (t...@sss.pgh.pa.us) wrote: Stephen Frost sfr...@snowman.net writes: Including the value of listen_addresses along w/ the port would make it useful. If we really don't want the cluster-name concept (which, personally, I like quite a bit), how about including the listen_address value if it isn't '*'? Nah, let's do cluster name. That way, somebody who's only got one postmaster isn't suddenly going to see a lot of useless clutter, ie the user gets to decide what to add to ps output. SHOW cluster_name might be useful at the application level as well, I suspect. Ah, yes, agreed, that could be quite useful. I still think the brackets are unnecessary though. Either way is fine for me on this. Also, -1 for adding another log_line_prefix escape. If you're routing multiple clusters logging to the same place (which is already a bit unlikely IMO), you can put distinguishing strings in log_line_prefix already. And it's not like we've got an infinite supply of letters for those escapes. Agreed. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Cluster name in ps output
On 2014-05-05 09:52:33 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: Aren't you potentially dereferencing a NULL pointer here? Hmm -- I thought the GUC machinery would make sure cluster_name either pointed to the default I provided, an empty string, or a string read from the configuration file. Perhaps I need to go and read up on how GUCs work. That's true - but I am not sure you can guarantee it's only called after the GUC machinery has started up. The elog code MUST be able to work before GUC initialization is done. What do you think will happen if we fail to open postgresql.conf, for example? But elog. shouldn't call init_ps_display(), right? Anyway, I am all for initializing it sensibly, after all it was I that suggested doing so above... Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 2014-05-05 08:03:04 -0400, Stephen Frost wrote: Craig, * Craig Ringer (cr...@2ndquadrant.com) wrote: postgres[5433]: checkpointer process at least as useful. The only time that's not unique is in a BSD jail or lxc container, and in those cases IIRC ps can show you the jail/container anyway. Uhh, that's not at all true. You can trivially have multiple IPs on a box w/o jails or containers (aliased interfaces) and then run PG on the default port- which I find to be *far* more convenient than having the same IP and a bunch of different ports. Only that you then need different socket directories. Do you really do that regularly? Anyway, I am happy having the cluster_name thingy. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 2014-05-05 10:07:46 -0400, Tom Lane wrote: Stephen Frost sfr...@snowman.net writes: Including the value of listen_addresses along w/ the port would make it useful. If we really don't want the cluster-name concept (which, personally, I like quite a bit), how about including the listen_address value if it isn't '*'? Nah, let's do cluster name. That way, somebody who's only got one postmaster isn't suddenly going to see a lot of useless clutter, ie the user gets to decide what to add to ps output. SHOW cluster_name might be useful at the application level as well, I suspect. Hm. What about unifiyng this with event_source? Not sure if it's a good idea, but it's a pretty similar thing. Also, -1 for adding another log_line_prefix escape. If you're routing multiple clusters logging to the same place (which is already a bit unlikely IMO), you can put distinguishing strings in log_line_prefix already. And it's not like we've got an infinite supply of letters for those escapes. Using syslog and including the same config file from multiple clusters isn't that uncommon. But I can live without it. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
* Andres Freund (and...@2ndquadrant.com) wrote: On 2014-05-05 08:03:04 -0400, Stephen Frost wrote: Uhh, that's not at all true. You can trivially have multiple IPs on a box w/o jails or containers (aliased interfaces) and then run PG on the default port- which I find to be *far* more convenient than having the same IP and a bunch of different ports. Only that you then need different socket directories. Do you really do that regularly? Yup. I've wished for that to be the default in Debian quite a few times, actually... Much easier to deal with firewall rules and users who are coming from pgAdmin and friends if they only have to deal with understanding what hosts they need to connect to, and not worry about the port also. Anyway, I am happy having the cluster_name thingy. Agreed. Thanks, Stpehen signature.asc Description: Digital signature
Re: [HACKERS] Cluster name in ps output
Andres Freund and...@2ndquadrant.com writes: On 2014-05-05 10:07:46 -0400, Tom Lane wrote: Also, -1 for adding another log_line_prefix escape. If you're routing multiple clusters logging to the same place (which is already a bit unlikely IMO), you can put distinguishing strings in log_line_prefix already. And it's not like we've got an infinite supply of letters for those escapes. Using syslog and including the same config file from multiple clusters isn't that uncommon. But I can live without it. So, if you are sharing a config file, how is it that you can set a per-cluster cluster_name but not a per-cluster log_line_prefix? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 2014-05-05 13:07:48 -0400, Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: On 2014-05-05 10:07:46 -0400, Tom Lane wrote: Also, -1 for adding another log_line_prefix escape. If you're routing multiple clusters logging to the same place (which is already a bit unlikely IMO), you can put distinguishing strings in log_line_prefix already. And it's not like we've got an infinite supply of letters for those escapes. Using syslog and including the same config file from multiple clusters isn't that uncommon. But I can live without it. So, if you are sharing a config file, how is it that you can set a per-cluster cluster_name but not a per-cluster log_line_prefix? Well, it's a included file. With log_line_prefix support just cluster_name has to be set per cluster. Without you need string interpolation in the config management to include cluster_name in log_line_prefix. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Cluster name in ps output
On 5 May 2014 17:22, Andres Freund and...@2ndquadrant.com wrote:
On 2014-05-05 13:07:48 -0400, Tom Lane wrote:
Andres Freund and...@2ndquadrant.com writes:
On 2014-05-05 10:07:46 -0400, Tom Lane wrote:
Also, -1 for adding another log_line_prefix escape. If you're routing
multiple clusters logging to the same place (which is already a bit
unlikely IMO), you can put distinguishing strings in log_line_prefix
already. And it's not like we've got an infinite supply of letters
for those escapes.
Using syslog and including the same config file from multiple clusters
isn't that uncommon. But I can live without it.
So, if you are sharing a config file, how is it that you can set a
per-cluster cluster_name but not a per-cluster log_line_prefix?
Well, it's a included file. With log_line_prefix support just
cluster_name has to be set per cluster. Without you need string
interpolation in the config management to include cluster_name in
log_line_prefix.
Attached as cluster-name-in-ps-v3-a.patch is a new version with
the following changes:
* the brackets removed, as suggested by Tom Lane
* static initialization of cluster_name to avoid any possibility
of an uninitialized/null pointer being used before GUC
initialization, as suggested by Andres Freund
* cluster_name moved to config group CONN_AUTH_SETTINGS, on the
basis that it's similar to bonjour_name, but it isn't
really... open to suggestions for a better config_group!
* a small amount of documentation in config.sgml (with
cluster_name under Connection Settings, which probably
isn't right either)
* sample conf file updated to show cluster_name and %C in
log_line_prefix
A shorter version without the log_line_prefix support that Tom didn't
like is attached as cluster-name-in-ps-v3-b.patch. I will try to add these
to the open commitfest, and see if there is something I can usefully
review in return.
I verified that SHOW cluster_name works as expected and you can't
change it with SET.
Thanks,
Thomas Munro
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 4a33f87..0fd414e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -681,6 +681,24 @@ include 'filename'
/listitem
/varlistentry
+ varlistentry id=guc-cluster-name xreflabel=cluster_name
+ termvarnamecluster_name/varname (typestring/type)/term
+ indexterm
+ primaryvarnamecluster_name/ configuration parameter/primary
+ /indexterm
+ listitem
+ para
+Sets the cluster name that appears in the process title for all
+processes in this cluster. No name is shown if this parameter is set
+to the empty string literal''/ (which is the default). The
+process title is typically viewed by the commandps/ command, or in
+Windows by using the applicationProcess Explorer/. The cluster
+name can also be included in the varnamelog_line_prefix/varname.
+This parameter can only be set at server start.
+ /para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-tcp-keepalives-idle xreflabel=tcp_keepalives_idle
termvarnametcp_keepalives_idle/varname (typeinteger/type)/term
indexterm
@@ -4212,6 +4230,11 @@ local0.*/var/log/postgresql
/thead
tbody
row
+ entryliteral%C/literal/entry
+ entryCluster name/entry
+ entryno/entry
+/row
+row
entryliteral%a/literal/entry
entryApplication name/entry
entryyes/entry
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 977fc66..0adfee7 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2345,6 +2345,12 @@ log_line_prefix(StringInfo buf, ErrorData *edata)
else
appendStringInfo(buf, %lx.%x, (long) (MyStartTime), MyProcPid);
break;
+ case 'C':
+if (padding != 0)
+ appendStringInfo(buf, %*s, padding, cluster_name);
+else
+ appendStringInfoString(buf, cluster_name);
+break;
case 'p':
if (padding != 0)
appendStringInfo(buf, %*d, padding, MyProcPid);
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 15020c4..71897b8 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;
+const char *cluster_name = ;
char *data_directory;
char *ConfigFileName;
char *HbaFileName;
@@ -3091,6 +3092,17 @@ static struct config_string ConfigureNamesString[] =
},
{
+ {cluster_name, PGC_POSTMASTER, CONN_AUTH_SETTINGS,
+ 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.),
