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/
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
