On Mon, Oct 24, 2022 at 11:39:19AM +0200, Alvaro Herrera wrote:
> Reading it again, I agree with your conclusion, so I'll push as you
> proposed with some extra tests, after they complete running.
Thanks for the fix, Álvaro!
--
Michael
signature.asc
Description: PGP signature
On 2022-Oct-24, Michael Paquier wrote:
> On the contrary, it seems to me that putting the assertion within the
> if() block makes the assertion weaker, because we would never check
> for an incorrect state after do_pg_abort_backup() is registered (aka
> any pg_backup_start() call) when not enterin
On Mon, Oct 24, 2022 at 11:42:58AM +0900, Kyotaro Horiguchi wrote:
> At Sat, 22 Oct 2022 09:56:06 +0200, Alvaro Herrera
> wrote in
>> 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
At Sat, 22 Oct 2022 09:56:06 +0200, Alvaro Herrera
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 fo
On Sat, Oct 22, 2022 at 1:56 PM Alvaro Herrera wrote:
>
> > 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?
>
> Why is that simpler?
IMO, the assertion looks complex there and thinking if
On 2022-Oct-22, Bharath Rupireddy wrote:
> +/* 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?
On Sat, Oct 22, 2022 at 1:26 PM Alvaro Herrera 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
>
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 ||
>
On Fri, Oct 21, 2022 at 7:06 PM Michael Paquier wrote:
>
> 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
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 d
On Fri, Oct 21, 2022 at 3:10 PM Kyotaro Horiguchi
wrote:
> It seems to me that the comment is true and the condition is a thinko.
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 */
While playing with a proposed patch, I noticed that a session crashes
after a failed call to pg_backup_start().
postgres=# select pg_backup_start(repeat('x', 1026));
ERROR: backup label too long (max 1024 bytes)
postgres=# \q
> TRAP: failed Assert("during_backup_start ^ (sessionBackupState ==
>
12 matches
Mail list logo
