Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Date
Msg-id CAHGQGwFe8ChT9sNGQJce1r2YavDBuUF+hxw5Q8axCKQ2kbs14g@mail.gmail.com
Whole thread Raw
In response to Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]  (Greg Smith <greg@2ndQuadrant.com>)
Responses Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]  (Josh Berkus <josh@agliodbs.com>)
Re: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]  (Amit Kapila <amit.kapila@huawei.com>)
List pgsql-hackers
On Mon, Mar 11, 2013 at 4:39 PM, Greg Smith <greg@2ndquadrant.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



pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: matview patch readability/correctness gripe
Next
From: Josh Berkus
Date:
Subject: Re: postgres_fdw vs data formatting GUCs (was Re: [v9.3] writable foreign tables)