Re: [HACKERS] Logging configuration changes [REVIEW]

2009-09-17 Thread Peter Eisentraut
On ons, 2009-09-16 at 10:54 +0530, Abhijit Menon-Sen wrote:
 I can't help but think that it would be nice to report the default value
 of a parameter that is reset (i.e. parameter $x reset to default value
 $y). The first attached patch does this by calling GetConfigOption()
 after the value has been reset by set_config_option(). It also skips
 the report if the earlier value was the same as the default.

I have applied the rest, but the problem with this change is that it
logs the values without units.  I'm not sure that we want to expose
that.



-- 
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] Logging configuration changes [REVIEW]

2009-09-16 Thread Jaime Casanova
On Wed, Sep 16, 2009 at 12:24 AM, Abhijit Menon-Sen a...@toroid.org wrote:

 LOG:  received SIGHUP, reloading configuration files
 LOG:  parameter log_connections reset to default value off
 LOG:  parameter log_disconnections reset to default value off
 LOG:  Parameter max_connections cannot be changed without restarting the 
 server
 LOG:  parameter log_checkpoints changed to on


ok, maybe this is not the most brilliant observation but someone has
to say it... keep the same case in the word parameter ;)

-- 
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157

-- 
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] Logging configuration changes [REVIEW]

2009-09-16 Thread Abhijit Menon-Sen
At 2009-09-16 01:18:10 -0500, jcasa...@systemguards.com.ec wrote:

 ok, maybe this is not the most brilliant observation but someone has
 to say it... keep the same case in the word parameter ;)

Oops, thanks. Re²vised patch attached.

-- ams
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 9e9c3f7..a290601 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -260,9 +260,8 @@ ProcessConfigFile(GucContext context)
 		{
 			ereport(elevel,
 	(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
-	 errmsg(attempted change of parameter \%s\ ignored,
-			gconf-name),
-	 errdetail(This parameter cannot be changed after server start.)));
+	 errmsg(parameter \%s\ cannot be changed without restarting the server,
+			gconf-name)));
 			continue;
 		}
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2224d56..8fa9599 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4599,18 +4599,16 @@ set_config_option(const char *name, const char *value,
 if (changeVal  !is_newvalue_equal(record, value))
 	ereport(elevel,
 			(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
-	   errmsg(attempted change of parameter \%s\ ignored,
-			  name),
-			 errdetail(This parameter cannot be changed after server start.)));
+	   errmsg(parameter \%s\ cannot be changed without restarting the server,
+			  name)));
 return true;
 			}
 			if (context != PGC_POSTMASTER)
 			{
 ereport(elevel,
 		(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
-	   errmsg(attempted change of parameter \%s\ ignored,
-			  name),
-		 errdetail(This parameter cannot be changed after server start.)));
+	   errmsg(parameter \%s\ cannot be changed without restarting the server,
+			  name)));
 return false;
 			}
 			break;

-- 
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] Logging configuration changes [REVIEW]

2009-09-15 Thread Abhijit Menon-Sen
(This is my review of the small patch Peter posted on 2009-08-29. See
http://archives.postgresql.org/message-id/1251495487.18151.12.ca...@vanquo.pezone.net
for the original message.)

At 2009-08-29 00:38:07 +0300, pete...@gmx.net wrote:

 Found an easy solution; see attached patch.

Neat. The patch (a) applies to HEAD and builds correctly, (b) does what
it's supposed to, i.e. report parameters whose value has been changed or
reset to the default, and (c) seems sensible.

I can't help but think that it would be nice to report the default value
of a parameter that is reset (i.e. parameter $x reset to default value
$y). The first attached patch does this by calling GetConfigOption()
after the value has been reset by set_config_option(). It also skips
the report if the earlier value was the same as the default.

 On a related note, I suggest reverting or revising this logging change:
 http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=293c816e4aad8e760bcb4eaba0aa16da0ccd2d04

FWIW, I agree about this too.

I would suggest changing the errmsg to just Parameter \%s\ cannot be
changed without restarting the server. I have attached a second patch
to do this.

LOG:  received SIGHUP, reloading configuration files
LOG:  parameter log_connections reset to default value off
LOG:  parameter log_disconnections reset to default value off
LOG:  Parameter max_connections cannot be changed without restarting the 
server
LOG:  parameter log_checkpoints changed to on

-- ams
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 9e9c3f7..3bda73b 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -252,6 +252,8 @@ ProcessConfigFile(GucContext context)
 	{
 		struct config_generic *gconf = guc_variables[i];
 		GucStack   *stack;
+		const char *pre_value;
+		const char *post_value;
 
 		if (gconf-reset_source != PGC_S_FILE ||
 			(gconf-status  GUC_IS_IN_FILE))
@@ -280,9 +282,18 @@ ProcessConfigFile(GucContext context)
 stack-source = PGC_S_DEFAULT;
 		}
 
+		if (context == PGC_SIGHUP)
+			pre_value = pstrdup(GetConfigOption(gconf-name));
+
 		/* Now we can re-apply the wired-in default */
 		set_config_option(gconf-name, NULL, context, PGC_S_DEFAULT,
 		  GUC_ACTION_SET, true);
+
+		post_value = pstrdup(GetConfigOption(gconf-name));
+		if (context == PGC_SIGHUP  strcmp(pre_value, post_value) != 0)
+			ereport(elevel,
+	(errmsg(parameter \%s\ reset to default value \%s\,
+			gconf-name, post_value)));
 	}
 
 	/*
@@ -309,9 +320,18 @@ ProcessConfigFile(GucContext context)
 	/* If we got here all the options checked out okay, so apply them. */
 	for (item = head; item; item = item-next)
 	{
+		const char *pre_value;
+
+		if (context == PGC_SIGHUP)
+			pre_value = pstrdup(GetConfigOption(item-name));
+
 		if (set_config_option(item-name, item-value, context,
 			   	 PGC_S_FILE, GUC_ACTION_SET, true))
 		{
+			if (context == PGC_SIGHUP  strcmp(pre_value, GetConfigOption(item-name)) != 0)
+ereport(elevel,
+		(errmsg(parameter \%s\ changed to \%s\,
+item-name, item-value)));
 			set_config_sourcefile(item-name, item-filename,
   item-sourceline);
 		}
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index 9e9c3f7..a290601 100644
--- a/src/backend/utils/misc/guc-file.l
+++ b/src/backend/utils/misc/guc-file.l
@@ -260,9 +260,8 @@ ProcessConfigFile(GucContext context)
 		{
 			ereport(elevel,
 	(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
-	 errmsg(attempted change of parameter \%s\ ignored,
-			gconf-name),
-	 errdetail(This parameter cannot be changed after server start.)));
+	 errmsg(Parameter \%s\ cannot be changed without restarting the server,
+			gconf-name)));
 			continue;
 		}
 
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index 2224d56..8fa9599 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -4599,18 +4599,16 @@ set_config_option(const char *name, const char *value,
 if (changeVal  !is_newvalue_equal(record, value))
 	ereport(elevel,
 			(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
-	   errmsg(attempted change of parameter \%s\ ignored,
-			  name),
-			 errdetail(This parameter cannot be changed after server start.)));
+	   errmsg(Parameter \%s\ cannot be changed without restarting the server,
+			  name)));
 return true;
 			}
 			if (context != PGC_POSTMASTER)
 			{
 ereport(elevel,
 		(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
-	   errmsg(attempted change of parameter \%s\ ignored,
-			  name),
-		 errdetail(This parameter cannot be changed after server start.)));
+	   errmsg(Parameter \%s\ cannot be changed without restarting the server,
+			  name)));
 return false;
 			}
 			break;

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription: