Thread: Re: [PATCHES] Bug in WAL backup documentation
Bernd Helmle <mailings@oopsware.de> writes:
> Our WAL backup documentation says in some parts of it:
> ..."%p is replaced by the absolute path of the file to archive..." [1]
> I think this is (at least for 8.1 and upcoming 8.2 releases) wrong, since
> the archiver replaces this with pg_xlog/<LOGFILENAME> only,
Good point. Do we want to consider that this is a code bug rather than
a doc bug? The relative path is more efficient as long as the archiver
script doesn't do a "cd", but if it does then there'd be a problem.
You could argue that the code should be tweaked to continue supplying
an absolute path.
Since 8.1 has done this all along and no one's actually complained about
it, I guess no one is using scripts that do "cd". I'm inclined to go
with Bernd's suggestion to change the docs to match the code, but does
anyone have a contrary opinion?
regards, tom lane
On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: > Since 8.1 has done this all along and no one's actually complained about > it, I guess no one is using scripts that do "cd". I'm inclined to go > with Bernd's suggestion to change the docs to match the code, but does > anyone have a contrary opinion? Arguably you could give people a choice, say %P for the absolute path and %p for the relative one. In Unix you can easily prepend $PWD to the string, but I don't know how easy that is in Windows. Have a nice day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.
Tom Lane wrote: > Bernd Helmle <mailings@oopsware.de> writes: > Since 8.1 has done this all along and no one's actually complained about > it, I guess no one is using scripts that do "cd". I'm inclined to go > with Bernd's suggestion to change the docs to match the code, but does > anyone have a contrary opinion? I think supplying the absolute path makes archiving scripts less error-prone, which is a good time. So I'd vote for absolute paths. greetings, Florian Pflug
On Fri, 2006-11-03 at 17:34 +0100, Martijn van Oosterhout wrote: > On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: > > Since 8.1 has done this all along and no one's actually complained about > > it, I guess no one is using scripts that do "cd". I'm inclined to go > > with Bernd's suggestion to change the docs to match the code, but does > > anyone have a contrary opinion? > In Unix you can easily prepend $PWD to the > string, but I don't know how easy that is in Windows. Windows input anyone? Given the lack of a comprehensive test suite at this stage, I'd vote on the side of least change right now. We know the existing mechanism works, and as Martijn point out there is a workaround, plus as Tom discusses this would only happen if people "cd" which in my book would be bad programming form anyway. +1 Doc bug for 8.2, feature request for 8.3, unless Windows bites. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Simon Riggs wrote: > On Fri, 2006-11-03 at 17:34 +0100, Martijn van Oosterhout wrote: >> On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: >> > Since 8.1 has done this all along and no one's actually complained >> about >> > it, I guess no one is using scripts that do "cd". I'm inclined to go >> > with Bernd's suggestion to change the docs to match the code, but does >> > anyone have a contrary opinion? > >> In Unix you can easily prepend $PWD to the >> string, but I don't know how easy that is in Windows. > > Windows input anyone? > Of course you can get the current directory on Windows, if that's what the question is. cheers andrew
> > > Since 8.1 has done this all along and no one's actually > complained > > > about it, I guess no one is using scripts that do "cd". I'm > > > inclined to go with Bernd's suggestion to change the docs > to match > > > the code, but does anyone have a contrary opinion? > > > In Unix you can easily prepend $PWD to the string, but I don't know > > how easy that is in Windows. > > Windows input anyone? %CD% gives the same as $PWD in a command shell: C:\Program Files\Microsoft Visual Studio 8\VC>echo %CD% C:\Program Files\Microsoft Visual Studio 8\VC //Magnus
"Simon Riggs" <simon@2ndquadrant.com> writes:
>> On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote:
>>> Since 8.1 has done this all along and no one's actually complained about
>>> it, I guess no one is using scripts that do "cd". I'm inclined to go
>>> with Bernd's suggestion to change the docs to match the code, but does
>>> anyone have a contrary opinion?
> +1 Doc bug for 8.2, feature request for 8.3, unless Windows bites.
Looking back in the archives, I note that one of the arguments for
making the server use relative paths everywhere was so that it'd be
robust against things like DBAs moving directories that contain live
postmasters. If we provide a %P option, or otherwise encourage people
to write scripts that depend on the absolute path of $PGDATA, we'd lose
some of this robustness. So that might be an argument for leaving the
code as-is indefinitely ... not a very strong argument maybe, but it's
more than just we're-too-lazy-to-add-%P.
Anyway, I've corrected the documentation in HEAD and 8.1.
regards, tom lane
On Sat, 2006-11-04 at 13:29 -0500, Tom Lane wrote: > "Simon Riggs" <simon@2ndquadrant.com> writes: > >> On Fri, Nov 03, 2006 at 11:25:09AM -0500, Tom Lane wrote: > >>> Since 8.1 has done this all along and no one's actually complained about > >>> it, I guess no one is using scripts that do "cd". I'm inclined to go > >>> with Bernd's suggestion to change the docs to match the code, but does > >>> anyone have a contrary opinion? > > > +1 Doc bug for 8.2, feature request for 8.3, unless Windows bites. > > Looking back in the archives, I note that one of the arguments for > making the server use relative paths everywhere was so that it'd be > robust against things like DBAs moving directories that contain live > postmasters. If we provide a %P option, or otherwise encourage people > to write scripts that depend on the absolute path of $PGDATA, we'd lose > some of this robustness. So that might be an argument for leaving the > code as-is indefinitely ... not a very strong argument maybe, but it's > more than just we're-too-lazy-to-add-%P. > > Anyway, I've corrected the documentation in HEAD and 8.1. I think I can fulfil Bernd, Florian and Martijn's wishes by supplying an additional substitutable parameter %d which is replaced by the DataDir. This allows people to use an absolute directory if they wish, allows us to continue with the functionality of %p as-is and all without a possible confusion between %p and %P. It also allows %d to be used as an identifier which might be used to locate the appropriate archive for those with multiple servers without editing the archive_command for each of those servers. So using %d/%p will give you the absolute path for forward-slashers. Works for archive and recovery. Patch included, code and docs. Code comments now discuss relative paths also. Comments? -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Attachment
"Simon Riggs" <simon@2ndquadrant.com> writes:
> On Sat, 2006-11-04 at 13:29 -0500, Tom Lane wrote:
>> Looking back in the archives, I note that one of the arguments for
>> making the server use relative paths everywhere was so that it'd be
>> robust against things like DBAs moving directories that contain live
>> postmasters. If we provide a %P option, or otherwise encourage people
>> to write scripts that depend on the absolute path of $PGDATA, we'd lose
>> some of this robustness.
> I think I can fulfil Bernd, Florian and Martijn's wishes by supplying an
> additional substitutable parameter %d which is replaced by the DataDir.
This fails to respond to the concern that DataDir might be out-of-date.
regards, tom lane
On Sun, 2006-11-05 at 11:10 -0500, Tom Lane wrote: > "Simon Riggs" <simon@2ndquadrant.com> writes: > > On Sat, 2006-11-04 at 13:29 -0500, Tom Lane wrote: > >> Looking back in the archives, I note that one of the arguments for > >> making the server use relative paths everywhere was so that it'd be > >> robust against things like DBAs moving directories that contain live > >> postmasters. If we provide a %P option, or otherwise encourage people > >> to write scripts that depend on the absolute path of $PGDATA, we'd lose > >> some of this robustness. > > > I think I can fulfil Bernd, Florian and Martijn's wishes by supplying an > > additional substitutable parameter %d which is replaced by the DataDir. > > This fails to respond to the concern that DataDir might be out-of-date. I'm not suggesting that the option is necessary, but I am suggesting offering it to those who consider it useful. Let's allow it, but document the concern about its use in certain circumstances. I'm pretty sure most people don't move live postmasters very frequently, plus it isn't clear to me why we should support the people that want that to do that, yet not the people who want the absolute-path option. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
"Simon Riggs" <simon@2ndquadrant.com> writes:
> I'm pretty sure most people don't move live postmasters very frequently,
> plus it isn't clear to me why we should support the people that want
> that to do that, yet not the people who want the absolute-path option.
As already discussed upthread, anyone who wants the path can get it from
`pwd` or local equivalent --- and that mechanism is robust (as long as
the directory move doesn't happen while any particular instance of the
script is running). I don't see why we should go out of our way to
provide a bad substitute for pwd.
BTW, I note that some post-startup uses of DataDir have crept back in,
in places like utils/adt/dbsize.c. I'll be sure to clean those up
before release...
regards, tom lane
On Sun, 2006-11-05 at 11:49 -0500, Tom Lane wrote: > I don't see why we should go out of our way to > provide a bad substitute for pwd. That argument is conclusive. Agreed. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
On Sun, 2006-11-05 at 15:02 +0000, Simon Riggs wrote: > Code comments now discuss relative paths also. Patch containing just the minor cleanup of docs and code comments. -- Simon Riggs EnterpriseDB http://www.enterprisedb.com
Attachment
On Sun, Nov 05, 2006 at 11:49:36 -0500, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > As already discussed upthread, anyone who wants the path can get it from > `pwd` or local equivalent --- and that mechanism is robust (as long as > the directory move doesn't happen while any particular instance of the > script is running). I don't see why we should go out of our way to > provide a bad substitute for pwd. I think you also still need read access to the intervening directories. If the command works by walking up and matching inode numbers with names, then it will break if it can't read the names. (For example /bin/pwd breaks when it can't read a parent directories filenames.)
On Tue, Nov 07, 2006 at 07:11:35PM -0600, Bruno Wolff III wrote: > On Sun, Nov 05, 2006 at 11:49:36 -0500, > Tom Lane <tgl@sss.pgh.pa.us> wrote: > > > > As already discussed upthread, anyone who wants the path can get it from > > `pwd` or local equivalent --- and that mechanism is robust (as long as > > the directory move doesn't happen while any particular instance of the > > script is running). I don't see why we should go out of our way to > > provide a bad substitute for pwd. > > I think you also still need read access to the intervening directories. > If the command works by walking up and matching inode numbers with names, > then it will break if it can't read the names. (For example /bin/pwd > breaks when it can't read a parent directories filenames.) That's system dependant though, Linux getcwd doesn't have that problem for example. Should probably dig up some documention on which systems would be affected by this. Have a ncie day, -- Martijn van Oosterhout <kleptog@svana.org> http://svana.org/kleptog/ > From each according to his ability. To each according to his ability to litigate.