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.

--
Greg Smith   2ndQuadrant US    g...@2ndquadrant.com   Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.com


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to