Re: Crash after a call to pg_backup_start()

On Sat, Oct 22, 2022 at 1:26 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> 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.

+        /* We should be here only by one of these reasons, never both */
+        Assert(during_backup_start ^
+               (sessionBackupState == SESSION_BACKUP_RUNNING));
+

What's the problem even if we're here when both of them are true? The
runningBackups is incremented anyways, right? Why can't we just get
rid of the Assert and treat during_backup_start as
backup_marked_active_in_shmem or something like that to keep things
simple?

--
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com