confusing archive_command example

Started by Josh Kupershmidtabout 16 years ago13 messagesdocs
Jump to latest
#1Josh Kupershmidt
schmiddy@gmail.com

Hi,
On the page:
http://www.postgresql.org/docs/current/interactive/continuous-archiving.html#BACKUP-ARCHIVING-WAL

an example archive_command of:
archive_command = 'cp -i %p /mnt/server/archivedir/%f </dev/null'

is given. Then, a few lines later, an example archive command of:
archive_command = 'test ! -f .../%f && cp %p .../%f'

is given. I think this second command would be more clear if it used
"/mnt/server/archivedir/" instead of "...", to tie in with the
previous archive_command. It took me a minute to figure out that the
three dots were supposed to be an ellipsis instead of a typo for the
parent directory "..".

Josh

#2Bruce Momjian
bruce@momjian.us
In reply to: Josh Kupershmidt (#1)
Re: confusing archive_command example

Josh Kupershmidt wrote:

Hi,
On the page:
http://www.postgresql.org/docs/current/interactive/continuous-archiving.html#BACKUP-ARCHIVING-WAL

an example archive_command of:
archive_command = 'cp -i %p /mnt/server/archivedir/%f </dev/null'

is given. Then, a few lines later, an example archive command of:
archive_command = 'test ! -f .../%f && cp %p .../%f'

is given. I think this second command would be more clear if it used
"/mnt/server/archivedir/" instead of "...", to tie in with the
previous archive_command. It took me a minute to figure out that the
three dots were supposed to be an ellipsis instead of a typo for the
parent directory "..".

I agree our use of cp -i </dev/null is a little too fancy not to be
explained in the docs, so I have done so with the attached, applied
patch.

As far as "...", those are used to show only the important changes in
the string, which I think is the right approach, but I did change the
line so the dots are not right up against slashes:

archive_command = 'test ! -f ... %f &amp;&amp; cp %p ... %f'

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

Attachments:

/rtmp/difftext/x-diffDownload+11-11
#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#2)
Re: confusing archive_command example

Bruce Momjian <bruce@momjian.us> writes:

As far as "...", those are used to show only the important changes in
the string, which I think is the right approach, but I did change the
line so the dots are not right up against slashes:
archive_command = 'test ! -f ... %f &amp;&amp; cp %p ... %f'

This is *not* an improvement, because it makes it look like the %f is a
separate command argument. Please revert if you can't think of
something better.

regards, tom lane

#4Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#3)
Re: confusing archive_command example

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

As far as "...", those are used to show only the important changes in
the string, which I think is the right approach, but I did change the
line so the dots are not right up against slashes:
archive_command = 'test ! -f ... %f &amp;&amp; cp %p ... %f'

This is *not* an improvement, because it makes it look like the %f is a
separate command argument. Please revert if you can't think of
something better.

What about:

archive_command = 'test ! -f ...%f &amp;&amp; cp %p ...%f'

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#4)
Re: confusing archive_command example

Bruce Momjian <bruce@momjian.us> writes:

What about:
archive_command = 'test ! -f ...%f &amp;&amp; cp %p ...%f'

Perhaps, though I still think this isn't an improvement over the
original.

regards, tom lane

#6Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#5)
Re: confusing archive_command example

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

What about:
archive_command = 'test ! -f ...%f &amp;&amp; cp %p ...%f'

Perhaps, though I still think this isn't an improvement over the
original.

His complaint was that .../%f looks like ../%f; is that a valid
concern? I have reverted the change. Also, should we be using test !
-e instead of -f?

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Bruce Momjian (#6)
Re: confusing archive_command example

Bruce Momjian <bruce@momjian.us> writes:

His complaint was that .../%f looks like ../%f; is that a valid
concern?

Well, it does look like it, I'm just not seeing an easy fix that makes
that better. I think the original suggestion was to turn it into a
concrete example by writing something like /mnt/archive/%f.

I have reverted the change. Also, should we be using test !
-e instead of -f?

No opinion.

regards, tom lane

#8Greg Sabino Mullane
greg@turnstep.com
In reply to: Bruce Momjian (#2)
Re: confusing archive_command example

-----BEGIN PGP SIGNED MESSAGE-----
Hash: RIPEMD160

As far as "...", those are used to show only the important changes in
the string, which I think is the right approach, but I did change the
line so the dots are not right up against slashes:

archive_command = 'test ! -f ... %f &amp;&amp; cp %p ... %f'

The triple dots need to be removed entirely, too confusing. Write
everything out.

- --
Greg Sabino Mullane greg@turnstep.com
End Point Corporation http://www.endpoint.com/
PGP Key: 0x14964AC8 201003312007
http://biglumber.com/x/web?pk=2529DF6AB8F79407E94445B4BC9B906714964AC8
-----BEGIN PGP SIGNATURE-----

iEYEAREDAAYFAkuz5AwACgkQvJuQZxSWSshGMACdEBRm5VScsv4w2fGTZTTr3b19
Zj8AoIFV9exrwQfO8t8MSwL0BXx41ZCU
=VP+w
-----END PGP SIGNATURE-----

#9Bruce Momjian
bruce@momjian.us
In reply to: Tom Lane (#7)
Re: confusing archive_command example

Tom Lane wrote:

Bruce Momjian <bruce@momjian.us> writes:

His complaint was that .../%f looks like ../%f; is that a valid
concern?

Well, it does look like it, I'm just not seeing an easy fix that makes
that better. I think the original suggestion was to turn it into a
concrete example by writing something like /mnt/archive/%f.

I have reverted the change. Also, should we be using test !
-e instead of -f?

No opinion.

Well -e tests for any type of file, while -f is only for regular files.
In practice, there should only be regular files in the archive
directory. But because we are always super-cautious, I changed it to
-e.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

#10Peter Eisentraut
peter_e@gmx.net
In reply to: Bruce Momjian (#6)
Re: confusing archive_command example

On ons, 2010-03-31 at 19:51 -0400, Bruce Momjian wrote:

Also, should we be using test ! -e instead of -f?

test -e is not portable.

#11Greg Smith
gsmith@gregsmith.com
In reply to: Peter Eisentraut (#10)
Re: confusing archive_command example

Peter Eisentraut wrote:

On ons, 2010-03-31 at 19:51 -0400, Bruce Momjian wrote:

Also, should we be using test ! -e instead of -f?

test -e is not portable.

Right; there are plenty of shells where "test -e" isn't implemented, I
believe some Solaris and/or HP-UX systems have that issue. The original
"test -f" was already the best example to give here.

As for the update Josh suggested, there are two issues here. The PITR
documentation at
http://developer.postgresql.org/pgdocs/postgres/continuous-archiving.html
starts with this example:

archive_command = 'cp -i %p /mnt/server/archivedir/%f </dev/null'

And then it used to show this one (now it has 'test -e'):

archive_command = 'test ! -f .../%f && cp %p .../%f'

I agree this is confusing. That second line should just read like this
to match the other examples:

archive_command = 'test ! -f /mnt/server/archivedir/%f && cp %p
/mnt/server/archivedir/%f'

And to revert the "test -e" change.

The third issue here, not mentioned yet, is that there's also an example
of setting up archive_command in its GUC documentation:

http://developer.postgresql.org/pgdocs/postgres/runtime-config-wal.html

This has drifted out of sync with the section being updated here this
week--its archive_command example isn't even using "-i". I think the
right thing to do here is to migrate the Windows example out of 18.5.3
from 24.3.1, then replace the existing examples in 18.5.3 to instead
suggest 24.3.1 has them. Given that all of this documentation giving
examples for how to correctly configure that parameter, like this cp
trivia, only appears in the PITR docs section, I don't see any reason to
duplicate that in the GUC area. Having the second, minimally documented
version of those in 18.5.3 just encourages people to use what we know is
a bad practice by providing them as examples.

--
Greg Smith 2ndQuadrant US Baltimore, MD
PostgreSQL Training, Services and Support
greg@2ndQuadrant.com www.2ndQuadrant.us

#12Bruce Momjian
bruce@momjian.us
In reply to: Peter Eisentraut (#10)
Re: confusing archive_command example

Peter Eisentraut wrote:

On ons, 2010-03-31 at 19:51 -0400, Bruce Momjian wrote:

Also, should we be using test ! -e instead of -f?

test -e is not portable.

Thanks, reverted

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

#13Bruce Momjian
bruce@momjian.us
In reply to: Greg Smith (#11)
Re: confusing archive_command example

Greg Smith wrote:

The third issue here, not mentioned yet, is that there's also an example
of setting up archive_command in its GUC documentation:

http://developer.postgresql.org/pgdocs/postgres/runtime-config-wal.html

This has drifted out of sync with the section being updated here this
week--its archive_command example isn't even using "-i". I think the
right thing to do here is to migrate the Windows example out of 18.5.3
from 24.3.1, then replace the existing examples in 18.5.3 to instead
suggest 24.3.1 has them. Given that all of this documentation giving
examples for how to correctly configure that parameter, like this cp
trivia, only appears in the PITR docs section, I don't see any reason to
duplicate that in the GUC area. Having the second, minimally documented
version of those in 18.5.3 just encourages people to use what we know is
a bad practice by providing them as examples.

Agreed. The attached applied patch removes the example from the
configure section.

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

Attachments:

/rtmp/difftext/x-diffDownload+7-15