Re: ALTER SYSTEM SET command to change postgresql.conf parameters

On Tue, Dec 24, 2013 at 11:38 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>> On Sun, Dec 22, 2013 at 8:32 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>>> On Sun, Dec 22, 2013 at 3:01 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>>> I found the bug of ALTER SYSTEM SET patch. The procedure to reproduce
>>>> it is here.
>>>>
>>>> $ psql
>>>> =# ALTER SYSTEM SET shared_buffers = '512MB';
>>>> ALTER SYSTEM
>>>> =# \q
>>>> $ pg_ctl -D data reload
>>>> server signaled
>>>> 2013-12-22 18:24:13 JST LOG:  received SIGHUP, reloading configuration files
>>>> 2013-12-22 18:24:13 JST LOG:  parameter "shared_buffers" cannot be
>>>> changed without restarting the server
>>>> 2013-12-22 18:24:13 JST LOG:  configuration file "X??" contains
>>>> errors; unaffected changes were applied
>>>>
>>>
>>> Your analysis is absolutely right.
>>> The reason for this issue is that we want to display filename to which
>>> erroneous parameter
>>> belongs and in this case we are freeing the parameter info before actual error.
>>> To fix, we need to save the filename value before freeing parameter list.
>>>
>>> Please find the attached patch to fix this issue.
>
> When I read ProcessConfigFile() more carefully, I found another related
> problem. The procedure to reproduce the problem is here.
>
> --------------------
> $ psql
> =# ALTER SYSTEM SET shared_buffers = '256MB';
> =# \q
>
> $ echo "shared_buffers = '256MB'" >> $PGDATA/postgresql.conf
> $ pg_ctl -D $PGDATA reload
> 2013-12-25 03:05:44 JST LOG:  parameter "shared_buffers" cannot be
> changed without restarting the server
> 2013-12-25 03:05:44 JST LOG:  parameter "shared_buffers" cannot be
> changed without restarting the server
> 2013-12-25 03:05:44 JST LOG:  configuration file
> "/dav/alter-system/data/postgresql.auto.conf" contains errors;
> unaffected changes were applied
> --------------------
>
> Both postgresql.conf and postgresql.auto.conf contain the setting of
> the parameter that cannot be changed without restarting the server.
> But only postgresql.auto.conf was logged, and which would be confusing,
> I'm afraid.

The reason for above behaviour is that while applying configuration parameters,
it just note down the last file which has error and ignore others. Now
if we just
want to change behaviour for above case that it display both the files, then
during processing of parameters, we can save the errorconffile if it
is different
from previous. However, let us say if user uses include mechanism of conf files
and used new file for specifying some of the changed parameters, then again
similar thing can be observed.

I think here basic idea is user should avoid using multiple methods to change
configuration parameters, however we cannot stop them from doing the same.
So in some cases like above where user use multiple methods to change
configuration can add more work for him to understand and detect the error.

Also, some similar behaviour can be observed if user uses include mechanism
to specify some config parameters without even Alter System changes.

>I think that this problem should be fixed together with the
> problem that I reported before. Thought?

Here I think you might be worried that if we want to fix the behaviour reported
in this mail, it might overlap with the changes of previous fix. So I
think let us
first decide if we want to keep the current behaviour as it is or would like to
change it and if it needs change, what should be the new behaviour?

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com