Incorrectly reporting config errors

Thom Brown <thom@linux.com> writes:

I'm getting a report of a config error when changing a config value
that requires a restart:
...
2014-01-21 18:14:53 GMT [28718]: [4-1] user=,db=,client= LOG:
received SIGHUP, reloading configuration files
2014-01-21 18:14:53 GMT [28718]: [5-1] user=,db=,client= LOG:
parameter "max_connections" cannot be changed without restarting the
server
2014-01-21 18:14:53 GMT [28718]: [6-1] user=,db=,client= LOG:
configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

It doesn't contain errors.

Yeah it does: it's got a value that can't be applied. I think you're
making a semantic quibble.

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

On 01/21/2014 10:26 AM, Thom Brown wrote:

Hi all,

I'm getting a report of a config error when changing a config value
that requires a restart:

In postgresql.conf

max_connections = 92

(pg_ctl restart)

postgres=# show max_connections ;
max_connections
-----------------
92
(1 row)

(Edit postgresql.conf so that max_connections = 93)

(pg_ctl reload)

Now the log file contains:

2014-01-21 18:14:53 GMT [28718]: [4-1] user=,db=,client= LOG:
received SIGHUP, reloading configuration files
2014-01-21 18:14:53 GMT [28718]: [5-1] user=,db=,client= LOG:
parameter "max_connections" cannot be changed without restarting the
server
2014-01-21 18:14:53 GMT [28718]: [6-1] user=,db=,client= LOG:
configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

It doesn't contain errors. I changed the 92 to 93. If I restart, it
doesn't complain, and there's nothing in the log about the config
anymore.

This seems to be the case for any parameter with a postmaster context.

I can understand why it logs the message about it not changing without
a restart, but the other one seems like a bug.

You wanted a change in a value, the change did not occur, hence an error.

I've tested this on 9.3 and 9.4devel.

Thom

--
Adrian Klaver
adrian.klaver@gmail.com

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

On 21 January 2014 18:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thom Brown <thom@linux.com> writes:

I'm getting a report of a config error when changing a config value
that requires a restart:
...
2014-01-21 18:14:53 GMT [28718]: [4-1] user=,db=,client= LOG:
received SIGHUP, reloading configuration files
2014-01-21 18:14:53 GMT [28718]: [5-1] user=,db=,client= LOG:
parameter "max_connections" cannot be changed without restarting the
server
2014-01-21 18:14:53 GMT [28718]: [6-1] user=,db=,client= LOG:
configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

It doesn't contain errors.

Yeah it does: it's got a value that can't be applied. I think you're
making a semantic quibble.

I see it as technically wrong. There's nothing wrong with my config
file. A reload of the file may not be able to apply all the settings,
but there's no typo or mistake anywhere in my file. I would just need
to restart instead of reload.

However, given that you find it unsurprising, I'll leave it there.

Thom

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

On Tue, Jan 21, 2014 at 1:59 PM, Thom Brown <thom@linux.com> wrote:

On 21 January 2014 18:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thom Brown <thom@linux.com> writes:

I'm getting a report of a config error when changing a config value
that requires a restart:
...
2014-01-21 18:14:53 GMT [28718]: [4-1] user=,db=,client= LOG:
received SIGHUP, reloading configuration files
2014-01-21 18:14:53 GMT [28718]: [5-1] user=,db=,client= LOG:
parameter "max_connections" cannot be changed without restarting the
server
2014-01-21 18:14:53 GMT [28718]: [6-1] user=,db=,client= LOG:
configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

It doesn't contain errors.

Yeah it does: it's got a value that can't be applied. I think you're
making a semantic quibble.

I see it as technically wrong. There's nothing wrong with my config
file. A reload of the file may not be able to apply all the settings,
but there's no typo or mistake anywhere in my file. I would just need
to restart instead of reload.

However, given that you find it unsurprising, I'll leave it there.

I kind of agree with Thom. I understand why it's doing what it's
doing, but it still seems sort of lame.

--
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

Robert Haas <robertmhaas@gmail.com> writes:

I kind of agree with Thom. I understand why it's doing what it's
doing, but it still seems sort of lame.

Well, the point of the message is to report that we failed to apply
all the settings requested by the file. If you prefer some wording
squishier than "error", we could bikeshed the message phrasing.
But I don't think we should suppress the message entirely; nor does
it seem worthwhile to try to track whether all the failures were of
precisely this type.

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

On Tue, Jan 21, 2014 at 3:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

I kind of agree with Thom. I understand why it's doing what it's
doing, but it still seems sort of lame.

Well, the point of the message is to report that we failed to apply
all the settings requested by the file. If you prefer some wording
squishier than "error", we could bikeshed the message phrasing.
But I don't think we should suppress the message entirely; nor does
it seem worthwhile to try to track whether all the failures were of
precisely this type.

Well, to me the whole thing smacks of:

LOG: there is a problem
LOG: please be aware that we logged a message about a problem

The only real argument for the message:

LOG: configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

...is that somebody might think that the presence of a single error
caused all the changes to be ignored. And there's a good reason they
might think that: we used to do it that way. But on the flip side, if
we emitted a LOG or WARNING message every time the user did something
that works in a way incompatible with previous releases, we'd go
insane. So I think the argument for just dumping that message
altogether is actually pretty good.

Bikeshedding the wording is, of course, another viable option. For
that matter, ignoring the problem is a pretty viable option, too. :-)

--
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

Robert Haas <robertmhaas@gmail.com> writes:

The only real argument for the message:

LOG: configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

...is that somebody might think that the presence of a single error
caused all the changes to be ignored. And there's a good reason they
might think that: we used to do it that way. But on the flip side, if
we emitted a LOG or WARNING message every time the user did something
that works in a way incompatible with previous releases, we'd go
insane. So I think the argument for just dumping that message
altogether is actually pretty good.

Hm. I don't recall what the arguments were for adding that message,
but you have a point that it might have served its purpose by now.

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

On 01/21/2014 12:29 PM, Robert Haas wrote:

On Tue, Jan 21, 2014 at 1:59 PM, Thom Brown <thom@linux.com> wrote:

On 21 January 2014 18:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Thom Brown <thom@linux.com> writes:

I'm getting a report of a config error when changing a config value
that requires a restart:
...
2014-01-21 18:14:53 GMT [28718]: [4-1] user=,db=,client= LOG:
received SIGHUP, reloading configuration files
2014-01-21 18:14:53 GMT [28718]: [5-1] user=,db=,client= LOG:
parameter "max_connections" cannot be changed without restarting the
server
2014-01-21 18:14:53 GMT [28718]: [6-1] user=,db=,client= LOG:
configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

It doesn't contain errors.

Yeah it does: it's got a value that can't be applied. I think you're
making a semantic quibble.

I see it as technically wrong. There's nothing wrong with my config
file. A reload of the file may not be able to apply all the settings,
but there's no typo or mistake anywhere in my file. I would just need
to restart instead of reload.

However, given that you find it unsurprising, I'll leave it there.

I kind of agree with Thom. I understand why it's doing what it's
doing, but it still seems sort of lame.

Though I am not sure why it is lame when it seems to be following
protocol; announce the problem, then tell where it originates. Seems
like useful information to me.

postgres-2014-01-21 14:39:54.738 PST-0ERROR: parameter
"max_connections" cannot be changed without restarting the server
postgres-2014-01-21 14:39:54.738 PST-0STATEMENT: SET max_connections=99;

-2014-01-21 14:42:23.166 PST-0LOG: received SIGHUP, reloading
configuration files
-2014-01-21 14:42:23.168 PST-0LOG: parameter "max_connections" cannot
be changed without restarting the server
-2014-01-21 14:42:23.169 PST-0LOG: configuration file
"/usr/local/pgsql93/data/postgresql.conf" contains errors; unaffected
changes were applied

--
Adrian Klaver
adrian.klaver@gmail.com

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

On 2014-01-21 16:13:11 -0500, Robert Haas wrote:

On Tue, Jan 21, 2014 at 3:37 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

I kind of agree with Thom. I understand why it's doing what it's
doing, but it still seems sort of lame.

Well, the point of the message is to report that we failed to apply
all the settings requested by the file. If you prefer some wording
squishier than "error", we could bikeshed the message phrasing.
But I don't think we should suppress the message entirely; nor does
it seem worthwhile to try to track whether all the failures were of
precisely this type.

Well, to me the whole thing smacks of:

LOG: there is a problem
LOG: please be aware that we logged a message about a problem

The only real argument for the message:

LOG: configuration file "/home/thom/Development/data/postgresql.conf"
contains errors; unaffected changes were applied

...is that somebody might think that the presence of a single error
caused all the changes to be ignored. And there's a good reason they
might think that: we used to do it that way. But on the flip side, if
we emitted a LOG or WARNING message every time the user did something
that works in a way incompatible with previous releases, we'd go
insane. So I think the argument for just dumping that message
altogether is actually pretty good.

I don't find that argument all that convincing. The expectation that a
config file isn't applied if it contains errors is a generally
reasonable one, not just because we used to do it that way. I also don't
see the disadvantage of the current behavour of reporting that we didn't
apply the change. Additionally it makes it easier to look for such
errors, by having a relatively easy to remember message to search for in
the log.

What I think is that we're reporting such problems way too often. The
usual cause I know of is something like:
shared_buffers = 4GB
...
shared_buffers = 6GB

When reloading the config we'll report an error in the line setting it
to 4GB because shared_buffers is PGC_POSTMASTER leading to a spurious
"contains errors" message. Rather annoying.

Now, I am sure somebody will argue along the lines of "well, don't do
that then", but I don't see much merit in that argument. A rather common
and sensible configuration is to have a common configuration file used
across servers, which then is overwritten by a per-server or per-cluster
config file containing values specific to a server/cluster.

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

Andres Freund <andres@2ndquadrant.com> wrote:

A rather common and sensible configuration is to have a common
configuration file used across servers, which then is overwritten
by a per-server or per-cluster config file containing values
specific to a server/cluster.

Agreed.

My preference would be to not generate noise for interim states;
just report net changes.  And don't say that a file "contains
errors" when we mean "those options are ignored on reload; they
will only take effect on restart".

--
Kevin Grittner
EDB: 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

Kevin Grittner <kgrittn@ymail.com> writes:

My preference would be to not generate noise for interim states;
just report net changes.

Yeah. Is it worth explicitly detecting and dropping redundant assignments
to the same variable? A naive check for that would be O(N^2) in the
number of entries in the conf file, but perhaps that's still cheap enough
in practice. This would mean for example that

shared_buffers = 'oops'
shared_buffers = '128MB'

would not draw an error, which doesn't bother me but might bother
somebody.

And don't say that a file "contains
errors" when we mean "those options are ignored on reload; they
will only take effect on restart".

I'm not happy about complicating that logic even more. I think the
reasonable choices here are to reword that message somehow, or just
drop it completely.

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

On 2014-01-22 12:10:30 -0500, Tom Lane wrote:

Kevin Grittner <kgrittn@ymail.com> writes:

My preference would be to not generate noise for interim states;
just report net changes.

Yeah. Is it worth explicitly detecting and dropping redundant assignments
to the same variable? A naive check for that would be O(N^2) in the
number of entries in the conf file, but perhaps that's still cheap enough
in practice. This would mean for example that

shared_buffers = 'oops'
shared_buffers = '128MB'

would not draw an error, which doesn't bother me but might bother
somebody.

Wouldn't bother me overly much. But it doesn't seem too hard to avoid
it. Simply split the current ProcessConfigFile() into one more phase.
Currently we seem to be parsing the config file, then iterate over the
list returned from that to set options. If we'd instead have one pass
over the items storing the new value in a new config_generic variable,
after checking, and then a second pass over all gucs setting those that
have changed we should end up at the same complexity since it could be
merged with the reset pass.

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

Tom Lane <tgl@sss.pgh.pa.us> wrote:

Kevin Grittner <kgrittn@ymail.com> writes:

My preference would be to not generate noise for interim states;
just report net changes.

Yeah.  Is it worth explicitly detecting and dropping redundant assignments
to the same variable?  A naive check for that would be O(N^2) in the
number of entries in the conf file, but perhaps that's still cheap enough
in practice.  This would mean for example that

   shared_buffers = 'oops'
   shared_buffers = '128MB'

would not draw an error, which doesn't bother me but might bother
somebody.

It doesn't bother me any.

And don't say that a file "contains
errors" when we mean "those options are ignored on reload; they
will only take effect on restart".

I'm not happy about complicating that logic even more.  I think the
reasonable choices here are to reword that message somehow, or just
drop it completely.

I agree.  No strong preference which,

--
Kevin Grittner
EDB: 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