Re: Fix for consume_xids advancing XIDs incorrectly

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Yushi Ogiwara <btogiwarayuushi(at)oss(dot)nttdata(dot)com>
Cc: Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Fix for consume_xids advancing XIDs incorrectly
Date: 2024-10-16 17:58:50
Message-ID: CAD21AoAr92L0Yd-vF908He4B94F0W6Y1XC2ZUpm3WWoCLvONBw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Tue, Oct 15, 2024 at 10:06 PM Yushi Ogiwara
<btogiwarayuushi(at)oss(dot)nttdata(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shubham Khanna 2024-10-16 17:59:25 Re: Pgoutput not capturing the generated columns
Previous Message Shubham Khanna 2024-10-16 17:54:56 Re: Pgoutput not capturing the generated columns