Re: [HACKERS] Logging configuration changes [REVIEW]
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]
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]
On Wed, Sep 16, 2009 at 12:24 AM, Abhijit Menon-Sen 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]
(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 li
Re: [HACKERS] Logging configuration changes
On ons, 2009-08-26 at 22:13 -0400, Tom Lane wrote:
> Seems to me it would be too chatty to be useful, at least for people who
> set more than one or two things in postgresql.conf. Would it be that
> hard to track which values actually changed? Without having looked at
> the code, I'm thinking that much of the infrastructure must be there
> already now that we have support for undoing commented-out settings.
Found an easy solution; see attached patch.
On a related note, I suggest reverting or revising this logging change:
http://git.postgresql.org/gitweb?p=postgresql.git;a=commitdiff;h=293c816e4aad8e760bcb4eaba0aa16da0ccd2d04
Putting the reason for an error or warning into the detail part is not
acceptable style, IMO.
Index: src/backend/utils/misc/guc-file.l
===
RCS file: /cvsroot/pgsql/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.59
diff -u -3 -p -r1.59 guc-file.l
--- src/backend/utils/misc/guc-file.l 9 Apr 2009 14:21:02 - 1.59
+++ src/backend/utils/misc/guc-file.l 28 Aug 2009 21:30:56 -
@@ -283,6 +283,10 @@ ProcessConfigFile(GucContext context)
/* Now we can re-apply the wired-in default */
set_config_option(gconf->name, NULL, context, PGC_S_DEFAULT,
GUC_ACTION_SET, true);
+ if (context == PGC_SIGHUP)
+ ereport(elevel,
+ (errmsg("parameter \"%s\" removed from configuration file, reset to default",
+ gconf->name)));
}
/*
@@ -309,9 +313,18 @@ ProcessConfigFile(GucContext context)
/* If we got here all the options checked out okay, so apply them. */
for (item = head; item; item = item->next)
{
+ 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);
}
--
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
Peter Eisentraut writes: > On sön, 2009-07-12 at 22:55 +0300, Peter Eisentraut wrote: >> On occasion, I would have found it useful if a SIGHUP didn't only log *that* >> it reloaded the configuration files, but also logged *what* had changed >> (postgresql.conf changes in particular, not so much interested in >> pg_hba.conf). Especially in light of the common mistake of forgetting to >> uncomment a changed value, this would appear to be useful. > Looked into this, looks trivial, except that this would log *all* > parameters that were set as a result of a reload, instead of only the > ones that changed. We don't have the information of what it was > previously readily available. > I think it would still be useful that way, but some people might find it > annoying. Seems to me it would be too chatty to be useful, at least for people who set more than one or two things in postgresql.conf. Would it be that hard to track which values actually changed? Without having looked at the code, I'm thinking that much of the infrastructure must be there already now that we have support for undoing commented-out settings. 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] Logging configuration changes
On sön, 2009-07-12 at 22:55 +0300, Peter Eisentraut wrote: > On occasion, I would have found it useful if a SIGHUP didn't only log *that* > it reloaded the configuration files, but also logged *what* had changed > (postgresql.conf changes in particular, not so much interested in > pg_hba.conf). Especially in light of the common mistake of forgetting to > uncomment a changed value, this would appear to be useful. Looked into this, looks trivial, except that this would log *all* parameters that were set as a result of a reload, instead of only the ones that changed. We don't have the information of what it was previously readily available. I think it would still be useful that way, but some people might find it annoying. Comments? -- 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
On Sun, Jul 12, 2009 at 3:55 PM, Peter Eisentraut wrote: > On occasion, I would have found it useful if a SIGHUP didn't only log *that* > it reloaded the configuration files, but also logged *what* had changed > (postgresql.conf changes in particular, not so much interested in > pg_hba.conf). Especially in light of the common mistake of forgetting to > uncomment a changed value, this would appear to be useful. > > Comments? Sounds neat. ...Robert -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
