pgsql: Allow external recovery_config_directory
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data directory.
Server needs read/write permissions on this directory.
Branch
------
master
Details
-------
http://git.postgresql.org/pg/commitdiff/bc5334d8679c428a709d150666b288171795bd76
Modified Files
--------------
doc/src/sgml/config.sgml | 17 +++++++++++++++++
src/backend/access/transam/xlog.c | 18 +++++++++++-------
src/backend/utils/init/globals.c | 1 +
src/backend/utils/init/miscinit.c | 19 +++++++++++++++++++
src/backend/utils/misc/guc.c | 24 ++++++++++++++++++++++++
src/include/miscadmin.h | 2 ++
src/include/utils/guc.h | 1 +
7 files changed, 75 insertions(+), 7 deletions(-)
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On Wed, Mar 27, 2013 at 12:47 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data directory.
Server needs read/write permissions on this directory.
Should we consider verifying that it has write permissions and fail
early if not?
AFAICT, right now we fail *after* doing a bunch of things if the user
only has readonly permissions?
I'm sure a common thing will be for people to try to set this to
/etc/postgresql or similar...
--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 27.03.2013 13:47, Simon Riggs wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data directory.
Server needs read/write permissions on this directory.
This was a surprising commit. Does this get us any closer to merging
postgresql.conf and recovery.conf?
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Mar 27, 2013 at 9:12 PM, Heikki Linnakangas <hlinnakangas@vmware.com
wrote:
On 27.03.2013 13:47, Simon Riggs wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data
directory.
Server needs read/write permissions on this directory.This was a surprising commit. Does this get us any closer to merging
postgresql.conf and recovery.conf?
At first glance, I am not sure this goes in the right direction. Why is it
necessary to add a GUC parameter for that?
In the patch I sent based on Masao's first version, if we merge of
postgresql.conf and recovery.conf, users will be encouraged to migrate to
the new system by including recovery.conf or a file containing recovery
parameters using include_path or include_if_exists, so you shouldn't need a
new parameter to include recovery.conf. I have the feeling that this
complicates even more the settings.
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.
--
Michael
On Wed, Mar 27, 2013 at 9:59 PM, Michael Paquier
<michael.paquier@gmail.com>wrote:
On Wed, Mar 27, 2013 at 9:12 PM, Heikki Linnakangas <
hlinnakangas@vmware.com> wrote:On 27.03.2013 13:47, Simon Riggs wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data
directory.
Server needs read/write permissions on this directory.This was a surprising commit. Does this get us any closer to merging
postgresql.conf and recovery.conf?At first glance, I am not sure this goes in the right direction. Why is it
necessary to add a GUC parameter for that?
In the patch I sent based on Masao's first version, if we merge of
postgresql.conf and recovery.conf, users will be encouraged to migrate to
the new system by including recovery.conf or a file containing recovery
parameters using include_path or include_if_exists, so you shouldn't need a
new parameter to include recovery.conf. I have the feeling that this
complicates even more the settings.
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.
Please note also that based on the documentation include* params can used
absolute paths:
http://www.postgresql.org/docs/devel/static/config-setting.html#CONFIG-INCLUDES
--
Michael
On 27 March 2013 12:12, Heikki Linnakangas <hlinnakangas@vmware.com> wrote:
On 27.03.2013 13:47, Simon Riggs wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data
directory.
Server needs read/write permissions on this directory.This was a surprising commit. Does this get us any closer to merging
postgresql.conf and recovery.conf?
Why so? I made a clear proposal on how to proceed and this was the
first part of it.
--
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
On 27 March 2013 12:59, Michael Paquier <michael.paquier@gmail.com> wrote:
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.
I'm following what was agreed on 24 December.
We can have the whole debate again, if you wish. There is no reason to
break backwards compatibility to get what we want.
--
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
On 27 March 2013 12:02, Magnus Hagander <magnus@hagander.net> wrote:
On Wed, Mar 27, 2013 at 12:47 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data directory.
Server needs read/write permissions on this directory.Should we consider verifying that it has write permissions and fail
early if not?
Sounds like a good plan. Will do.
AFAICT, right now we fail *after* doing a bunch of things if the user
only has readonly permissions?I'm sure a common thing will be for people to try to set this to
/etc/postgresql or similar...
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 27 March 2013 11:47, Simon Riggs <simon@2ndquadrant.com> wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data directory.
Server needs read/write permissions on this directory.
The docs don't build due to this commit. Please remove the unpaired
"<variablelist>" from line 333 of config.sgml.
--
Thom
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On Wed, Mar 27, 2013 at 9:11 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 27 March 2013 12:59, Michael Paquier <michael.paquier@gmail.com> wrote:
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.I'm following what was agreed on 24 December.
I assume that you are referring to this message:
/messages/by-id/CA+U5nMK8n=sQ-xPvBVtiCS3NbVObjUVM5xBR+FAeQN-RjjGxSQ@mail.gmail.com
I don't see a followup from anyone clearly agreeing that this was a
useful thing to do. There is a lot of support for turning
recovery.conf parameters into GUCs. But I don't remember anyone
supporting this idea, and like Heikki and Michael, I don't understand
how it moves the ball forward.
Considering there's been no discussion of this particular change in
three months, and not a whole lot back then, I think it would have
been polite to post the patch and ask for comments before committing
it.
--
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
On 27 March 2013 13:21, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Mar 27, 2013 at 9:11 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 27 March 2013 12:59, Michael Paquier <michael.paquier@gmail.com> wrote:
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.I'm following what was agreed on 24 December.
I assume that you are referring to this message:
/messages/by-id/CA+U5nMK8n=sQ-xPvBVtiCS3NbVObjUVM5xBR+FAeQN-RjjGxSQ@mail.gmail.com
I don't see a followup from anyone clearly agreeing that this was a
useful thing to do.
Please look again.
There is a lot of support for turning
recovery.conf parameters into GUCs.
Who is against it? I am not. Even, I am working on it now, as already
said in at least 3 different places.
But I don't remember anyone
supporting this idea, and like Heikki and Michael, I don't understand
how it moves the ball forward.Considering there's been no discussion of this particular change in
three months, and not a whole lot back then, I think it would have
been polite to post the patch and ask for comments before committing
it.
Given various confusions and multiple patches, posting another
wouldn't help much.
In terms of politeness, I certainly mean no rudeness, only to move
forward as agreed.
--
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
On 27 March 2013 13:14, Thom Brown <thom@linux.com> wrote:
On 27 March 2013 11:47, Simon Riggs <simon@2ndquadrant.com> wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data directory.
Server needs read/write permissions on this directory.The docs don't build due to this commit. Please remove the unpaired
"<variablelist>" from line 333 of config.sgml.
Thanks
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 27.03.2013 15:11, Simon Riggs wrote:
On 27 March 2013 12:59, Michael Paquier<michael.paquier@gmail.com> wrote:
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.I'm following what was agreed on 24 December.
Well, there wasn't much discussion about it back then. The way I read
the thread is that people agreed with the general approach, as now
implemented in Michael's patch, based on Fujii's earlier patch. This
might be a good idea or not, but it's a new and separate feature, not
related to whatever else we might do with recovery.conf.
If we are to discuss the merits of this patch now, a few thoughts:
1. This is going to make life more complicated for tools that want to
mess with recovery.conf, as it's no longer guaranteed to be in $PGDATA.
2. An admin can no longer tell if a server is in standby or PITR mode
just by checking for $PGDATA/recovery.conf
3. Would it make sense to make the option "recovery_config_file",
pointing to the file, instead of just the directory?
4. Could you achieve the same with a symlink in $PGDATA?
We can have the whole debate again, if you wish. There is no reason to
break backwards compatibility to get what we want.
AFAICS this is completely orthogonal to backwards-compatibility and
other aspects of the upcoming patch to merge recovery.conf and
postgresql.conf.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On Wed, Mar 27, 2013 at 9:40 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 27 March 2013 13:21, Robert Haas <robertmhaas@gmail.com> wrote:
On Wed, Mar 27, 2013 at 9:11 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 27 March 2013 12:59, Michael Paquier <michael.paquier@gmail.com> wrote:
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.I'm following what was agreed on 24 December.
I assume that you are referring to this message:
/messages/by-id/CA+U5nMK8n=sQ-xPvBVtiCS3NbVObjUVM5xBR+FAeQN-RjjGxSQ@mail.gmail.com
I don't see a followup from anyone clearly agreeing that this was a
useful thing to do.Please look again.
I have a better idea: how about if you tell me where you see any such
message? Because I think the reason I don't see it is because it
doesn't exist.
It's not my job to go back and scour the archives for evidence that
there is some consensus around this commit. It's your job to provide
some evidence that such a consensus exists. Or else revert the
commit, because so far no one but you seems to believe that this was a
good idea. The fact that nobody specifically objected to one line in
an email message you posted three months ago does not constitute
grounds to go change it without so much as posting the patch. Or at
least I can't imagine that any other committer would take it that way.
Even Tom posts his patches before committing them, unless there's
been specific and recent discussion of the topic. What gives you the
right to do otherwise?
--
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
On 03/27/2013 07:47 AM, Simon Riggs wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data directory.
Server needs read/write permissions on this directory.
This patch appears to have broken Windows builds on the buildfarm.
cheers
andrew
--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers
On 27 March 2013 13:53, Robert Haas <robertmhaas@gmail.com> wrote:
Please look again.
I have a better idea: how about if you tell me where you see any such
message? Because I think the reason I don't see it is because it
doesn't exist.
/messages/by-id/20130109204225.GB21747@momjian.us
/messages/by-id/20121224171529.GB19778@alap2.fritz.box
--
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
On Wed, Mar 27, 2013 at 10:06 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 27 March 2013 13:53, Robert Haas <robertmhaas@gmail.com> wrote:
Please look again.
I have a better idea: how about if you tell me where you see any such
message? Because I think the reason I don't see it is because it
doesn't exist./messages/by-id/20130109204225.GB21747@momjian.us
/messages/by-id/20121224171529.GB19778@alap2.fritz.box
I don't think the first one can be read as a message of support for
this change, but I agree that the second one can.
--
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
On Wed, Mar 27, 2013 at 10:11 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
On 27 March 2013 12:59, Michael Paquier <michael.paquier@gmail.com> wrote:
Also, based on Greg's spec (that Robert and I basically agreed on), if
recovery.conf is found at the root of data folder an error is returned to
user, recommending him to migrate correctly by referring to dedicated
documentation.I'm following what was agreed on 24 December.
We can have the whole debate again, if you wish. There is no reason to
break backwards compatibility to get what we want.
OK here is an idea I just got: why not replacing the possibility to define
a custom repository for recovery.conf by the possibility to define a custom
*file*?
Here would be the plan:
1) we move all the recovery parameters to GUC as proposed Masao's patch
(and somewhat my patch now).
2) as default, the custom file that is used to trigger recovery is called
recovery.conf and needs to be located in data folder. This could be the
default value used by this feature.
3) When migrating to the new system, we recommend users to include
recovery.conf with include_if_exists. Even better, we add by default an
include_if_exists during initdb in postgresql.conf to include recovery.conf.
If we do that users don't even need to perform any migration operation.
You don't even need to maintain two parsing interfaces for recovery
parameters, and you don't even need to think about things like which file
has the priority on the other.
So happy end.
--
Michael
On Wed, Mar 27, 2013 at 10:43 PM, Heikki Linnakangas <
hlinnakangas@vmware.com> wrote:
3. Would it make sense to make the option "recovery_config_file", pointing
to the file, instead of just the directory?
+1 on that. I just sent the same suggestion.
--
MichaelOn 27.03.2013 15:09, Simon Riggs wrote:
On 27 March 2013 12:12, Heikki Linnakangas<hlinnakangas@vmware.com> wrote:
On 27.03.2013 13:47, Simon Riggs wrote:
Allow external recovery_config_directory
If required, recovery.conf can now be located outside of the data
directory.
Server needs read/write permissions on this directory.This was a surprising commit. Does this get us any closer to merging
postgresql.conf and recovery.conf?Why so? I made a clear proposal on how to proceed and this was the
first part of it.
You didn't answer the question. Does this get us any closer to merging
postgresql.conf and recovery.conf? Why is this bundled in with that?
ISTM the quickest way forward is to revert this, and proceed with the
rest of the plan: get Michael/Fujii's patch into shape, and commit it.
If we still think this additional GUC is a good idea after that, we can
add it afterwards just as well.
- Heikki
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
