Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 7/10/13 9:39 AM, Heikki Linnakangas wrote: On 10.07.2013 02:54, Josh Berkus wrote: One bit of complexity I'd like to see go away is that we have two trigger files: one to put a server into replication, and one to promote it. The promotion trigger file is a legacy of warm standby, I believe. Maybe, now that we have pg_ctl promote available, we can eliminate the promotion trigger? No, see http://www.postgresql.org/message-id/5112a54b.8090...@vmware.com. Right, you had some good points in there. STONITH is so hard already, we need to be careful about eliminating options there. All the summaries added here have actually managed to revive this one usefully early in the release cycle! Well done. I just tried to apply Michael's 20130325_recovery_guc_v3.patch and the bit rot isn't that bad either. 5 rejection files, nothing big in them. The only overlap between the recovery.conf GUC work and refactoring the trigger file is that the trigger file is referenced in there, and we really need to point it somewhere to be most useful. Personally, I don't care if we support the old recovery.conf-trigger behavior as long as I'm not forced to use it. If you accept Heikki's argument for why the file can't go away altogether, and it's pretty reasonable, I think two changes reach a point where everyone can live with this: -We need a useful default filename for trigger_file to point at. -pg_ctl failover creates that file. As for the location to default to, the pg_standby docs suggest /tmp/pgsql.trigger.5432 while the Binary Replication Tutorial on the wiki uses a RedHat directory layout $PGDATA/failover The main reason I've preferred something in the data directory is that triggering a standby is too catastrophic for me to be comfortable putting it in /tmp. Any random hooligan with a shell account can trigger a standby and break its replication. Putting the unix socket into /tmp only works because the server creates the file as part of startup. Here that's not possible, because creating the trigger is the signalling mechanism. I don't think there needs to be a CLI interface for putting the alternate possible text into the trigger--that you can ask for 'fast' startup. It's nice to have available as an expert, but it's fine for that to be harder to do. -- Greg Smith 2ndQuadrant USg...@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
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 10.07.2013 02:54, Josh Berkus wrote: On 07/08/2013 11:43 PM, Simon Riggs wrote: 1. MOVE SETTINGS All settings move into the postgresql.conf. Comment: AFAIK, all agree this. Good to go then. +1. 2. RELOCATE RECOVERY PARAMETER FILE(s) As of 9.2, relocating the postgresql.conf means there are no user writable files needed in the data directory. Comment: AFAIK, all except Heikki wanted this. He has very strong objections to my commit that would have allowed relocating recovery.conf outside of the data directory. By which he means both the concepts of triggerring replication and of specifying parameters. Changes in 9.3 specifically write files to the data directory that expect this. Yeah, I didn't understand why this was shot down either. Heikki? Does this refer to this: http://www.postgresql.org/message-id/5152f778.2070...@vmware.com ? I listed some objections and suggestions there. Probably the biggest issue back then, however, was that it was committed so late in the release cycle. In any case, relocating the config/trigger file has nothing to do with turning recovery.conf parameters into GUCs, so let's not confuse this patch with that goal. 3. SEPARATE TRIGGER FILE Creating a standby.enabled file in the directory that houses the postgresql.conf (same logic as include uses to locate things) puts the system into recovery mode. That feature needs to save some state, and making those decisions based on existence of a file is already a thing we do. Rather than emulating the rename to recovery.done that happens now, the server can just delete it, to keep from incorrectly returning to a state it's exited. A UI along the lines of the promote one, allowing pg_ctl standby, should fall out of here. I think this is enough that people who just want to use replication features need never hear about this file at all, at least during the important to simplify first run through. Comment: AFAIK, all except Heikki are OK with this. Sorry, I don't quite understand what this is about. Can you point me to the previous discussion on this? pg_ctl standby sounds handy. It's not very useful without something like pg_rewind or some other mechanism to do a clean failover, though. Have to make sure that we have enough safeguards in place that you can't shoot yourself in the foot with that, though; if you turn a master server into a standby with that, must make sure that you can't corrupt the database if you point that standby to another standby. But I don't see how that's related to changing recovery.conf parameters into gucs. One bit of complexity I'd like to see go away is that we have two trigger files: one to put a server into replication, and one to promote it. The promotion trigger file is a legacy of warm standby, I believe. Maybe, now that we have pg_ctl promote available, we can eliminate the promotion trigger? No, see http://www.postgresql.org/message-id/5112a54b.8090...@vmware.com. Also, previously, deleting the recovery.conf file did not cause the server to be promoted AFAIK. Is that something we should change if we're going to keep a trigger file to start replication? Deleting recovery.conf file (and restarting) takes the server out of standby mode, but in an unsafe way. Yeah, would be nice to do something about that. 4. DISALLOW PREVIOUS API If startup finds a recovery.conf file where it used to live at, abort--someone is expecting the old behavior. Hint to RTFM or include a short migration guide right on the spot. That can have a nice section about how you might use the various postgresql.conf include* features if they want to continue managing those files separately. Example: rename it as replication.conf and use include_if_exists if you want to be able to rename it to recovery.done like before. Or drop it into a conf.d directory where the rename will make it then skipped. Comment: I am against this. Tool vendors are not the problem here. There is no good reason to just break everybody's scripts with no warning of future deprecataion and an alternate API, especially since we now allow multiple parameter files. Well, the issue is not so much the presence of a recovery.conf file full of config variables ... which as you point out is now effectively supported ... but the use of that as a trigger file. So I think the two points you make here are flipped. Personally, I don't care if we support the old recovery.conf-trigger behavior as long as I'm not forced to use it. The main objection to supporting both was code complexity, I believe. I'm also undecided on this one. If we can figure out a good way forward that keeps backwards-compatibility, good. But it's not worth very much to me - if we can get a better interface overall by dropping backwards-compatibility, then let's drop it. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 5 July 2013 19:49, Josh Berkus j...@agliodbs.com wrote: Robert, Simon, All, On 04/01/2013 04:51 AM, Robert Haas wrote: On Thu, Mar 28, 2013 at 11:48 AM, Simon Riggs si...@2ndquadrant.com wrote: a) recovery parameters are made into GUCs (for which we have a patch from Fujii) b) all processes automatically read recovery.conf as the last step in reading configuration files, if it exists, even if data_directory parameter is in use (attached patch) c) we trigger archive recovery by the presence of either recovery.conf or recovery.trigger in the data directory. At the end, we rename to recovery.done just as we do now. This means that any parameters put into recovery.conf will not be re-read when we SIGHUP after end of recovery. Note that recovery.trigger will NOT be read for parameters and is assumed to be zero-length. (minor patch required) I still prefer Greg Smith's proposal. http://www.postgresql.org/message-id/4ee91248.8010...@2ndquadrant.com So, this seems to still be stalled. Is there a way forward on this which won't cause us to wait *another* version before we have working replication GUCs? This needs to be broken down rather than just say I like Greg's proposal, or I have written a patch. Writing the patch is not the/an issue. Greg's points were these (I have numbered them and named/characterised them) 1. MOVE SETTINGS All settings move into the postgresql.conf. Comment: AFAIK, all agree this. 2. RELOCATE RECOVERY PARAMETER FILE(s) As of 9.2, relocating the postgresql.conf means there are no user writable files needed in the data directory. Comment: AFAIK, all except Heikki wanted this. He has very strong objections to my commit that would have allowed relocating recovery.conf outside of the data directory. By which he means both the concepts of triggerring replication and of specifying parameters. Changes in 9.3 specifically write files to the data directory that expect this. 3. SEPARATE TRIGGER FILE Creating a standby.enabled file in the directory that houses the postgresql.conf (same logic as include uses to locate things) puts the system into recovery mode. That feature needs to save some state, and making those decisions based on existence of a file is already a thing we do. Rather than emulating the rename to recovery.done that happens now, the server can just delete it, to keep from incorrectly returning to a state it's exited. A UI along the lines of the promote one, allowing pg_ctl standby, should fall out of here. I think this is enough that people who just want to use replication features need never hear about this file at all, at least during the important to simplify first run through. Comment: AFAIK, all except Heikki are OK with this. 4. DISALLOW PREVIOUS API If startup finds a recovery.conf file where it used to live at, abort--someone is expecting the old behavior. Hint to RTFM or include a short migration guide right on the spot. That can have a nice section about how you might use the various postgresql.conf include* features if they want to continue managing those files separately. Example: rename it as replication.conf and use include_if_exists if you want to be able to rename it to recovery.done like before. Or drop it into a conf.d directory where the rename will make it then skipped. Comment: I am against this. Tool vendors are not the problem here. There is no good reason to just break everybody's scripts with no warning of future deprecataion and an alternate API, especially since we now allow multiple parameter files. -- 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
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 07/08/2013 11:43 PM, Simon Riggs wrote: This needs to be broken down rather than just say I like Greg's proposal, or I have written a patch. Writing the patch is not the/an issue. Greg's points were these (I have numbered them and named/characterised them) Thanks for the nice summary! Makes it easy for the rest of us to address. 1. MOVE SETTINGS All settings move into the postgresql.conf. Comment: AFAIK, all agree this. Good to go then. 2. RELOCATE RECOVERY PARAMETER FILE(s) As of 9.2, relocating the postgresql.conf means there are no user writable files needed in the data directory. Comment: AFAIK, all except Heikki wanted this. He has very strong objections to my commit that would have allowed relocating recovery.conf outside of the data directory. By which he means both the concepts of triggerring replication and of specifying parameters. Changes in 9.3 specifically write files to the data directory that expect this. Yeah, I didn't understand why this was shot down either. Heikki? 3. SEPARATE TRIGGER FILE Creating a standby.enabled file in the directory that houses the postgresql.conf (same logic as include uses to locate things) puts the system into recovery mode. That feature needs to save some state, and making those decisions based on existence of a file is already a thing we do. Rather than emulating the rename to recovery.done that happens now, the server can just delete it, to keep from incorrectly returning to a state it's exited. A UI along the lines of the promote one, allowing pg_ctl standby, should fall out of here. I think this is enough that people who just want to use replication features need never hear about this file at all, at least during the important to simplify first run through. Comment: AFAIK, all except Heikki are OK with this. One bit of complexity I'd like to see go away is that we have two trigger files: one to put a server into replication, and one to promote it. The promotion trigger file is a legacy of warm standby, I believe. Maybe, now that we have pg_ctl promote available, we can eliminate the promotion trigger? Also, previously, deleting the recovery.conf file did not cause the server to be promoted AFAIK. Is that something we should change if we're going to keep a trigger file to start replication? Also, I'm not keen on the idea that the start-replication trigger file will still be *required*. I really want to be able to manage my setup entirely through configuration/pg_ctl directives and not be forced to use a trigger file. 4. DISALLOW PREVIOUS API If startup finds a recovery.conf file where it used to live at, abort--someone is expecting the old behavior. Hint to RTFM or include a short migration guide right on the spot. That can have a nice section about how you might use the various postgresql.conf include* features if they want to continue managing those files separately. Example: rename it as replication.conf and use include_if_exists if you want to be able to rename it to recovery.done like before. Or drop it into a conf.d directory where the rename will make it then skipped. Comment: I am against this. Tool vendors are not the problem here. There is no good reason to just break everybody's scripts with no warning of future deprecataion and an alternate API, especially since we now allow multiple parameter files. Well, the issue is not so much the presence of a recovery.conf file full of config variables ... which as you point out is now effectively supported ... but the use of that as a trigger file. So I think the two points you make here are flipped. Personally, I don't care if we support the old recovery.conf-trigger behavior as long as I'm not forced to use it. The main objection to supporting both was code complexity, I believe. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 07/05/2013 10:09 PM, Michael Paquier wrote: Yeah, it would be good to revive this thread now, which is the beginning of the development cycle. As of now, just to recall everybody, an agreement on this patch still needs to be found... Simon is concerned with backward compatibility. Greg presented a nice spec some time ago (Robert and I liked it) which would break backward compatibility but allow to begin with a fresh infrastructure. As folks know, I favor Smith's approach. However, as far as I can find, GSmith never posted a patch for his version. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 2013/07/09, at 4:09, Josh Berkus j...@agliodbs.com wrote: On 07/05/2013 10:09 PM, Michael Paquier wrote: Yeah, it would be good to revive this thread now, which is the beginning of the development cycle. As of now, just to recall everybody, an agreement on this patch still needs to be found... Simon is concerned with backward compatibility. Greg presented a nice spec some time ago (Robert and I liked it) which would break backward compatibility but allow to begin with a fresh infrastructure. As folks know, I favor Smith's approach. However, as far as I can find, GSmith never posted a patch for his version. Actually I did. http://www.postgresql.org/message-id/CAB7nPqR+fpopEDMoecK+AfZB5a8kUUvxpU=1a2JiX5d9s=0...@mail.gmail.com -- Michael (Sent from my mobile phone)
Re: [HACKERS] Changing recovery.conf parameters into GUCs
Robert, Simon, All, On 04/01/2013 04:51 AM, Robert Haas wrote: On Thu, Mar 28, 2013 at 11:48 AM, Simon Riggs si...@2ndquadrant.com wrote: a) recovery parameters are made into GUCs (for which we have a patch from Fujii) b) all processes automatically read recovery.conf as the last step in reading configuration files, if it exists, even if data_directory parameter is in use (attached patch) c) we trigger archive recovery by the presence of either recovery.conf or recovery.trigger in the data directory. At the end, we rename to recovery.done just as we do now. This means that any parameters put into recovery.conf will not be re-read when we SIGHUP after end of recovery. Note that recovery.trigger will NOT be read for parameters and is assumed to be zero-length. (minor patch required) I still prefer Greg Smith's proposal. http://www.postgresql.org/message-id/4ee91248.8010...@2ndquadrant.com So, this seems to still be stalled. Is there a way forward on this which won't cause us to wait *another* version before we have working replication GUCs? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On Sat, Jul 6, 2013 at 3:49 AM, Josh Berkus j...@agliodbs.com wrote: Robert, Simon, All, On 04/01/2013 04:51 AM, Robert Haas wrote: On Thu, Mar 28, 2013 at 11:48 AM, Simon Riggs si...@2ndquadrant.com wrote: a) recovery parameters are made into GUCs (for which we have a patch from Fujii) b) all processes automatically read recovery.conf as the last step in reading configuration files, if it exists, even if data_directory parameter is in use (attached patch) c) we trigger archive recovery by the presence of either recovery.conf or recovery.trigger in the data directory. At the end, we rename to recovery.done just as we do now. This means that any parameters put into recovery.conf will not be re-read when we SIGHUP after end of recovery. Note that recovery.trigger will NOT be read for parameters and is assumed to be zero-length. (minor patch required) I still prefer Greg Smith's proposal. http://www.postgresql.org/message-id/4ee91248.8010...@2ndquadrant.com So, this seems to still be stalled. Is there a way forward on this which won't cause us to wait *another* version before we have working replication GUCs? Yeah, it would be good to revive this thread now, which is the beginning of the development cycle. As of now, just to recall everybody, an agreement on this patch still needs to be found... Simon is concerned with backward compatibility. Greg presented a nice spec some time ago (Robert and I liked it) which would break backward compatibility but allow to begin with a fresh infrastructure. -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On Thu, Mar 28, 2013 at 11:48 AM, Simon Riggs si...@2ndquadrant.com wrote: What we want to do is make recovery parameters into GUCs, allowing them to be reset by SIGHUP and also to allow all users to see the parameters in use, from any session. The existing mechanism for recovery is that 1. we put parameters in a file called recovery.conf 2. we use the existence of a recovery.conf file to trigger archive recovery/replication I also wish to see backwards compatibility maintained, so am proposing the following: a) recovery parameters are made into GUCs (for which we have a patch from Fujii) b) all processes automatically read recovery.conf as the last step in reading configuration files, if it exists, even if data_directory parameter is in use (attached patch) c) we trigger archive recovery by the presence of either recovery.conf or recovery.trigger in the data directory. At the end, we rename to recovery.done just as we do now. This means that any parameters put into recovery.conf will not be re-read when we SIGHUP after end of recovery. Note that recovery.trigger will NOT be read for parameters and is assumed to be zero-length. (minor patch required) I still prefer Greg Smith's proposal. http://www.postgresql.org/message-id/4ee91248.8010...@2ndquadrant.com -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
Simon, All, The new approach seems fine to me; I haven't looked at the code. If Tom doesn't feel like it's overly complicated, then this seems like a good compromise. The desire to move recovery.conf/trigger to a different directory is definitely wanted by our Debian contingent. Right now, the fact that Debian has all .confs in /etc/, but that it doesn't work to relocate recovery.conf, is a constant source of irritation. -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
Josh Berkus j...@agliodbs.com writes: The desire to move recovery.conf/trigger to a different directory is definitely wanted by our Debian contingent. Right now, the fact that Debian has all .confs in /etc/, but that it doesn't work to relocate recovery.conf, is a constant source of irritation. It seems like this is confusing two different problems. If we get rid of recovery.conf per se in favor of folding the settings into GUCs in the regular config file, then the first aspect of the issue goes away, no? The second aspect is where to put the trigger file, and I'm not at all convinced that anybody would want the trigger file to be in the same place they put external config files, mainly because the trigger has to be in a server-writable directory. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 29 March 2013 01:17, Michael Paquier michael.paqu...@gmail.com wrote: The main argument on which this proposal is based on is to keep backward-compatibility. The main objective is to get recovery parameters as GUCs, as I said On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs si...@2ndquadrant.com wrote: What we want to do is make recovery parameters into GUCs, allowing them to be reset by SIGHUP and also to allow all users to see the parameters in use, from any session. From the user's perspective, we are making no changes. All recovery parameters will work exactly the same as they always did, just now we get to see their values more easily and we can potentially place them in a different file if we wish. The user will have no idea that we plan to do some internal refactoring of how we process the parameters. So IMHO simplicity means continuing to work the way it always did work. We simply announce PostgreSQL now supports configuration directories. All parameters, including recovery parameters, can be placed in any configuration file, or in $PGDATA/recovery.conf, as before. We introduced pg_ctl promote with a new API without removing existing ones, and some people are still in favour of keeping both APIs. Doing the same thing here makes sense and reduces conceptual change. Early discussions had difficulties because of the lack of config directories, include_if_exists and this patch. We now have the technical capability to meet every request. Circumstances have changed and outcomes may change also. -- 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
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs si...@2ndquadrant.com wrote: On 29 March 2013 01:17, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs si...@2ndquadrant.com wrote: Early discussions had difficulties because of the lack of config directories, include_if_exists and this patch. We now have the technical capability to meet every request. Circumstances have changed and outcomes may change also. Thanks for the clarifications. The following questions are still unanswered: 1) If recovery.trigger and recovery.conf are specified. To which one the priority is given? 2) If both recovery.trigger and recovery.conf are used, let's imagine that the server removes recovery.trigger but fails in renaming recovery.conf but a reason or another. Isn't there a risk of inconsistency if both triggering methods are used at the same time? 3) Forcing a harcode of include_is_exists = 'recovery.conf' at the bottom of postgresql.conf doesn't look like a hack? 4) Based on your proposal, are all the parameters included in postgresql.conf.sample or not? Or only primary_conninfo, trigger_file and standby_mode? -- Michael
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 29 March 2013 13:24, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs si...@2ndquadrant.com wrote: On 29 March 2013 01:17, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs si...@2ndquadrant.com wrote: Early discussions had difficulties because of the lack of config directories, include_if_exists and this patch. We now have the technical capability to meet every request. Circumstances have changed and outcomes may change also. Thanks for the clarifications. The following questions are still unanswered: Minor points only. We can implement this differently if you have alternate proposals. 1) If recovery.trigger and recovery.conf are specified. To which one the priority is given? Neither. No priority is required. If either is present we are triggered. 2) If both recovery.trigger and recovery.conf are used, let's imagine that the server removes recovery.trigger but fails in renaming recovery.conf but a reason or another. Isn't there a risk of inconsistency if both triggering methods are used at the same time? No. If writes to the filesystem fail, you have much bigger problems. 3) Forcing a harcode of include_is_exists = 'recovery.conf' at the bottom of postgresql.conf doesn't look like a hack? Well, that's just an emotive term to describe something you don't like. There are no significant downsides, just a few lines of code, like we have in many places for various purposes, such as the support of multiple APIs for triggering standbys. 4) Based on your proposal, are all the parameters included in postgresql.conf.sample or not? Or only primary_conninfo, trigger_file and standby_mode? Other values are specific to particular situations (e.g. PITR) and if set in the wrong context could easily break replication. We could add them if people wish it, but it would be commented out with a clear don't touch these message, so it seems more sensible to avoid them IMHO. -- 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
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On Fri, Mar 29, 2013 at 01:56:50PM +, Simon Riggs wrote: On 29 March 2013 13:24, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Mar 29, 2013 at 9:59 PM, Simon Riggs si...@2ndquadrant.com wrote: On 29 March 2013 01:17, Michael Paquier michael.paqu...@gmail.com wrote: On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs si...@2ndquadrant.com wrote: Early discussions had difficulties because of the lack of config directories, include_if_exists and this patch. We now have the technical capability to meet every request. Circumstances have changed and outcomes may change also. Thanks for the clarifications. The following questions are still unanswered: Minor points only. We can implement this differently if you have alternate proposals. Seems we are doing design on this long after the final 9.3 commit fest should have closed. I think we need to punt this to 9.4. -- Bruce Momjian br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
On 29 March 2013 01:17, Michael Paquier michael.paqu...@gmail.com wrote: I highly recommend that you use one of the latest updated version I sent. Fujii's version had some bugs, one of them being that as standbyModeRequested can be set to true if specified in postgresql.conf, a portion of the code using in xlog.c can be triggered even if ArchiveRecoveryRequested is not set to true. I also added fixes related to documentation. Yes, that is what I meant by Fujii's patch. He would still be credited first, having done the main part of the work. I'll call it Michael's updated version to avoid any further confusion. -- 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
[HACKERS] Changing recovery.conf parameters into GUCs
What we want to do is make recovery parameters into GUCs, allowing them to be reset by SIGHUP and also to allow all users to see the parameters in use, from any session. The existing mechanism for recovery is that 1. we put parameters in a file called recovery.conf 2. we use the existence of a recovery.conf file to trigger archive recovery/replication I also wish to see backwards compatibility maintained, so am proposing the following: a) recovery parameters are made into GUCs (for which we have a patch from Fujii) b) all processes automatically read recovery.conf as the last step in reading configuration files, if it exists, even if data_directory parameter is in use (attached patch) c) we trigger archive recovery by the presence of either recovery.conf or recovery.trigger in the data directory. At the end, we rename to recovery.done just as we do now. This means that any parameters put into recovery.conf will not be re-read when we SIGHUP after end of recovery. Note that recovery.trigger will NOT be read for parameters and is assumed to be zero-length. (minor patch required) This allows these use cases 1. Existing users create $PGDATA/recovery.conf and everything works as before. No servers crash, because the HA instructions in the wiki still work. Users can now see the parameters in pg_settings and we can use SIGHUP without restarting server. Same stuff, new benefits. 2. New users wish to move their existing recovery.conf file to the config directory. Same file contents, same file name (if desired), same behaviour, just more convenient location for config management tools. Recovery is triggered by recovery.trigger in $PGDATA. Trigger and configuration are now separate, if desired. 3. Split mode. We can put things like trigger_file into the postgresql.conf directory and also put other parameters (for example PITR settings) into recovery.conf. Where multiple tools are in use, we support both APIs. Specific details... * primary_conninfo, trigger_file and standby_mode are added to postgresql.conf.sample * all ex-recovery.conf parameters are SIGHUP, so no errors if recovery.conf has changed to .done If desired, this behaviour could be enabled by a parameter called recovery_conf_enabled = on (default). When set to off, this would throw an error if recovery.conf exists. (I don't see a need for this) The patch to implement this is very small (attached). This works standalone, but obviously barfs at the actual parameter parsing stage. Just in case it wasn't clear, this patch is intended to go with the parts of Fujji's patch that relate to GUC changes. If we agree, I will merge and re-post before commit. -- Simon Riggs http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services read_recovery.conf_from_data_directory.v1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Changing recovery.conf parameters into GUCs
Hi, The main argument on which this proposal is based on is to keep backward-compatibility. This has been discussed before many times and the position of each people is well-known, so I am not going back to that... So, based on *only* what I see in this thread... On Fri, Mar 29, 2013 at 12:48 AM, Simon Riggs si...@2ndquadrant.com wrote: What we want to do is make recovery parameters into GUCs, allowing them to be reset by SIGHUP and also to allow all users to see the parameters in use, from any session. The existing mechanism for recovery is that 1. we put parameters in a file called recovery.conf 2. we use the existence of a recovery.conf file to trigger archive recovery/replication I also wish to see backwards compatibility maintained, so am proposing the following: a) recovery parameters are made into GUCs (for which we have a patch from Fujii) b) all processes automatically read recovery.conf as the last step in reading configuration files, if it exists, even if data_directory parameter is in use (attached patch) c) we trigger archive recovery by the presence of either recovery.conf or recovery.trigger in the data directory. At the end, we rename to recovery.done just as we do now. This means that any parameters put into recovery.conf will not be re-read when we SIGHUP after end of recovery. Note that recovery.trigger will NOT be read for parameters and is assumed to be zero-length. (minor patch required) This allows these use cases 1. Existing users create $PGDATA/recovery.conf and everything works as before. No servers crash, because the HA instructions in the wiki still work. Users can now see the parameters in pg_settings and we can use SIGHUP without restarting server. Same stuff, new benefits. Forcing hardcoding of include_if_exists recovery.conf at the bottom of postgresql.conf is not a good thing for the user as it makes postgresql.conf processing more opaque and complicates parameter reloading. IMO, simplicity and transparency of operations are important when processing parameters. 2. New users wish to move their existing recovery.conf file to the config directory. Same file contents, same file name (if desired), same behaviour, just more convenient location for config management tools. Recovery is triggered by recovery.trigger in $PGDATA. Trigger and configuration are now separate, if desired. So, people could be able to also define a recovery.trigger file? What about the case where both recovery.conf and recovery.trigger are found in the base folder? Priority needs to be given to one way of processing or the other. Is it really our goal to confuse the user with internal priority mechanisms at least for GUC handling? 3. Split mode. We can put things like trigger_file into the postgresql.conf directory and also put other parameters (for example PITR settings) into recovery.conf. Where multiple tools are in use, we support both APIs. Specific details... * primary_conninfo, trigger_file and standby_mode are added to postgresql.conf.sample Not adding all the recovery_target_* parameters would confuse the user. With this proposal it is actually possible to define a recovery target inside postgresql.conf even if the parameter is not plainly written in postgresql.conf.sample. * all ex-recovery.conf parameters are SIGHUP, so no errors if recovery.conf has changed to .done Drop of recovery.trigger at the same time? And what about the case where both files are specified, that the server can only remove one of the trigger files, and is then restarted with only 1 trigger file present? If desired, this behaviour could be enabled by a parameter called recovery_conf_enabled = on (default). When set to off, this would throw an error if recovery.conf exists. (I don't see a need for this) Me neither, the less GUCs the better. The patch to implement this is very small (attached). This works standalone, but obviously barfs at the actual parameter parsing stage. Just in case it wasn't clear, this patch is intended to go with the parts of Fujji's patch that relate to GUC changes. If we agree, I will merge and re-post before commit. If an agreement is reached based on this proposal, I highly recommend that you use one of the latest updated version I sent. Fujii's version had some bugs, one of them being that as standbyModeRequested can be set to true if specified in postgresql.conf, a portion of the code using in xlog.c can be triggered even if ArchiveRecoveryRequested is not set to true. I also added fixes related to documentation. Comments from others are welcome. -- Michael
