MIME-Version: 1.0
References: <2a62f138c9dcb6fcf5889b076d123aa2@oss.nttdata.com>
 <CAD21AoATuDuqSr=V3vCSbHbOg6g9zKL61EEbe4ZPG3xOUa0Pcw@mail.gmail.com> <50ee82a96ac0935de88f9a5ef021f04d@oss.nttdata.com>
In-Reply-To: <50ee82a96ac0935de88f9a5ef021f04d@oss.nttdata.com>
From: Masahiko Sawada <sawada.mshk@gmail.com>
Date: Wed, 16 Oct 2024 10:58:50 -0700
Message-ID: <CAD21AoAr92L0Yd-vF908He4B94F0W6Y1XC2ZUpm3WWoCLvONBw@mail.gmail.com>
Subject: Re: Fix for consume_xids advancing XIDs incorrectly
To: Yushi Ogiwara <btogiwarayuushi@oss.nttdata.com>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: <https://www.postgresql.org/message-id/CAD21AoAr92L0Yd-vF908He4B94F0W6Y1XC2ZUpm3WWoCLvONBw%40mail.gmail.com>
Precedence: bulk

On Tue, Oct 15, 2024 at 10:06=E2=80=AFPM 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 =3D
> ReadNextFullTransactionId(), where the ReadNextFullTransactionId
> function returns the (current XID) + 1.

But it's possible that concurrent transactions consume XIDs in meanwhile, n=
o?

>
> Separately, I found that consume_xids(0) does not behave as expected.
> Below is an example:
>
> postgres=3D# select txid_current();
>   txid_current
> --------------
>          45496
> (1 row)
>
> postgres=3D# select consume_xids(0);
>   consume_xids
> --------------
>          45497
> (1 row)
>
> postgres=3D# 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=3D# select txid_current();
>   txid_current
> --------------
>          45498
> (1 row)
>
> postgres=3D# 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,

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