Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
От | Amit Kapila |
---|---|
Тема | Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] |
Дата | |
Msg-id | 009301ce2ebd$85cdd980$91698c80$@kapila@huawei.com обсуждение исходный текст |
Ответ на | Re: Proposal for Allow postgresql.conf values to be changed via SQL [review] (Greg Smith <greg@2ndQuadrant.com>) |
Ответы |
Re: Proposal for Allow postgresql.conf values to be changed
via SQL [review]
|
Список | pgsql-hackers |
On Monday, April 01, 2013 1:03 PM Greg Smith wrote: > At this point SET PERSISTENT is looking complete enough that some parts > of it might be committed. There's been a lot of useful progress in > nailing down the early/obvious problems and what fundamental approach > makes sense. Accepting the whole thing still seems a bit too invasive > to chew on this late in 9.3 though. Unfortunately it's not until > everything goes in that a really useful change happens. > > What I'll mainly talk about here is how to break this into smaller > pieces to consider as potential future commits, hopeful ones that might > all go in early for 9.4. There's no identified committer for this > feature yet. I think which committer (or committers, plural, given the > number of pieces) is willing to take on the GUC changes, and how > aggressively they want to consider the pieces of this, is going to > determine the timeline for when the moving parts of this idea are > adopted. > > As for the code that seems high risk to me, there's a whole class of > subtle problems possible in the memory management in particular. The > first stress test I did (just to try and break the feature) shook out > what looks so far like a memory allocation concern in the existing > codebase, before this new feature is even considered. That didn't feel > like a good sign to me. > > Stepping back, I see four potential commits worth of changes here now, > and I would consider them in this order: > > 1) Fix for pg_reload_conf memory context growth problem uncovered > during > testing. This is a small patch and minor bug fix that could even go > into 9.3. The problem is not so severe it seems worth the disruption > of > backporting to earlier versions, and it wouldn't really hurt much to > kick the problem forward to 9.4. > > 2) Change the default postgresql.conf to include a config directory. > This has been one of my soapbox positions for a while now, and I think > this feature is working well enough now to have proven the form of > re-arrangement does something useful. This is a small change to the > PostgreSQL code that will ripple out to impact packaging. It would be > possible to do this part and face the main disruption for 9.3 even if > the exact SET PERSISTENT implementation is pushed forward. If there > was > not much else going on with packaging right now, I might even advocate > that myself. Unfortunately, right now "not much else going on" is the > exact opposite of the situation all the packagers are in. It's just a > bad time to talk about it. > > 3) Rearrangement of GUC validation into validate_conf_option function. > This is ~400 lines of code that could be changed as a zero-impact > refactoring, one that just makes the SET PERSISTENT option easier to > implement. > > 4) Implementation of SET PERSISTENT into a single file, > config/persistent.auto.conf. As written, a reasonable amount of error > checking is done to reduce the odds of someone breaking its > implementation without realizing it. The responsibility of activating > the new configuration using pg_reload_conf is pushed toward the user, > documented but without an explicit warning. > > All of these changes have different risk vs. reward trade-offs in them. > I'd guess the validation_config_option change (3) is the riskiest of > this bunch, because it could easily break the standard GUC path in a > major way even for people who don't use the feature. It's not that > much > code, but it is going to take a good bit of committer level review to > accept due to its risk. I think in that case we can have 3 separate patches 1. Memory growth defect fix 2. Default postgresql.conf to include config directory and SET Persistent into single file implementation 3. Rearrangement of GUC validation into validate_conf_option function. As already there is review happened for point 2 and point 1 is an existing code defect fix, so in my opinion Patch 1 and 2 should be considered for 9.3. With Regards, Amit Kapila.
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Greg SmithДата:
Сообщение: Re: Proposal for Allow postgresql.conf values to be changed via SQL [review]
Следующее
От: Robert HaasДата:
Сообщение: Re: [PATCH] Exorcise "zero-dimensional" arrays (Was: Re: Should array_length() Return NULL)