Re: postgresql.auto.conf and reload

From: Josh Berkus <josh(at)agliodbs(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Mark Kirkwood <mark(dot)kirkwood(at)catalyst(dot)net(dot)nz>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgresql.auto.conf and reload
Date: 2014-07-28 18:33:58
Message-ID: 53D69796.5060100@agliodbs.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 07/28/2014 11:03 AM, Fujii Masao wrote:
> On Sat, Jul 26, 2014 at 1:07 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>> On Fri, Jul 25, 2014 at 6:11 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> On Wed, Jul 9, 2014 at 11:05 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
>>> wrote:
>>>> Okay. As mentioned upthread, I have fixed by ensuring that for duplicate
>>>> config params, retain only which comes later during parsing.
>>>> I think it might have been bit simpler to fix, if we try to fix after
>>>> parsing
>>>> is complete, but I think for that we might need to replicate the logic
>>>> at multiple places.
>>>
>>> ISTM that the patch works fine. Only concern is that the logic needs
>>> O(n^2) comparison, which may cause performance problem. But
>>> "n" in O(n^2) is the number of uncommented parameters and I don't
>>> think it's so big, ISTM I can live with the logic...
>>
>> Thanks for reviewing the patch. I also think that having O(n^2)
>> comparisons should not be a problem in this logic as it will be processed
>> only during load/parse of config file which we don't do in performance
>> sensitive path.
>
> Yep.
>
> There is other side effect on this patch. With the patch, when reloading
> the configurartion file, the server cannot warm an invalid setting value
> if it's not the last setting of the parameter. This may cause problematic
> situation as follows.
>
> 1. ALTER SYSTEM SET work_mem TO '1024kB';
> 2. Reload the configuration file ---> success
> 3. Then, a user accidentally adds "work_mem = '2048KB'" into postgresql.conf
> The setting value '2048KB' is invalid, and the unit should be
> 'kB' instead of 'KB'.
> 4. Reload the configuration file ---> success
> The invalid setting is ignored because the setting of work_mem in
> postgresql.auto.conf is preferred. So a user cannot notice that
> postgresql.conf
> has an invalid setting.
> 5. Failover on shared-disk HA configuration happens, then PostgreSQL fails to
> start up because of such an invalid setting. When PostgreSQL
> starts up, the last
> setting is preferred. But all the settings are checked.
>
> Can we live with this issue?

I'd think so, yes. That's pretty extreme corner-case.

Also, it's my perspective that users who change conf by concurrently
editing pg.conf *and* doing ALTER SYSTEM SET are hopeless anyway.
There's simply no way we can protect them from themselves.

--
Josh Berkus
PostgreSQL Experts Inc.
http://pgexperts.com

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2014-07-28 19:29:57 Re: [RFC] Should smgrtruncate() avoid sending sinval message for temp relations
Previous Message Fujii Masao 2014-07-28 18:03:06 Re: postgresql.auto.conf and reload