Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]

From: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
To: Greg Smith <greg(at)2ndquadrant(dot)com>
Cc: Amit Kapila <amit(dot)kapila(at)huawei(dot)com>, Andres Freund <andres(at)2ndquadrant(dot)com>, Boszormenyi Zoltan <zb(at)cybertec(dot)at>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Date: 2013-03-11 17:32:04
Message-ID: CAHGQGwFe8ChT9sNGQJce1r2YavDBuUF+hxw5Q8axCKQ2kbs14g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Mar 11, 2013 at 4:39 PM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote:
> On 3/11/13 2:48 AM, Amit Kapila wrote:
>>>
>>> 1) When you change a sighup or user setting, it writes the config file
>>> out. But it does not signal for a reload. Example:
>>
>>
>> I think we cannot guarantee even after calling pg_reload_conf(), as this
>> is
>> an
>> asynchronous function call.
>> We already once had a discussion about this point and as I can understand
>> it
>> is
>> concluded that it should not be handled. Refer the below mail:
>> http://www.postgresql.org/message-id/21869.1360683928@sss.pgh.pa.us
>
>
> I wasn't complaining that the change isn't instant. I understand that can't
> be done. But I think the signal to reload should be sent. If people
> execute SET PERSISTENT, and it doesn't actually do anything until the server
> is next restarted, they will be very surprised. It's OK if it doesn't do
> anything for a second, or until new sessions connect, because that's just
> how SIGHUP/session variables work. That's a documentation issue. Not
> reloading the config at all, I think that's going to trigger a ton of future
> support problems.

I agree with you if SET PERSISTENT reloads only postgresql.auto.conf.
But in current conf reload mechanism, other configuration files like
pg_hba.conf are also reloaded when pg_read_conf() is executed. Probably
I don't like this behavior. Users might get surprised that the configuration
changes by their manual operation (by not SET PERSISTENT) are also
activated by SET PERSISTENT.

And, this change would make the patch bigger...

>>> The check for the include_dir entry needs to happen each time SET
>>> PERSISTENT runs.
>>
>> This can be handled, but for this we need to parse the whole
>> postgresql.conf
>>
>> file
>> and then give the warning. This will increase the time of SET PERSISTENT
>> command.
>
>
> I don't think that's a problem. I can run the command 1800 times per second
> on my laptop right now. If it could only run once per second, that would
> still be fast enough. Doing this the most reliable way it can be handled is
> the important part; if that happens to be the slowest way, too, that is
> acceptable.

Why should the include_dir entry for SET PERSISTENT exist in postgresql.conf?
Is there any use case that users change that include_dir setting? If not, what
about hardcoding the include_dir entry for SET PERSISTENT in the core?
If we do this, we don't need to handle this problem at all.

Regards,

--
Fujii Masao

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Josh Berkus 2013-03-11 17:48:01 Re: postgres_fdw vs data formatting GUCs (was Re: [v9.3] writable foreign tables)
Previous Message Tom Lane 2013-03-11 17:25:02 matview patch readability/correctness gripe