Message-ID: <c56647fa-4306-7170-17be-d91b627d22f0@oss.nttdata.com>
Date: Wed, 20 Jul 2022 14:00:00 +0900
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.11.0
Subject: Re: Backup command and functions can cause assertion failure and
 segmentation fault
Content-Language: en-US
To: Michael Paquier <michael@paquier.xyz>
Cc: Robert Haas <robertmhaas@gmail.com>,
 Masahiko Sawada <sawada.mshk@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
References: <3374718f-9fbf-a950-6d66-d973e027f44c@oss.nttdata.com>
 <CAD21AoBkR611M6K+6i-xNDjmCd-=hFN_svNgRtvuezyjviw1sA@mail.gmail.com>
 <6a759108-a02c-34fd-856c-d7c09d29d2ba@oss.nttdata.com>
 <Yr6XJg2QDT1aHYyJ@paquier.xyz>
 <fa0f1057-923e-ad61-e6f4-ba5fc29135ec@oss.nttdata.com>
 <YsYkPzSpnr5Yt3uA@paquier.xyz>
 <bef8abcd-8cb8-1320-9f2b-bc9677413625@oss.nttdata.com>
 <CA+TgmoYSp060HUuGWbKrZjP+X6kb7dgRiwZZz-hzVEDLkDhZDQ@mail.gmail.com>
 <Ys/NCI4Eo9300GnQ@paquier.xyz>
 <02347f81-304f-3a14-1a33-5f40dfd7da48@oss.nttdata.com>
 <YtIkE12Ur3zjG91y@paquier.xyz>
From: Fujii Masao <masao.fujii@oss.nttdata.com>
In-Reply-To: <YtIkE12Ur3zjG91y@paquier.xyz>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Archived-At: 
 <https://www.postgresql.org/message-id/c56647fa-4306-7170-17be-d91b627d22f0%40oss.nttdata.com>
Precedence: bulk


On 2022/07/16 11:36, Michael Paquier wrote:
> I was thinking about doing that only on HEAD.  One thing interesting
> about this patch is that it can also be used as a point of reference
> for other future things.

Ok, here are review comments:

+my $connstr =
+  $node->connstr('postgres') . " replication=database dbname=postgres";

Since the result of connstr() includes "dbname=postgres", you don't need to add "dbname=postgres" again.

+# The psql command should fail on pg_stop_backup().

Typo: s/pg_stop_backup/pg_stop_backup

I reported two trouble cases; they are the cases where BASE_BACKUP is canceled and terminated, respectively. But you added the test only for one of them. Is this intentional?

>> Since one of them failed to be applied to v14 or before cleanly, I
>> also created the patch for those back branches. So I attached three
>> patches.
> 
> Fine by me.

I pushed these bugfix patches at first. Thanks!

Regards,

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