From: | Noah Misch <noah(at)leadboat(dot)com> |
---|---|
To: | Nathan Bossart <nathandbossart(at)gmail(dot)com> |
Cc: | David Steele <david(at)pgmasters(dot)net>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, robertmhaas(at)gmail(dot)com, masao(dot)fujii(at)oss(dot)nttdata(dot)com, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: remove more archiving overhead |
Date: | 2022-07-31 06:51:56 |
Message-ID: | 20220731065156.GA3694276@rfd.leadboat.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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>
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2022-07-31 11:17:55 | Re: Reducing the maintenance overhead of test_oat_hooks |
Previous Message | Noah Misch | 2022-07-31 06:17:47 | Re: Race between KeepFileRestoredFromArchive() and restartpoint |