Thread: BUG #15335: Documentation is wrong about archive_command and existingfiles
BUG #15335: Documentation is wrong about archive_command and existingfiles
From
PG Bug reporting form
Date:
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.)
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
Stephen Frost
Date:
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
Attachment
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
Jeff Janes
Date:
On Thu, Aug 16, 2018 at 12:10 PM, PG Bug reporting form <noreply@postgresql.org> wrote:
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."
Sure, that is possible. But it is also possible that the network error caused the file to be short, but an identical prefix of what it is supposed to be. Or maybe it was corrupted by the error, and so not even an identical prefix. 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.
Cheers,
Jeff
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
"Phil Endecott"
Date:
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.
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
Jeff Janes
Date:
On Sat, Aug 18, 2018 at 12:51 PM, Phil Endecott <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.
Cheers,
Jeff
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
Michael Paquier
Date:
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
Attachment
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
David Steele
Date:
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
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
Stephen Frost
Date:
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
Attachment
Re: BUG #15335: Documentation is wrong about archive_command andexisting files
From
David Steele
Date:
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