From: Fujii Masao <masao.fujii@oss.nttdata.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: David Steele <david@pgmasters.net>
Cc: Andres Freund <andres@anarazel.de>
Cc: Noah Misch <noah@leadboat.com>
Cc: Stephen Frost <sfrost@snowman.net>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Suraj Kharage <suraj.kharage@enterprisedb.com>
Cc: tushar <tushar.ahuja@enterprisedb.com>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>
Cc: Rushabh Lathia <rushabh.lathia@gmail.com>
Cc: Tels <nospam-pg-abuse@bloodgate.com>
Cc: Andrew Dunstan <andrew.dunstan@2ndquadrant.com>
Cc: PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Cc: Jeevan Chalke <jeevan.chalke@enterprisedb.com>
Cc: vignesh C <vignesh21@gmail.com>
Subject: Re: backup manifests
Date: Mon, 13 Apr 2020 11:09:34 +0900
Message-ID: <01e3ed3a-8729-5aaa-ca84-e60e3ca59db8@oss.nttdata.com> (raw)
In-Reply-To: <777d236e-f46c-8099-bb5e-a6e4efc43999@oss.nttdata.com>
References: <20200330185944.42bxysvem6b757ew@alap3.anarazel.de>
<CA+TgmoawEeE5qpFgj5Vy2zZGKzd3ZSEhGrD_JdPqPd2GB8u1Cw@mail.gmail.com>
<20200331225034.odt3l2w4h3dr6ggk@alap3.anarazel.de>
<CA+TgmobV+kW5cBCa8_LGvytdOH71gnpqDsmKBR7bghEFgR7Y1w@mail.gmail.com>
<CA+Tgmob+xDcvEUTznkvianyJzKK8nUM6bWfkrnZQkL-XEa3NEA@mail.gmail.com>
<20200402172318.3kvfrrewxmemzpia@alap3.anarazel.de>
<CA+Tgmoa2_8Ti9xez8wEH7Z_RJQv=cwPC=MeqUXcdVW=5-uMCig@mail.gmail.com>
<20200402182346.6iffoadxu2hsbi2s@alap3.anarazel.de>
<CA+TgmoboTL8cBt_P5Jx6RjL=Hk3Y8PbpGsh9KcH_MZuswVqdYA@mail.gmail.com>
<CA+TgmoaGoQM74CMJhqg3_M7osWgkT2xSxo9aGVu9E4Nt+8Z4sg@mail.gmail.com>
<20200402194750.7ze74b4t6b6o4cui@alap3.anarazel.de>
<e3101c23-26b7-6fa2-14a8-af24c96602b5@pgmasters.net>
<CA+TgmoaKhxPr0FWsEiZoXpMER44pMJ28L-Z7wcRrOKoGFwjiUQ@mail.gmail.com>
<78f76a3d-1a28-a97d-0394-5c96985dd1c0@oss.nttdata.com>
<CA+TgmobHMS6z2A5Sa+C7WD86bDjKOt6fbmueoWyoQa3kEVgbtA@mail.gmail.com>
<777d236e-f46c-8099-bb5e-a6e4efc43999@oss.nttdata.com>
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.fujii@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.sgmlindex 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.cindex 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.cindex 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;
Attachments:
[text/plain] fix_typo_in_protocol_sgml.patch (2.0K, ../01e3ed3a-8729-5aaa-ca84-e60e3ca59db8@oss.nttdata.com/2-fix_typo_in_protocol_sgml.patch)
download | inline diff:diff --git a/doc/src/sgml/protocol.sgml b/doc/src/sgml/protocol.sgmlindex 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
[text/plain] remove_unnecessary_getopt_option.patch (532B, ../01e3ed3a-8729-5aaa-ca84-e60e3ca59db8@oss.nttdata.com/3-remove_unnecessary_getopt_option.patch)
download | inline diff:diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.cindex 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)
[text/plain] receive_manifest_only_when_enabled.patch (543B, ../01e3ed3a-8729-5aaa-ca84-e60e3ca59db8@oss.nttdata.com/4-receive_manifest_only_when_enabled.patch)
download | inline diff:diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.cindex 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;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: masao.fujii@oss.nttdata.com, robertmhaas@gmail.com, david@pgmasters.net, andres@anarazel.de, noah@leadboat.com, sfrost@snowman.net, amit.kapila16@gmail.com, suraj.kharage@enterprisedb.com, tushar.ahuja@enterprisedb.com, rajkumar.raghuwanshi@enterprisedb.com, rushabh.lathia@gmail.com, nospam-pg-abuse@bloodgate.com, andrew.dunstan@2ndquadrant.com, jeevan.chalke@enterprisedb.com, vignesh21@gmail.com
Subject: Re: backup manifests
In-Reply-To: <01e3ed3a-8729-5aaa-ca84-e60e3ca59db8@oss.nttdata.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox