[Fujii Masao <masao.fujii@oss.nttdata.com>]

On 2020/04/04 4:22, Robert Haas wrote:
> On Thu, Apr 2, 2020 at 4:34 PM David Steele <david@pgmasters.net> wrote:
>> +1. These would be great tests to have and a win for pg_basebackup
>> overall but I don't think they should be a prerequisite for this commit.
> 
> Not to mention the server. I can't say that I have a lot of confidence
> that all of the server behavior in this area is well-understood and
> sane.
> 
> I've pushed all the patches.

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.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

Attachments: download all (2 files, .tar.gz)

skip_backup_manifest.patch (text/plain, 1.3K) download
diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index 5d94b9c229..de77534fec 100644
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -266,6 +266,13 @@ static const struct exclude_list_item excludeFiles[] =
 	{BACKUP_LABEL_FILE, false},
 	{TABLESPACE_MAP, false},
 
+	/*
+	 * If there's a backup_manifest, it belongs to a backup that was used
+	 * to start this server. It is *not* correct for this backup. Our
+	 * backup_manifest is injected into the backup separately if users want it.
+	 */
+	{"backup_manifest", false},
+
 	{"postmaster.pid", false},
 	{"postmaster.opts", false},
 
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 9088f1f80f..daaefa751c 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -113,6 +113,13 @@ static const struct exclude_list_item excludeFiles[] =
 	{"backup_label", false},	/* defined as BACKUP_LABEL_FILE */
 	{"tablespace_map", false},	/* defined as TABLESPACE_MAP */
 
+	/*
+	 * If there's a backup_manifest, it belongs to a backup that was used
+	 * to start this server. It is *not* correct for this backup. Our
+	 * backup_manifest is injected into the backup separately if users want it.
+	 */
+	{"backup_manifest", false},
+
 	{"postmaster.pid", false},
 	{"postmaster.opts", false},
 


typo.patch (text/plain, 675B) download
diff --git a/doc/src/sgml/ref/pg_validatebackup.sgml b/doc/src/sgml/ref/pg_validatebackup.sgml
index 19888dc196..20d445efb8 100644
--- a/doc/src/sgml/ref/pg_validatebackup.sgml
+++ b/doc/src/sgml/ref/pg_validatebackup.sgml
@@ -132,7 +132,7 @@ PostgreSQL documentation
       <listitem>
        <para>
         Exit as soon as a problem with the backup is detected. If this option
-        is not specified, <literal>pg_basebackup</literal> will continue
+        is not specified, <literal>pg_validatebackup</literal> will continue
         checking the backup even after a problem has been detected, and will
         report all problems detected as errors.
        </para>