Re: Crash after a call to pg_backup_start()

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Richard Guo <guofenglinux(at)gmail(dot)com>
Cc: Kyotaro Horiguchi <horikyota(dot)ntt(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-21 13:35:44
Message-ID: Y1KgMNnhbF8h8/T8@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 21, 2022 at 05:53:25PM +0800, Richard Guo wrote:
> Yeah, the two conditions could be both false. How about we update the
> comment a bit to emphasize this? Such as
>
> /* At most one of these conditions can be true */
> or
> /* 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);

Please note that I have not checked in details all the interactions
behind register_persistent_abort_backup_handler() before entering in
do_pg_backup_start() and the ERROR_CLEANUP block used in this
routine (just a matter of some elog(ERROR)s put here and there, for
example). Anyway, yes, both conditions can be false, and that's easy
to reproduce:
1) Do pg_backup_start().
2) Do pg_backup_stop().
3) Stop the session to kick do_pg_abort_backup()
4) Assert()-boom.
--
Michael

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Japin Li 2022-10-21 13:46:18 Re: parse partition strategy string in gram.y
Previous Message Robert Haas 2022-10-21 13:15:39 Re: cross-platform pg_basebackup