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

On 2013-03-13 09:09:24 +0100, Boszormenyi Zoltan wrote:
> 2013-03-13 07:42 keltezéssel, Craig Ringer írta:
> >On 03/12/2013 06:27 AM, Craig Ringer wrote:
> >>>>Think also about the case where someone wants to change multiple
> >>>>values together and having just some set and not others would be
> >>>>inconsistent.
> >>Yeah, that's a killer. The reload would need to be scheduled for COMMIT
> >>time, it can't be done by `SET PERSISTENT` directly.
> >Thinking about this some more, I'm not sure this is a good idea.
> >
> >Right now, SET takes effect immediately. Always, without exception.
> >Delaying SET PERSISTENT's effects until commit would make it
> >inconsistent with SET's normal behaviour.
> >
> >However, *not* delaying it would make it another quirky
> >not-transactional not-atomic command. That's OK, but if it's not going
> >to follow transactional semantics it should not be allowed to run within
> >a transaction, like VACUUM .
> >
> >Writing the changes immediately but deferring the reload until commit
> >seems to be the worst of those two worlds.
>
> I was thinking about it a little. There is a hook that runs at the end
> of (sub-)transactions. It can be abused for this purpose to make
> SET PERSISTENT transactional. The subtransactions can also stack
> these settings, by forgetting settings upon ROLLBACK [ TO SAVEPOINT ]
> and overwriting previous settings upon COMMIT and RELEASE SAVEPOINT.
> All it needs is a list and maintaining intermediate pointers when entering
> into a new level of SAVEPOINT. The functions to register such hooks are
> in src/include/access/xact.h:
>
> extern void RegisterXactCallback(XactCallback callback, void *arg);
> extern void UnregisterXactCallback(XactCallback callback, void *arg);
> extern void RegisterSubXactCallback(SubXactCallback callback, void *arg);
> extern void UnregisterSubXactCallback(SubXactCallback callback, void *arg);

(sub)xact commit/abort already calls AtEOXact_GUC(commit, nestlevel). So you
wouldn't even need that. It seems we could add another value to enum
GucStackState, like GUC_SET_PERSISTENT - and process those only if commit &&
nestlevel == 1.
Everytime you see one with commit && nestlevel > 1 you put them into them into
the stack one level up.

This seems like its somewhat in line with the way SET LOCAL is implemented?

> On the other hand, it would make a lot of sense to implement it
> as one setting per file or extending the code to allow modifying
> settings in bulk. The one setting per file should be easier and
> it would also allow extensions to drop their settings automatically
> into the automatic config directory. I don't know who mentioned
> this idea about extensions but it also came up a while back.

I still think one-setting-per-file is the right way to go, yes.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services