Re: Crash after a call to pg_backup_start()

From: Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>
To: alvherre(at)alvh(dot)no-ip(dot)org
Cc: michael(at)paquier(dot)xyz, guofenglinux(at)gmail(dot)com, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: Crash after a call to pg_backup_start()
Date: 2022-10-24 02:42:58
Message-ID: 20221024.114258.1877173487836497015.horikyota.ntt@gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At Sat, 22 Oct 2022 09:56:06 +0200, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> wrote in
> On 2022-Oct-21, Michael Paquier wrote:
>
> > On Fri, Oct 21, 2022 at 05:53:25PM +0800, Richard Guo wrote:
>
> > > /* These conditions can not be both true */
> >
> > If you do that, it would be a bit easier to read as of the following
> > assertion instead? Like:
> > Assert(!during_backup_start ||
> > sessionBackupState == SESSION_BACKUP_NONE);
>
> My intention here was that the Assert should be inside the block, that
> is, we already know that at least one is true, and we want to make sure
> that they are not *both* true.
>
> AFAICT the attached patch also fixes the bug without making the assert
> weaker.

I'm fine with either of them, but..

The reason for that works the same way is that the if() block excludes
the case of (!during_backup_start && S_B_RUNNING)<*1>. In other words
the strictness is a kind of illusion [*a]. Actually the assertion does
not detect the case <*1>. In this regard, moving the current
assertion into the if() block might be confusing.

regards,

<*1>: It's evidently superfluous but "strictness" and "illusion" share
the exactly the same pronounciation in Japanese "Ghen-Ka-Ku".

--
Kyotaro Horiguchi
NTT Open Source Software Center

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Julien Rouhaud 2022-10-24 02:44:26 Re: cross-platform pg_basebackup
Previous Message Peter Smith 2022-10-24 01:53:16 Re: [DOCS] Stats views and functions not in order?