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

2013-03-13 13:45 keltezéssel, Andres Freund írta:
> 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.

Yes, thanks.

> It seems we could add another value to enum
> GucStackState, like GUC_SET_PERSISTENT - and process those only if commit &&
> nestlevel == 1.

Maybe it's not needed, only enum GucAction needs a new
GUC_ACTION_PERSISTENT value since that's what has any
business in push_old_value(). Adding two new members to
GucStack like these are enough
bool has_persistent;
config_var_value persistent;
and SET PERSISTENT can be treated as GUC_SET. push_old_value()
can merge GUC values set in the same transaction level.

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

--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
Gröhrmühlgasse 26
A-2700 Wiener Neustadt, Austria
Web: http://www.postgresql-support.de
http://www.postgresql.at/