On 23 December 2012 15:24, Fujii Masao <masao.fu...@gmail.com> wrote:
> The latest patch is the following. Of course, this cannot be applied > cleanly in HEAD. > http://archives.postgresql.org/message-id/CAHGQGwHB==cjehme6jiuy-knutrx-k3ywqzieg1gpnb3ck6...@mail.gmail.com > >> Just by looking at the last few posts, this seems like a no brainer. The >> impression I get is that there's a patch that's otherwise ready to be >> applied, but Simon and some others want to have backwards-compatiblity added >> to it first. The patch isn't ready to be applied. Reviewing this patch again from scratch, it does the following... 1. Makes recovery.conf parameters into GUCs 2. Moves these parameters to postgresql.conf 3. Changes all the docs referring to recovery.conf What the patch doesn't change is the requirement to have a file that causes the server to place itself into archive recovery. So there is no more recovery.conf and instead we have a file called recovery.trigger instead. We now support the concept of multiple .conf files, so continuing to use a file called recovery.conf for those parameters works just fine now without much effort. Note also that the recovery.conf requirement to put things in quotes still works when they are GUCs, its just no longer a requirement like it used to be. So previous files will work just fine. I don't see any point doing (2) or (3), especially since it makes it a huge patch and especially since the trend is to break down large config files into smaller pieces for use by Chef and Puppet. Given the continuing need for a file to trigger recovery, changing the name of the file from recovery.conf to recovery.trigger just breaks backwards compatibility for absolutely no gain whatsoever (as long as we do (1)). I think it would be much easier to do just (1) in this release. What we do want is the ability to move recovery.conf elsewhere (if we choose), so that data directory is not writable by anything but postgres server. If we do that, we'll have to make the config directory writable by the server, whereas it wasn't before, so that can cause problems also. That's the most useful additional change that I see in this area, even better than (1). (Note that you can't tell which server is the master purely from reading primary_conninfo, since that you can't tell if you're a cascading standby, or not. As discussed elsewhere, we could actually do some more pushups to identify the current >From my perspective this patch and its design wasn't well thought through and on balance I'm happy it wasn't committed earlier. >From all of the above, I think its worth doing this * allowing recovery.conf to be in a different directory * make recovery config parameters into GUCs * no other changes to way things currently work I don't think that represents enough change to keep people happy, but I don't see anything else useful being suggested in this patch. Other design thoughts welcome, but personally, I think rushing this design through at this stage is likely to require us to change the design again in later releases. Productive comments welcome, Merry Christmas -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
