pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
10+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2024-02-08 165d921c9a e3e05addee 503299b7f7 18388291aa 4efaf4b09e d44060cfcc Fix wrong logic in TransactionIdInRecentPast()
* BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
@ 2023-11-24 06:28 PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: PG Bug reporting form @ 2023-11-24 06:28 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: kyzevan23@mail.ru
The following bug has been logged on the website:
Bug reference: 18212
Logged by: Egor Chindyaskin
Email address: kyzevan23@mail.ru
PostgreSQL version: 16.1
Operating system: Ubuntu 23.10
Description:
Hello! Recently, Me and Andrew Bille came across a situation where the
txid_status() and pg_xact_status() functions return invalid status of the
specified transaction if xid is very far ahead.
The following script reproduces the situation:
# This script uses pg_resetwal and dd. This is a hack to move xid far
ahead
PGDATA=/tmp/pgsql/data
killall postgres
rm -rf $PGDATA
initdb -U postgres -k -D $PGDATA
pg_ctl -D $PGDATA -l log start
psql -U postgres -c "UPDATE pg_database SET datallowconn = 't' WHERE datname
= 'template0';"
pg_ctl -D $PGDATA -l log stop
pg_resetwal -x 536870911 -l 000000010000000000000004 $PGDATA
dd if=/dev/zero of=$PGDATA/pg_xact/01FF bs=8192 count=2048
pg_ctl -D $PGDATA -l log start
vacuumdb --all -U postgres
pg_ctl -D $PGDATA -l log stop
pg_resetwal -x 1073741822 -l 000000010000000000000004 $PGDATA
dd if=/dev/zero of=$PGDATA/pg_xact/03FF bs=8192 count=2048
pg_ctl -D $PGDATA -l log start
vacuumdb --all -U postgres
pg_ctl -D $PGDATA -l log stop
pg_resetwal -x 1610612733 -l 000000010000000000000004 $PGDATA
dd if=/dev/zero of=$PGDATA/pg_xact/05FF bs=8192 count=2048
pg_ctl -D $PGDATA -l log start
vacuumdb --all -U postgres
pg_ctl -D $PGDATA -l log stop
pg_resetwal -x 2147484382 -l 000000010000000000000004 $PGDATA
dd if=/dev/zero of=$PGDATA/pg_xact/0800 bs=8192 count=2048
pg_ctl -D $PGDATA -l log start
vacuumdb --all -U postgres
psql -U postgres -c "SELECT txid_current()"
psql -U postgres -c "SELECT txid_status(3)" -c "SELECT
pg_xact_status('3'::xid8)"
As a result, we get that, instead of the expected NULL, the functions return
the "in progress" status of transaction 3, which is far in the past:
...
txid_current
--------------
2147484382
(1 row)
txid_status
-------------
in progress
(1 row)
pg_xact_status
----------------
in progress
(1 row)
---
Best regards,
Egor Chindyaskin
Postgres Professional: http://postgrespro.com
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
@ 2023-11-24 11:38 ` Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Karina Litskevich @ 2023-11-24 11:38 UTC (permalink / raw)
To: kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
Hi,
The problem here is in TransactionIdInRecentPast() function. If
ShmemVariableCache->oldestClogXid had been stored as a FullTransactionId,
this function would be very simple for normal transaction ids: ids between
oldestClogXid and next full transaction id are in the recent past (return
true), ids before oldestClogXid are too far in the past (return false),
ids after next full xid are in the future (raise an error). Expected
results are demonstrated in the following picture, where vertical bar is
used to separate different epochs.
|pppppppppppppppp|pppppppppppprrff|ffffffffffffffff|
^ ^
| |
| next fxid
|
oldestClogXid
The problem is that oldestClogXid is a TransactionId, so it's epoch is not
stored, and the following condition is wrong:
if (xid_epoch + 1 < now_epoch
|| (xid_epoch + 1 == now_epoch && xid < now_epoch_next_xid)
|| TransactionIdPrecedes(xid, ShmemVariableCache->oldestClogXid))
return false;
First, given full transaction id (xid_epoch, xid) is compared to the next
full transaction id (now_epoch, now_epoch_next_xid), and if it's more than
one epoch older, it's far in the past. For newer ids, it's transaction id
part without an epoch is compared to oldestClogXid, but it's a modulo-2^32
comparison, so only 2^31 xids before oldestClogXid are considered preceding
it. Thus, all full transaction ids between (next full transaction id - 2^32)
and (oldestClogXid - 2^31) are mistakenly considered to be in the recent
past.
|pppppppppppppprr|rrrrpppppppprrff|ffffffffffffffff|
^ ^
| |
| next fxid
|
oldestClogXid
In the attached patch I suggest calculating an epoch for oldestClogXid
assuming it's not much older than next full transaction id, make a full
transaction id for oldestClogXid, and then just compare it to the given
full transaction id.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
Attachments:
v1-0001-Fix-TransactionIdInRecentPast.patch (text/x-patch, 2.4K) download
From 4239e9524d9c82f1f0c4f11b4078206b193a065a Mon Sep 17 00:00:00 2001
From: Karina Litskevich <litskevichkarina@gmail.com>
Date: Tue, 21 Nov 2023 15:22:52 +0300
Subject: [PATCH v1] Fix TransactionIdInRecentPast()
---
src/backend/utils/adt/xid8funcs.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index 06ae940df6..6ce560a721 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -98,11 +98,12 @@ StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP,
static bool
TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
{
- uint32 xid_epoch = EpochFromFullTransactionId(fxid);
TransactionId xid = XidFromFullTransactionId(fxid);
uint32 now_epoch;
TransactionId now_epoch_next_xid;
FullTransactionId now_fullxid;
+ TransactionId oldest_xid;
+ FullTransactionId oldest_fxid;
now_fullxid = ReadNextFullTransactionId();
now_epoch_next_xid = XidFromFullTransactionId(now_fullxid);
@@ -135,17 +136,22 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
Assert(LWLockHeldByMe(XactTruncationLock));
/*
- * If the transaction ID has wrapped around, it's definitely too old to
- * determine the commit status. Otherwise, we can compare it to
- * ShmemVariableCache->oldestClogXid to determine whether the relevant
- * CLOG entry is guaranteed to still exist.
+ * If fxid is not older than ShmemVariableCache->oldestClogXid, the relevant
+ * CLOG entry is guaranteed to still exist. To compare them correctly make
+ * FullTransactionId from ShmemVariableCache->oldestClogXid. Determine the
+ * right epoch knowing that oldest_fxid should be not much older than
+ * now_fullxid.
*/
- if (xid_epoch + 1 < now_epoch
- || (xid_epoch + 1 == now_epoch && xid < now_epoch_next_xid)
- || TransactionIdPrecedes(xid, ShmemVariableCache->oldestClogXid))
- return false;
-
- return true;
+ oldest_xid = ShmemVariableCache->oldestClogXid;
+ Assert(TransactionIdPrecedes(oldest_xid, now_epoch_next_xid));
+ if (oldest_xid < now_epoch_next_xid)
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch, oldest_xid);
+ else
+ {
+ Assert(now_epoch > 0);
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch - 1, oldest_xid);
+ }
+ return !FullTransactionIdPrecedes(fxid, oldest_fxid);
}
/*
--
2.34.1
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
@ 2023-11-28 05:20 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Kyotaro Horiguchi @ 2023-11-28 05:20 UTC (permalink / raw)
To: litskevichkarina@gmail.com; +Cc: kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
Good catch!
At Fri, 24 Nov 2023 14:38:23 +0300, Karina Litskevich <litskevichkarina@gmail.com> wrote in
> one epoch older, it's far in the past. For newer ids, it's transaction id
> part without an epoch is compared to oldestClogXid, but it's a modulo-2^32
> comparison, so only 2^31 xids before oldestClogXid are considered preceding
> it. Thus, all full transaction ids between (next full transaction id - 2^32)
> and (oldestClogXid - 2^31) are mistakenly considered to be in the recent
> past.
I'm not entirely sure about the specific range where the error occurs,
but I do believe that the second and third lines of the problematic
comparison seem to be invalid. It seems like there is confusion
between the epoch boundary of full XIDs and the wraparound boundary of
32-bit XIDs.
> In the attached patch I suggest calculating an epoch for oldestClogXid
> assuming it's not much older than next full transaction id, make a full
> transaction id for oldestClogXid, and then just compare it to the given
> full transaction id.
I considered bringing this down to a comparison of 32-bit XIDs, but
couldn't come up with a clean method. Therefore, using full XID seems
to be the right approach. However, it seems like there is an error in
the XID comparison condition. There are cases where oldest_xid and
now_epoch_next_xid can have the same value. If we skip running
txid_current() in the repro in the your previous mail, and directly
execute txid_status(3), it would lead to assertion failure.
Also, I feel the comments could be more straight forward and simple
like this:
> Convert oldest_xid into a full XID to compare with the given
> XID. Alghouth it's guaranteed that the the oldest and newest XIDs
> are within the XID wraparound distance, they may have different
> epochs.
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
@ 2023-11-30 05:30 ` Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 13:51 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Karina Litskevich @ 2023-11-30 05:30 UTC (permalink / raw)
To: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; +Cc: kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
On Tue, Nov 28, 2023 at 8:20 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com>
wrote:
> I considered bringing this down to a comparison of 32-bit XIDs, but
> couldn't come up with a clean method. Therefore, using full XID seems
> to be the right approach. However, it seems like there is an error in
> the XID comparison condition. There are cases where oldest_xid and
> now_epoch_next_xid can have the same value. If we skip running
> txid_current() in the repro in the your previous mail, and directly
> execute txid_status(3), it would lead to assertion failure.
>
Thank you for your feedback! You're right, I haven't thought about
this corner case. Fixed in v2.
Also, I feel the comments could be more straight forward and simple
> like this:
>
> > Convert oldest_xid into a full XID to compare with the given
> > XID. Alghouth it's guaranteed that the the oldest and newest XIDs
> > are within the XID wraparound distance, they may have different
> > epochs.
>
I tried to improve the comment too. Anyway, don't hesitate to change
it to whatever you like better.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
Attachments:
v2-0001-Fix-TransactionIdInRecentPast.patch (text/x-patch, 2.4K) download
From 338b02d02ee78904ec726fa43f0c093331c2234f Mon Sep 17 00:00:00 2001
From: Karina Litskevich <litskevichkarina@gmail.com>
Date: Tue, 28 Nov 2023 13:21:15 +0300
Subject: [PATCH v2] Fix TransactionIdInRecentPast()
---
src/backend/utils/adt/xid8funcs.c | 28 +++++++++++++++++-----------
1 file changed, 17 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index 06ae940df6..f2a37b2708 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -98,11 +98,12 @@ StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP,
static bool
TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
{
- uint32 xid_epoch = EpochFromFullTransactionId(fxid);
TransactionId xid = XidFromFullTransactionId(fxid);
uint32 now_epoch;
TransactionId now_epoch_next_xid;
FullTransactionId now_fullxid;
+ TransactionId oldest_xid;
+ FullTransactionId oldest_fxid;
now_fullxid = ReadNextFullTransactionId();
now_epoch_next_xid = XidFromFullTransactionId(now_fullxid);
@@ -135,17 +136,22 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
Assert(LWLockHeldByMe(XactTruncationLock));
/*
- * If the transaction ID has wrapped around, it's definitely too old to
- * determine the commit status. Otherwise, we can compare it to
- * ShmemVariableCache->oldestClogXid to determine whether the relevant
- * CLOG entry is guaranteed to still exist.
+ * If fxid is not older than ShmemVariableCache->oldestClogXid, the relevant
+ * CLOG entry is guaranteed to still exist. Convert oldestClogXid into a
+ * full XID to compare it with fxid. Determine the right epoch knowing that
+ * the oldest and the newest XIDs are guaranteed to be within the XID
+ * wraparound distance.
*/
- if (xid_epoch + 1 < now_epoch
- || (xid_epoch + 1 == now_epoch && xid < now_epoch_next_xid)
- || TransactionIdPrecedes(xid, ShmemVariableCache->oldestClogXid))
- return false;
-
- return true;
+ oldest_xid = ShmemVariableCache->oldestClogXid;
+ Assert(TransactionIdPrecedesOrEquals(oldest_xid, now_epoch_next_xid));
+ if (oldest_xid <= now_epoch_next_xid)
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch, oldest_xid);
+ else
+ {
+ Assert(now_epoch > 0);
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch - 1, oldest_xid);
+ }
+ return !FullTransactionIdPrecedes(fxid, oldest_fxid);
}
/*
--
2.34.1
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
@ 2024-02-05 13:51 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-02-05 14:35 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Korotkov @ 2024-02-05 13:51 UTC (permalink / raw)
To: Karina Litskevich <litskevichkarina@gmail.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
Hi!
On Thu, Nov 30, 2023 at 7:30 AM Karina Litskevich
<litskevichkarina@gmail.com> wrote:
> On Tue, Nov 28, 2023 at 8:20 AM Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>>
>> I considered bringing this down to a comparison of 32-bit XIDs, but
>> couldn't come up with a clean method. Therefore, using full XID seems
>> to be the right approach. However, it seems like there is an error in
>> the XID comparison condition. There are cases where oldest_xid and
>> now_epoch_next_xid can have the same value. If we skip running
>> txid_current() in the repro in the your previous mail, and directly
>> execute txid_status(3), it would lead to assertion failure.
>
>
> Thank you for your feedback! You're right, I haven't thought about
> this corner case. Fixed in v2.
>
>> Also, I feel the comments could be more straight forward and simple
>> like this:
>>
>> > Convert oldest_xid into a full XID to compare with the given
>> > XID. Alghouth it's guaranteed that the the oldest and newest XIDs
>> > are within the XID wraparound distance, they may have different
>> > epochs.
>
>
> I tried to improve the comment too. Anyway, don't hesitate to change
> it to whatever you like better.
Thank you for your work on the subject.
I've slightly revised the patch (attached). I'm going to push it if
there are no objections.
------
Regards,
Alexander Korotkov
Attachments:
0001-Fix-wrong-logic-in-TransactionIdInRecentPast-v3.patch (application/octet-stream, 3.1K) download
From d682ecc4f67b43c30028827f2143ca1aa47132b1 Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Mon, 5 Feb 2024 15:40:49 +0200
Subject: [PATCH] Fix wrong logic in TransactionIdInRecentPast()
The TransactionIdInRecentPast() should return false for all the transactions
older than TransamVariables->oldestClogXid. However, the function contains
a bug in comparison FullTransactionId to TransactionID allowing full
transactions between nextXid - 2^32 and oldestClogXid - 2^31.
This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into
FullTransactionId first, then performing the comparison.
Backpatch to all supported versions.
Reported-by: Egor Chindyaskin
Bug: 18212
Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org
Author: Karina Litskevich
Reviewed-by: Kyotaro Horiguchi
Backpatch-through: 12
---
src/backend/utils/adt/xid8funcs.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index be5e28c93ab..827392b2a04 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -98,11 +98,12 @@ StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP,
static bool
TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
{
- uint32 xid_epoch = EpochFromFullTransactionId(fxid);
TransactionId xid = XidFromFullTransactionId(fxid);
uint32 now_epoch;
TransactionId now_epoch_next_xid;
FullTransactionId now_fullxid;
+ TransactionId oldest_xid;
+ FullTransactionId oldest_fxid;
now_fullxid = ReadNextFullTransactionId();
now_epoch_next_xid = XidFromFullTransactionId(now_fullxid);
@@ -135,17 +136,24 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
Assert(LWLockHeldByMe(XactTruncationLock));
/*
- * If the transaction ID has wrapped around, it's definitely too old to
- * determine the commit status. Otherwise, we can compare it to
- * TransamVariables->oldestClogXid to determine whether the relevant CLOG
- * entry is guaranteed to still exist.
+ * If fxid is not older than TransamVariables->oldestClogXid, the relevant
+ * CLOG entry is guaranteed to still exist. To compare them correctly
+ * make FullTransactionId from TransamVariables->oldestClogXid. Determine
+ * the right epoch knowing that oldest_fxid shouldn't be more than 2^31
+ * older than now_fullxid.
*/
- if (xid_epoch + 1 < now_epoch
- || (xid_epoch + 1 == now_epoch && xid < now_epoch_next_xid)
- || TransactionIdPrecedes(xid, TransamVariables->oldestClogXid))
- return false;
-
- return true;
+ oldest_xid = TransamVariables->oldestClogXid;
+ Assert(TransactionIdPrecedes(oldest_xid, now_epoch_next_xid));
+ if (oldest_xid < now_epoch_next_xid)
+ {
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch, oldest_xid);
+ }
+ else
+ {
+ Assert(now_epoch > 0);
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch - 1, oldest_xid);
+ }
+ return !FullTransactionIdPrecedes(fxid, oldest_fxid);
}
/*
--
2.39.3 (Apple Git-145)
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 13:51 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
@ 2024-02-05 14:35 ` Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 14:50 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Karina Litskevich @ 2024-02-05 14:35 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
On Mon, Feb 5, 2024 at 4:51 PM Alexander Korotkov <aekorotkov@gmail.com>
wrote:
> I've slightly revised the patch (attached). I'm going to push it if
> there are no objections.
>
Thank you very much!
It looks like you based your v3 patch on the v1. Have you noticed Kyotaro's
comments and my corrections in the v2 patch?
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 13:51 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
2024-02-05 14:35 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
@ 2024-02-05 14:50 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-02-06 12:58 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Alexander Korotkov @ 2024-02-05 14:50 UTC (permalink / raw)
To: Karina Litskevich <litskevichkarina@gmail.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
Hi, Karina!
On Mon, Feb 5, 2024 at 4:35 PM Karina Litskevich
<litskevichkarina@gmail.com> wrote:
> On Mon, Feb 5, 2024 at 4:51 PM Alexander Korotkov <aekorotkov@gmail.com> wrote:
>>
>> I've slightly revised the patch (attached). I'm going to push it if
>> there are no objections.
>
>
> Thank you very much!
>
> It looks like you based your v3 patch on the v1. Have you noticed Kyotaro's
> comments and my corrections in the v2 patch?
Thank you for pointing this out. Please, take a look at my v4 of the patch.
------
Regards,
Alexander Korotkov
Attachments:
0001-Fix-wrong-logic-in-TransactionIdInRecentPast-v4.patch (application/octet-stream, 3.1K) download
From 699556521354246cf29a6f74546beeb4829edfcd Mon Sep 17 00:00:00 2001
From: Alexander Korotkov <akorotkov@postgresql.org>
Date: Mon, 5 Feb 2024 15:40:49 +0200
Subject: [PATCH] Fix wrong logic in TransactionIdInRecentPast()
The TransactionIdInRecentPast() should return false for all the transactions
older than TransamVariables->oldestClogXid. However, the function contains
a bug in comparison FullTransactionId to TransactionID allowing full
transactions between nextXid - 2^32 and oldestClogXid - 2^31.
This commit fixes TransactionIdInRecentPast() by turning the oldestClogXid into
FullTransactionId first, then performing the comparison.
Backpatch to all supported versions.
Reported-by: Egor Chindyaskin
Bug: 18212
Discussion: https://postgr.es/m/18212-547307f8adf57262%40postgresql.org
Author: Karina Litskevich
Reviewed-by: Kyotaro Horiguchi
Backpatch-through: 12
---
src/backend/utils/adt/xid8funcs.c | 30 +++++++++++++++++++-----------
1 file changed, 19 insertions(+), 11 deletions(-)
diff --git a/src/backend/utils/adt/xid8funcs.c b/src/backend/utils/adt/xid8funcs.c
index be5e28c93ab..aa64a7c8b3f 100644
--- a/src/backend/utils/adt/xid8funcs.c
+++ b/src/backend/utils/adt/xid8funcs.c
@@ -98,11 +98,12 @@ StaticAssertDecl(MAX_BACKENDS * 2 <= PG_SNAPSHOT_MAX_NXIP,
static bool
TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
{
- uint32 xid_epoch = EpochFromFullTransactionId(fxid);
TransactionId xid = XidFromFullTransactionId(fxid);
uint32 now_epoch;
TransactionId now_epoch_next_xid;
FullTransactionId now_fullxid;
+ TransactionId oldest_xid;
+ FullTransactionId oldest_fxid;
now_fullxid = ReadNextFullTransactionId();
now_epoch_next_xid = XidFromFullTransactionId(now_fullxid);
@@ -135,17 +136,24 @@ TransactionIdInRecentPast(FullTransactionId fxid, TransactionId *extracted_xid)
Assert(LWLockHeldByMe(XactTruncationLock));
/*
- * If the transaction ID has wrapped around, it's definitely too old to
- * determine the commit status. Otherwise, we can compare it to
- * TransamVariables->oldestClogXid to determine whether the relevant CLOG
- * entry is guaranteed to still exist.
+ * If fxid is not older than TransamVariables->oldestClogXid, the relevant
+ * CLOG entry is guaranteed to still exist. Convert
+ * TransamVariables->oldestClogXid into a FullTransactionId to compare it
+ * with fxid. Determine the right epoch knowing that oldest_fxid
+ * shouldn't be more than 2^31 older than now_fullxid.
*/
- if (xid_epoch + 1 < now_epoch
- || (xid_epoch + 1 == now_epoch && xid < now_epoch_next_xid)
- || TransactionIdPrecedes(xid, TransamVariables->oldestClogXid))
- return false;
-
- return true;
+ oldest_xid = TransamVariables->oldestClogXid;
+ Assert(TransactionIdPrecedesOrEquals(oldest_xid, now_epoch_next_xid));
+ if (oldest_xid <= now_epoch_next_xid)
+ {
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch, oldest_xid);
+ }
+ else
+ {
+ Assert(now_epoch > 0);
+ oldest_fxid = FullTransactionIdFromEpochAndXid(now_epoch - 1, oldest_xid);
+ }
+ return !FullTransactionIdPrecedes(fxid, oldest_fxid);
}
/*
--
2.39.3 (Apple Git-145)
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 13:51 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
2024-02-05 14:35 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 14:50 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
@ 2024-02-06 12:58 ` Karina Litskevich <litskevichkarina@gmail.com>
2024-03-07 13:17 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Maxim Orlov <orlovmg@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Karina Litskevich @ 2024-02-06 12:58 UTC (permalink / raw)
To: Alexander Korotkov <aekorotkov@gmail.com>; +Cc: Kyotaro Horiguchi <horikyota.ntt@gmail.com>; kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
On Mon, Feb 5, 2024 at 5:50 PM Alexander Korotkov <aekorotkov@gmail.com>
wrote:
> Thank you for pointing this out. Please, take a look at my v4 of the
> patch.
>
Looks good to me.
Best regards,
Karina Litskevich
Postgres Professional: http://postgrespro.com/
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 13:51 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
2024-02-05 14:35 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 14:50 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
2024-02-06 12:58 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
@ 2024-03-07 13:17 ` Maxim Orlov <orlovmg@gmail.com>
2024-03-07 13:34 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 1 reply; 10+ messages in thread
From: Maxim Orlov @ 2024-03-07 13:17 UTC (permalink / raw)
To: Karina Litskevich <litskevichkarina@gmail.com>; +Cc: Alexander Korotkov <aekorotkov@gmail.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
AFAICS, patch is committed around a month ago.
commit 165d921c9a883814a35e8161fc692793e9c945a4
Author: Alexander Korotkov <akorotkov@postgresql.org>
Date: Thu Feb 8 12:45:26 2024 +0200
So, I've mark this thread committed.
--
Best regards,
Maxim Orlov.
^ permalink raw reply [nested|flat] 10+ messages in thread
* Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 13:51 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
2024-02-05 14:35 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 14:50 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Alexander Korotkov <aekorotkov@gmail.com>
2024-02-06 12:58 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Karina Litskevich <litskevichkarina@gmail.com>
2024-03-07 13:17 ` Re: BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction Maxim Orlov <orlovmg@gmail.com>
@ 2024-03-07 13:34 ` Alexander Korotkov <aekorotkov@gmail.com>
0 siblings, 0 replies; 10+ messages in thread
From: Alexander Korotkov @ 2024-03-07 13:34 UTC (permalink / raw)
To: Maxim Orlov <orlovmg@gmail.com>; +Cc: Karina Litskevich <litskevichkarina@gmail.com>; Kyotaro Horiguchi <horikyota.ntt@gmail.com>; kyzevan23@mail.ru; pgsql-bugs@lists.postgresql.org
On Thu, Mar 7, 2024 at 3:17 PM Maxim Orlov <orlovmg@gmail.com> wrote:
> AFAICS, patch is committed around a month ago.
>
> commit 165d921c9a883814a35e8161fc692793e9c945a4
> Author: Alexander Korotkov <akorotkov@postgresql.org>
> Date: Thu Feb 8 12:45:26 2024 +0200
>
> So, I've mark this thread committed.
This was forgotten by me. Thanks!
------
Regards,
Alexander Korotkov
^ permalink raw reply [nested|flat] 10+ messages in thread
end of thread, other threads:[~2024-03-07 13:34 UTC | newest]
Thread overview: 10+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2023-11-24 06:28 BUG #18212: Functions txid_status() and pg_xact_status() return invalid status of the specified transaction PG Bug reporting form <noreply@postgresql.org>
2023-11-24 11:38 ` Karina Litskevich <litskevichkarina@gmail.com>
2023-11-28 05:20 ` Kyotaro Horiguchi <horikyota.ntt@gmail.com>
2023-11-30 05:30 ` Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 13:51 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-02-05 14:35 ` Karina Litskevich <litskevichkarina@gmail.com>
2024-02-05 14:50 ` Alexander Korotkov <aekorotkov@gmail.com>
2024-02-06 12:58 ` Karina Litskevich <litskevichkarina@gmail.com>
2024-03-07 13:17 ` Maxim Orlov <orlovmg@gmail.com>
2024-03-07 13:34 ` Alexander Korotkov <aekorotkov@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox