Обсуждение: BUG #15335: Documentation is wrong about archive_command and existingfiles

The following bug has been logged on the website:

Bug reference:      15335
Logged by:          Phil Endecott
Email address:      spam_from_pgsql_lists@chezphil.org
PostgreSQL version: 9.6.10
Operating system:   Debian Stretch
Description:

The docs in section 25.3.1 say that archive_command should check if the
target file already exists and fail in that case.  It seems that this is not
entirely true; the command should succeed if the target file already exists
and its content is the same as the source.
This is explicitly mentioned in section 26.2.9 for the case of cascaded
replication with a shared archive, but I understand that this is actually
needed in all cases.  I encountered this during a failed attempt at
promotion, but there are likely to be other cases.  Quoting David Steele
from the -general mailing list:

"Duplicate WAL is possible in *all* cases. A trivial example is that
Postgres calls archive_command and it succeeds but an error happens (e.g.
network) right before Postgres is notified. It will wait a bit and try the
same WAL segment again."

Note that the example archive commands in the documentation (using cp) get
this wrong.  Minimal examples of archive commands that do this check
correctly would be very useful.

(I worry that the non-WAL files that archive_command and restore_command are
also invoked for, e.g. the .backup and .history files, have some additional
or possibly even conflicting requirements.)

Greetings,

* PG Bug reporting form (noreply@postgresql.org) wrote:
> The docs in section 25.3.1 say that archive_command should check if the
> target file already exists and fail in that case.  It seems that this is not
> entirely true; the command should succeed if the target file already exists
> and its content is the same as the source.
> This is explicitly mentioned in section 26.2.9 for the case of cascaded
> replication with a shared archive, but I understand that this is actually
> needed in all cases.  I encountered this during a failed attempt at
> promotion, but there are likely to be other cases.  Quoting David Steele
> from the -general mailing list:
>
> "Duplicate WAL is possible in *all* cases. A trivial example is that
> Postgres calls archive_command and it succeeds but an error happens (e.g.
> network) right before Postgres is notified. It will wait a bit and try the
> same WAL segment again."

So, I agree with all of the above.

> Note that the example archive commands in the documentation (using cp) get
> this wrong.  Minimal examples of archive commands that do this check
> correctly would be very useful.

I agree that the example command in the documentation which uses 'cp'
gets that wrong- it gets quite a few other things wrong too, really.

This is what the problem is though- there really isn't any way to have a
reasonable and *correct* "minimal" example of an archive command.  To do
this correctly, you'd really need to compare (or checksum) the file
that's in the archive with the file that's being proposed to be added to
the archive and then say everything is fine if they match, and throw an
error if they don't.  Of course, as just discussed elsewhere, you should
really also be checking the system-ID in the WAL files against the
system-ID of the WAL archive to make sure you aren't archiving WAL files
from one system into the archive of another (or across a pg_upgrade),
and further, you should be fsync'ing the WAL file after it's been copied
before telling PG that it's been saved...  Sadly, accomplishing all of
that in a one-line shell command is not really feasible.

> (I worry that the non-WAL files that archive_command and restore_command are
> also invoked for, e.g. the .backup and .history files, have some additional
> or possibly even conflicting requirements.)

Not really sure what you're getting at here.

Thanks!

Stephen

Hi Jeff,

Jeff Janes wrote:
> The possibilities are endless, and an example cannot cover all 
> of them while still usefully serving as an example. There are 
> canned systems for handling this more thoroughly, and you should 
> look into using one of them.

Do you believe that the docs should point this out?

Currently there are about 18,000 words of what appears to be very
thorough documentation about implementing replication, yet they
do not refer the reader to tools like pgBackRest.  When I post
about the issues that I have had, the overwhelming view of the
mailing lists is that pgBackRest is essential - yet this important
advice is not conveyed to new users of this feature who are relying
on the official documentation.


Regards, Phil.

On Thu, Aug 16, 2018 at 04:10:00PM +0000, PG Bug reporting form wrote:
> Note that the example archive commands in the documentation (using cp) get
> this wrong.  Minimal examples of archive commands that do this check
> correctly would be very useful.

This example is wrong for other reasons, one being that archive_command
should sync the archived file before returning its result back to the
backend to make it durable.

> (I worry that the non-WAL files that archive_command and restore_command are
> also invoked for, e.g. the .backup and .history files, have some additional
> or possibly even conflicting requirements.)

Conflicting requirements depend on your cluster and archiving strategy.
Overwriting a WAL segment archived is usually wrong.
--
Michael

On 8/18/18 8:09 PM, Jeff Janes wrote:
> On Sat, Aug 18, 2018 at 12:51 PM, Phil Endecott
> <spam_from_pgsql_lists@chezphil.org
> <mailto:spam_from_pgsql_lists@chezphil.org>> wrote:
> 
>     Hi Jeff,
> 
>     Jeff Janes wrote:
> 
>         The possibilities are endless, and an example cannot cover all
>         of them while still usefully serving as an example. There are
>         canned systems for handling this more thoroughly, and you should
>         look into using one of them.
> 
>     Do you believe that the docs should point this out?
> 
>     Currently there are about 18,000 words of what appears to be very
>     thorough documentation about implementing replication, yet they
>     do not refer the reader to tools like pgBackRest.  When I post
>     about the issues that I have had, the overwhelming view of the
>     mailing lists is that pgBackRest is essential - yet this important
>     advice is not conveyed to new users of this feature who are relying
>     on the official documentation.
> 
> 
> I think it might be a good idea to propose use of a canned solution, but
> I don't know how the community would feel about mentioning specific
> projects by name.  And if we didn't mention any by name, I think it
> would be pretty awkward to advise to give.
There is a wiki page that lists the major backup solutions:

https://wiki.postgresql.org/wiki/Binary_Replication_Tools

But I don't think we ever reference the wiki from the core user
documentation.

We could at least list all the things that a good archive command should
do and point out that the example in the docs doesn't do them and that
it is intended *only* as an example.

Regards,
-- 
-David
david@pgmasters.net

Greetings,

* David Steele (david@pgmasters.net) wrote:
> On 8/18/18 8:09 PM, Jeff Janes wrote:
> > I think it might be a good idea to propose use of a canned solution, but
> > I don't know how the community would feel about mentioning specific
> > projects by name.  And if we didn't mention any by name, I think it
> > would be pretty awkward to advise to give.
> There is a wiki page that lists the major backup solutions:
>
> https://wiki.postgresql.org/wiki/Binary_Replication_Tools

Interesting, though seems like there should be a distinction made
between "backup" tools and "replication" tools, they're certainly
different things.

> But I don't think we ever reference the wiki from the core user
> documentation.

Very rarely.

> We could at least list all the things that a good archive command should
> do and point out that the example in the docs doesn't do them and that
> it is intended *only* as an example.

This sounds like a good idea to me, in general.  I suggest we qualify
the command shown further and say it's only provided as an illustration
of how to set an archive_command and that it isn't an example.  Perhaps
we should even remove the shell-script bits and instead just have:

archive_command = 'archivecmd /mnt/server/archivedir/%f %p'

As long as we have something there that looks like valid shell script
and which doesn't obviously fail, people are likely going to continue to
use it.

Thanks!

Stephen

On 8/20/18 9:24 AM, Stephen Frost wrote:
> * David Steele (david@pgmasters.net) wrote:
> 
>> We could at least list all the things that a good archive command should
>> do and point out that the example in the docs doesn't do them and that
>> it is intended *only* as an example.
> 
> This sounds like a good idea to me, in general.  I suggest we qualify
> the command shown further and say it's only provided as an illustration
> of how to set an archive_command and that it isn't an example.  Perhaps
> we should even remove the shell-script bits and instead just have:
> 
> archive_command = 'archivecmd /mnt/server/archivedir/%f %p'

+1

> 
> As long as we have something there that looks like valid shell script
> and which doesn't obviously fail, people are likely going to continue to
> use it.

OK -- I'll work up a doc patch that lists the basic things an archive 
command should do and see where that leads us.

Regards,
-- 
-David
david@pgmasters.net