Re: Fix for consume_xids advancing XIDs incorrectly

On Tue, Oct 15, 2024 at 10:06 PM Yushi Ogiwara
<btogiwarayuushi@oss.nttdata.com> wrote:
>
> Hi,
>
> Thank you for your comment.
>
> Regarding the first patch, I believe it works correctly when
> consume_xids(1) is called.  This is because the lastxid variable in the
> consume_xids_common function is initialized as lastxid =
> ReadNextFullTransactionId(), where the ReadNextFullTransactionId
> function returns the (current XID) + 1.

But it's possible that concurrent transactions consume XIDs in meanwhile, no?

>
> Separately, I found that consume_xids(0) does not behave as expected.
> Below is an example:
>
> postgres=# select txid_current();
>   txid_current
> --------------
>          45496
> (1 row)
>
> postgres=# select consume_xids(0);
>   consume_xids
> --------------
>          45497
> (1 row)
>
> postgres=# select consume_xids(0);
>   consume_xids
> --------------
>          45497
> (1 row)
>
> In the example, the argument to consume_xids is 0, meaning it should not
> consume any XIDs. However, the first invocation of consume_xids(0) looks
> like unexpectedly consuming 1 XID though it's not consuming actually.
> This happens because consume_xids(0) returns the value from
> ReadNextFullTransactionId.
>
> I have updated the patch (skip.diff, attached to this e-mail) to address
> this issue. Now, when consume_xids(0) is called, it returns
> ReadNextFullTransactionId().value - 1, ensuring no XID is consumed as
> shown below:
>
> postgres=# select txid_current();
>   txid_current
> --------------
>          45498
> (1 row)
>
> postgres=# select consume_xids(0);
>   consume_xids
> --------------
>          45498
> (1 row)

Hmm, I think if we expect this function to return the last XID that
the function actually consumed, calling consume_xids with 0 should
raise an error instead. Even if it returns
ReadNextFullTransactionId().value - 1 as you proposed, other
concurrent transactions might consume XIDs between txid_current() and
consume_xids(0), resulting in consume_xids() appearing to have
consumed XIDs.

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com