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

On Sun, Dec 22, 2013 at 8:32 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Sun, Dec 22, 2013 at 3:01 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)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
>>
>> The configuration file name in the last log message is broken. This problem was
>> introduced by the ALTER SYSTEM SET patch.
>>
>>> FreeConfigVariables(head);
>>> <snip>
>>> else if (apply)
>>> ereport(elevel,
>>> (errcode(ERRCODE_CONFIG_FILE_ERROR),
>>> errmsg("configuration file \"%s\" contains errors; unaffected changes were applied",
>>> ErrorConfFile)));
>>
>> The cause of the problem is that, in ProcessConfigFile(), the log
>> message including
>> the 'ErrorConfFile' is emitted after 'head' is free'd even though
>> 'ErrorConfFile' points
>> to one of entry of the 'head'.
>
> 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.
>
> Note - In my m/c, I could not reproduce the issue. I think this is not
> surprising because we
> are not setting freed memory to NULL, so it can display anything
> (sometimes right value as well)

If you find that the provided patch doesn't fix the problem or you don't
find it appropriate due to some other reason, let me know the feedback.

I shall be able to provide updated patch early next as I will be on vacation for
remaining part of this week.

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