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

2013-03-17 17:05 keltezéssel, Greg Smith írta:
> On 3/14/13 4:48 PM, Boszormenyi Zoltan wrote:
>> The attached patch makes
>> SET PERSISTENT transactional and uses one setting per file.
>> It uses the currently existing parsing and validating code
>> and because of this, the patch is half the size of v12 from Amit.
>
> That's not a completely fair comparison, because you lost all the regression testing 
> code too.

True.

> This does look like a usefully tighter implementation in a few ways, so good progress on 
> that.
>
> I still can't see any reason to prefer this "one setting per file" idea.  As I see it, 
> that is pushing the complexity toward the user in a bad way, seemingly just so it's 
> easier to implement.  Most of my customers now use tools like Puppet to manage their 
> PostgreSQL configuration.  I do not want to have this conversation:
>
> Greg:  "You can use SET PERSISTENT to modify settings instead of changing the 
> postgresql.conf"
>
> User:  "That's useful.  How do we adjust Puppet to make sure it picks up the changes?"
>
> Greg:  "You just scan this config directory and add every file that appears in there!  
> Each setting will be in its own file."
>
> User:  "<shocked look>  It creates new files?  That isn't going to fit into our version 
> control approach easily.  Can we disable this feature so no one accidentally uses it?"
>
> I'm not exaggerating here--"one setting per file" makes this feature less than useless 
> to me.  It becomes something where I will have to waste time defending against people 
> using it.  I'd prefer to not have this at all than to do it that way.
>
> That we're breaking these settings off into their own file, instead of trying to edit 
> the postgresql.conf, to me is a pragmatic trade-off to keep the implementation from 
> being really complicated.  It's also a step forward in a larger series for how to 
> improve configuration management.  Just because that change introduces an entire 
> directory being scanned, I don't see that as an excuse to clutter it with a long list of 
> files too.

OK. I just wanted to show an alternative implementation.

I admit, I haven't read all mails from this thread so I don't know
how important for this feature to be non-transactional, or
whether it would be better to be transactional.

The one-file-to-rule-them-all approach can also be added to this
patch as well, but synchronizing transactions over parsing and
rewriting the extra file would decrease the relaxed effects of
synchronous_commit. On the other hand, writing out one file
per setting, although atomic to the level of one file, cannot be
guaranteed to be atomic as a whole for all settings changed
in the transaction without some synchronization.

As far as I can see, AtEOXact_GUC() is called outside the
critical section and is not synchronized any other way.
(Currently, there's no need to.)

Best regards,
Zoltán Böszörményi

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