On Fri, Jul 08, 2022 at 08:20:09AM -0400, David Steele wrote:
> On 7/7/22 21:56, Kyotaro Horiguchi wrote:
>> Thinking RFC'ish, the meaning of "may" and "must" is significant in
>> this description. On the other hand it uses both "may" and "can" but
>> I thinkthat their difference is not significant or "can" there is
>> somewhat confusing. I think the "can" should be "may" here.
>
> +1.
Done.
>> And since "must" is emphasized, doesn't "may" also needs emphasis?
>
> I think emphasis only on must is fine.
Yeah, I wanted to emphasize the importance of returning false in this case.
Since it's okay to return true or false in the identical/persisted file
case, I didn't think it deserved emphasis.
> Nathan, I don't see the language about being sure to persist to storage
> here?
It's here:
When an archive library encounters a pre-existing file, it may return
true if the WAL file has identical contents to the pre-existing archive
and the pre-existing archive is fully persisted to storage.
Since you didn't catch it, I wonder if it needs improvement. At the very
least, perhaps we should note that one way to do the latter is to persist
it yourself before returning true, and we could point to basic_archive.c as
an example. However, I'm hesitant to make these docs too much more
complicated than they already are. WDYT?
--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From a32f95d3b129d386ce03c194bd6cddad2f92b76b Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandboss...@gmail.com>
Date: Thu, 7 Apr 2022 14:11:59 -0700
Subject: [PATCH v4 1/2] Reduce overhead of renaming archive status files.
Presently, archive status files are durably renamed from .ready to
.done to indicate that a file has been archived. Persisting this
rename to disk accounts for a significant amount of the overhead
associated with archiving. While durably renaming the file
prevents re-archiving in most cases, archive commands and libraries
must already gracefully handle attempts to re-archive the last
archived file after a crash (e.g., a crash immediately after
archive_command exits but before the server renames the status
file).
This change reduces the amount of overhead associated with
archiving by using rename() instead of durable_rename() to rename
the archive status files. As a consequence, the server is more
likely to attempt to re-archive files after a crash, but as noted
above, archive commands and modules are already expected to handle
this. It is also possible that the server will attempt to re-
archive files that have been removed or recycled, but the archiver
already handles this, too.
Author: Nathan Bossart
---
src/backend/postmaster/pgarch.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/backend/postmaster/pgarch.c b/src/backend/postmaster/pgarch.c
index 25e31c42e1..6ce361707d 100644
--- a/src/backend/postmaster/pgarch.c
+++ b/src/backend/postmaster/pgarch.c
@@ -745,7 +745,19 @@ pgarch_archiveDone(char *xlog)
StatusFilePath(rlogready, xlog, ".ready");
StatusFilePath(rlogdone, xlog, ".done");
- (void) durable_rename(rlogready, rlogdone, WARNING);
+
+ /*
+ * To avoid extra overhead, we don't durably rename the .ready file to
+ * .done. Archive commands and libraries must gracefully handle attempts
+ * to re-archive files (e.g., if the server crashes just before this
+ * function is called), so it should be okay if the .ready file reappears
+ * after a crash.
+ */
+ if (rename(rlogready, rlogdone) < 0)
+ ereport(WARNING,
+ (errcode_for_file_access(),
+ errmsg("could not rename file \"%s\" to \"%s\": %m",
+ rlogready, rlogdone)));
}
--
2.25.1
>From 54e1225173211062940ed4c8130ba97c192a99ec Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathandboss...@gmail.com>
Date: Thu, 7 Jul 2022 10:43:30 -0700
Subject: [PATCH v4 2/2] add note about re-archiving in docs
---
doc/src/sgml/backup.sgml | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/doc/src/sgml/backup.sgml b/doc/src/sgml/backup.sgml
index 73a774d3d7..04a1f94ad0 100644
--- 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
+ pre-existing file contains different contents than the WAL file being
+ archived, the archive library <emphasis>must</emphasis> return false.
+ </para>
+
+ <para>
+ The example command above for Unix avoids overwriting a pre-existing archive
+ by including a separate
<command>test</command> step. On some Unix platforms, <command>cp</command> has
switches such as <option>-i</option> that can be used to do the same thing
less verbosely, but you should not rely on these without verifying that
--
2.25.1
