On Sat, Oct 22, 2022 at 1:26 PM Alvaro Herrera <alvhe...@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
