On 07/28/2014 11:03 AM, Fujii Masao wrote:
> On Sat, Jul 26, 2014 at 1:07 PM, Amit Kapila <amit.kapila16@gmail.com> wrote:
>> On Fri, Jul 25, 2014 at 6:11 PM, Fujii Masao <masao.fujii@gmail.com> wrote:
>>> On Wed, Jul 9, 2014 at 11:05 PM, Amit Kapila <amit.kapila16@gmail.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
