Re: [HACKERS] Cluster name in ps output

2014-07-07 Thread Robert Haas
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

2014-07-07 Thread Alvaro Herrera
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

2014-07-04 Thread Fujii Masao
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

2014-07-04 Thread Vik Fearing
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

2014-07-04 Thread Tom Lane
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

2014-07-01 Thread Vik Fearing
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

2014-06-30 Thread Josh Berkus
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

2014-06-30 Thread Andres Freund
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

2014-06-30 Thread Andres Freund
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

2014-06-29 Thread Thomas Munro
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

2014-06-29 Thread Andres Freund
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

2014-06-29 Thread Fujii Masao
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

2014-06-28 Thread Andres Freund
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

2014-06-26 Thread Abhijit Menon-Sen
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

2014-06-26 Thread Fujii Masao
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

2014-06-26 Thread Thomas Munro
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

2014-06-25 Thread Fujii Masao
 * 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

2014-06-24 Thread Abhijit Menon-Sen
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;
 

Re: [HACKERS] Cluster name in ps output

2014-05-05 Thread Andres Freund
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

2014-05-05 Thread Thomas Munro
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

2014-05-05 Thread Craig Ringer
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

2014-05-05 Thread Andres Freund
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

2014-05-05 Thread Stephen Frost
* 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

2014-05-05 Thread Stephen Frost
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

2014-05-05 Thread Andres Freund
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

2014-05-05 Thread Stephen Frost
* 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

2014-05-05 Thread Thomas Munro
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

2014-05-05 Thread Tom Lane
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

2014-05-05 Thread Stephen Frost
* 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

2014-05-05 Thread Tom Lane
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

2014-05-05 Thread Stephen Frost
* 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

2014-05-05 Thread Andres Freund
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

2014-05-05 Thread Andres Freund
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

2014-05-05 Thread Andres Freund
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

2014-05-05 Thread Stephen Frost
* 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

2014-05-05 Thread Tom Lane
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

2014-05-05 Thread Andres Freund
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

2014-05-05 Thread Thomas Munro
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.),