On 2020/04/09 23:06, Fujii Masao wrote:
On 2020/04/09 2:35, Robert Haas wrote:
On Wed, Apr 8, 2020 at 1:15 AM Fujii Masao <masao.fu...@oss.nttdata.com> wrote:
When there is a backup_manifest in the database cluster, it's included in
the backup even when --no-manifest is specified. ISTM that this is problematic
because the backup_manifest is obviously not valid for the backup.
So, isn't it better to always exclude the *existing* backup_manifest in the
cluster from the backup, like backup_label/tablespace_map? Patch attached.
Also I found the typo in the document. Patch attached.
Both patches look good. The second one is definitely a mistake on my
part, and the first one seems like a totally reasonable change.
Thanks!
Thanks for reviewing them! I pushed them.
I found other minor issues.
+ When this option is specified with a value of <literal>yes</literal>
+ or <literal>force-escape</literal>, a backup manifest is created
force-escape should be force-encode.
Patch attached.
- while ((c = getopt_long(argc, argv,
"CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvP",
+ while ((c = getopt_long(argc, argv,
"CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvPm:",
"m:" seems unnecessary, so should be removed?
Patch attached.
+ if (strcmp(basedir, "-") == 0)
+ {
+ char header[512];
+ PQExpBufferData buf;
+
+ initPQExpBuffer(&buf);
+ ReceiveBackupManifestInMemory(conn, &buf);
backup_manifest should be received only when the manifest is enabled,
so ISTM that the flag "manifest" should be checked in the above if-condition.
Thought? Patch attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgml
index 536de9a698..87292739b5 100644
--- a/doc/src/sgml/protocol.sgml
+++ b/doc/src/sgml/protocol.sgml
@@ -2578,19 +2578,19 @@ The commands accepted in replication mode are:
</varlistentry>
<varlistentry>
- <term><literal>MANIFEST</literal></term>
+ <term><literal>MANIFEST</literal>
<replaceable>manifest_option</replaceable></term>
<listitem>
<para>
When this option is specified with a value of <literal>yes</literal>
- or <literal>force-escape</literal>, a backup manifest is created
+ or <literal>force-encode</literal>, a backup manifest is created
and sent along with the backup. The manifest is a list of every
file present in the backup with the exception of any WAL files that
may be included. It also stores the size, last modification time, and
an optional checksum for each file.
- A value of <literal>force-escape</literal> forces all filenames
+ A value of <literal>force-encode</literal> forces all filenames
to be hex-encoded; otherwise, this type of encoding is performed only
for files whose names are non-UTF8 octet sequences.
- <literal>force-escape</literal> is intended primarily for testing
+ <literal>force-encode</literal> is intended primarily for testing
purposes, to be sure that clients which read the backup manifest
can handle this case. For compatibility with previous releases,
the default is <literal>MANIFEST 'no'</literal>.
@@ -2599,7 +2599,7 @@ The commands accepted in replication mode are:
</varlistentry>
<varlistentry>
- <term><literal>MANIFEST_CHECKSUMS</literal></term>
+ <term><literal>MANIFEST_CHECKSUMS</literal>
<replaceable>checksum_algorithm</replaceable></term>
<listitem>
<para>
Specifies the algorithm that should be applied to each file included
diff --git a/src/bin/pg_basebackup/pg_basebackup.c
b/src/bin/pg_basebackup/pg_basebackup.c
index de098b3558..f1af8f904a 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -2271,7 +2271,7 @@ main(int argc, char **argv)
atexit(cleanup_directories_atexit);
- while ((c = getopt_long(argc, argv,
"CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvPm:",
+ while ((c = getopt_long(argc, argv,
"CD:F:r:RS:T:X:l:nNzZ:d:c:h:p:U:s:wWkvP",
long_options,
&option_index)) != -1)
{
switch (c)
diff --git a/src/bin/pg_basebackup/pg_basebackup.c
b/src/bin/pg_basebackup/pg_basebackup.c
index f1af8f904a..65ca1b16f0 100644
--- a/src/bin/pg_basebackup/pg_basebackup.c
+++ b/src/bin/pg_basebackup/pg_basebackup.c
@@ -1211,7 +1211,7 @@ ReceiveTarFile(PGconn *conn, PGresult *res, int rownum)
* we're writing a tarfile to stdout, we don't have that option, so
* include it in the one tarfile we've got.
*/
- if (strcmp(basedir, "-") == 0)
+ if (strcmp(basedir, "-") == 0 && manifest)
{
char header[512];
PQExpBufferData buf;
