On Fri, Jul 08, 2022 at 09:54:50AM -0700, Nathan Bossart wrote: > Since it's okay to return true or false in the identical/persisted file > case, I didn't think it deserved emphasis.
I think returning false is not-okay: > --- a/doc/src/sgml/backup.sgml > +++ b/doc/src/sgml/backup.sgml > @@ -681,14 +681,28 @@ test ! -f > /mnt/server/archivedir/00000001000000A900000065 && cp pg_wal/0 > any pre-existing archive file. This is an important safety feature to > preserve the integrity of your archive in case of administrator error > (such as sending the output of two different servers to the same archive > - directory). > + directory). It is advisable to test your proposed archive library to > ensure > + that it does not overwrite an existing file. > </para> > > <para> > - It is advisable to test your proposed archive library to ensure that it > - indeed does not overwrite an existing file, <emphasis>and that it returns > - <literal>false</literal> in this case</emphasis>. > - The example command above for Unix ensures this by including a separate > + In rare cases, <productname>PostgreSQL</productname> may attempt to > + re-archive a WAL file that was previously archived. For example, if the > + system crashes before the server makes a durable record of archival > success, > + the server will attempt to archive the file again after restarting > (provided > + archiving is still enabled). When an archive library encounters a > + pre-existing file, it may return <literal>true</literal> if the WAL file > has > + identical contents to the pre-existing archive and the pre-existing > archive > + is fully persisted to storage. Alternatively, the archive library may > + return <literal>false</literal> anytime a pre-existing file is > encountered, > + but this will require manual action by an administrator to resolve. If a Inviting the administrator to resolve things is more dangerous than just returning true. I recommend making this text more opinionated and simpler: libraries must return true. Alternately, if some library has found a good reason to return false, this paragraph could give the reason. I don't know of such a reason, though. > + pre-existing file contains different contents than the WAL file being > + archived, the archive library <emphasis>must</emphasis> return false. > + </para>
