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

On Monday, March 11, 2013 11:02 PM Fujii Masao wrote:
> 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...

it's more of issue in consistency as pg_reload_conf() is asynchronous.

> >>> 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.

It will be for users who want to use the configuration settings as per
previous version, means by manually
setting in conf file.

With Regards,
Amit Kapila.