Обсуждение: Incorrectly reporting config errors
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. I've tested this on 9.3 and 9.4devel. Thom
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
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
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
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
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
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
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
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
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
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
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
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
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