Re: Fix for consume_xids advancing XIDs incorrectly

Hi,

Thank you for the report.

On Mon, Oct 14, 2024 at 6:51 PM Yushi Ogiwara
<btogiwarayuushi(at)oss(dot)nttdata(dot)com> wrote:
>
> Hi,
>
> I found that the consume_xids function incorrectly advances XIDs as
> shown:
>
> postgres=# select txid_current();
> txid_current
> --------------
> 746
> (1 row)
>
> postgres=# select consume_xids('100');
> consume_xids
> --------------
> 847
> (1 row)
>
> In the example, the consume_xids function consumes 100 XIDs when XID =
> 746, so the desired outcome from consume_xids should be 746 + 100 = 846,
> which differs from the actual outcome, 847.
>
> Behavior inside a transaction block:
>
> postgres=# select txid_current();
> txid_current
> --------------
> 1410
> (1 row)
>
> postgres=# begin;
> BEGIN
> postgres=*# select consume_xids('100');
> consume_xids
> --------------
> 1511
> (1 row)
> postgres=*# select consume_xids('100');
> consume_xids
> --------------
> 1521
> (1 row)
>
> Here, the first call inside the transaction block consumes 100+1 XIDs
> (incorrect), while the second call consumes exactly 100 XIDs (as
> expected)
>
> Summary:
>
> The function performs incorrectly when:
> - Outside of a transaction block
> - The first call inside a transaction block
> But works correctly when:
> - After the second call inside a transaction block
>
> The issue arises because consume_xids does not correctly count the
> consumed XIDs when it calls the GetTopTransactionId function, which
> allocates a new XID when none has been assigned.

I agree with your analysis.

I have one comment on the patch:

- (void) GetTopTransactionId();
+ if(!FullTransactionIdIsValid(GetTopFullTransactionIdIfAny()))
+ {
+ (void) GetTopTransactionId();
+ consumed++;
+ }

If we count this case as consumed too, I think we need to set the
returned value of GetTopTranasctionId() to lastxid. Because otherwise,
the return value when passing 1 to the function would be the latest
XID at the time but not the last XID consumed by the function. Passing
1 to this function is very unrealistic case, though.

Regards,

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