Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4f5d461

Browse files
committed
amcheck: Fix FullTransactionIdFromXidAndCtx() for xids before epoch 0
64bit xids can't represent xids before epoch 0 (see also be504a3). When FullTransactionIdFromXidAndCtx() was passed such an xid, it'd create a 64bit xid far into the future. Noticed while adding assertions in the course of investigating be504a3, as amcheck's test create such xids. To fix the issue, just return FirstNormalFullTransactionId in this case. A freshly initdb'd cluster already has a newer horizon. The most minimal version of this would make the messages for some detected corruptions differently inaccurate. To make those cases accurate, switch FullTransactionIdFromXidAndCtx() to use the 32bit modulo difference between xid and nextxid to compute the 64bit xid, yielding sensible "in the future" / "in the past" answers. Reviewed-by: Mark Dilger <mark.dilger@enterprisedb.com> Discussion: https://postgr.es/m/20230108002923.cyoser3ttmt63bfn@awork3.anarazel.de Backpatch: 14-, where heapam verification was introduced
1 parent 1632724 commit 4f5d461

File tree

2 files changed

+48
-13
lines changed

2 files changed

+48
-13
lines changed

contrib/amcheck/verify_heapam.c

+28-5
Original file line numberDiff line numberDiff line change
@@ -1574,17 +1574,40 @@ check_tuple(HeapCheckContext *ctx)
15741574
static FullTransactionId
15751575
FullTransactionIdFromXidAndCtx(TransactionId xid, const HeapCheckContext *ctx)
15761576
{
1577-
uint32 epoch;
1577+
uint64 nextfxid_i;
1578+
int32 diff;
1579+
FullTransactionId fxid;
15781580

15791581
Assert(TransactionIdIsNormal(ctx->next_xid));
15801582
Assert(FullTransactionIdIsNormal(ctx->next_fxid));
1583+
Assert(XidFromFullTransactionId(ctx->next_fxid) == ctx->next_xid);
15811584

15821585
if (!TransactionIdIsNormal(xid))
15831586
return FullTransactionIdFromEpochAndXid(0, xid);
1584-
epoch = EpochFromFullTransactionId(ctx->next_fxid);
1585-
if (xid > ctx->next_xid)
1586-
epoch--;
1587-
return FullTransactionIdFromEpochAndXid(epoch, xid);
1587+
1588+
nextfxid_i = U64FromFullTransactionId(ctx->next_fxid);
1589+
1590+
/* compute the 32bit modulo difference */
1591+
diff = (int32) (ctx->next_xid - xid);
1592+
1593+
/*
1594+
* In cases of corruption we might see a 32bit xid that is before epoch
1595+
* 0. We can't represent that as a 64bit xid, due to 64bit xids being
1596+
* unsigned integers, without the modulo arithmetic of 32bit xid. There's
1597+
* no really nice way to deal with that, but it works ok enough to use
1598+
* FirstNormalFullTransactionId in that case, as a freshly initdb'd
1599+
* cluster already has a newer horizon.
1600+
*/
1601+
if (diff > 0 && (nextfxid_i - FirstNormalTransactionId) < (int64) diff)
1602+
{
1603+
Assert(EpochFromFullTransactionId(ctx->next_fxid) == 0);
1604+
fxid = FirstNormalFullTransactionId;
1605+
}
1606+
else
1607+
fxid = FullTransactionIdFromU64(nextfxid_i - diff);
1608+
1609+
Assert(FullTransactionIdIsNormal(fxid));
1610+
return fxid;
15881611
}
15891612

15901613
/*

src/bin/pg_amcheck/t/004_verify_heapam.pl

+20-8
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ sub write_tuple
217217
my $relpath = "$pgdata/$rel";
218218

219219
# Insert data and freeze public.test
220-
use constant ROWCOUNT => 16;
220+
use constant ROWCOUNT => 17;
221221
$node->safe_psql(
222222
'postgres', qq(
223223
INSERT INTO public.test (a, b, c)
@@ -378,23 +378,24 @@ sub header
378378
elsif ($offnum == 3)
379379
{
380380
# Corruptly set xmin < datfrozenxid, further back, noting circularity
381-
# of xid comparison. For a new cluster with epoch = 0, the corrupt
382-
# xmin will be interpreted as in the future
383-
$tup->{t_xmin} = 4026531839;
381+
# of xid comparison.
382+
my $xmin = 4026531839;
383+
$tup->{t_xmin} = $xmin;
384384
$tup->{t_infomask} &= ~HEAP_XMIN_COMMITTED;
385385
$tup->{t_infomask} &= ~HEAP_XMIN_INVALID;
386386

387387
push @expected,
388-
qr/${$header}xmin 4026531839 equals or exceeds next valid transaction ID 0:\d+/;
388+
qr/${$header}xmin ${xmin} precedes oldest valid transaction ID 0:\d+/;
389389
}
390390
elsif ($offnum == 4)
391391
{
392392
# Corruptly set xmax < relminmxid;
393-
$tup->{t_xmax} = 4026531839;
393+
my $xmax = 4026531839;
394+
$tup->{t_xmax} = $xmax;
394395
$tup->{t_infomask} &= ~HEAP_XMAX_INVALID;
395396

396397
push @expected,
397-
qr/${$header}xmax 4026531839 equals or exceeds next valid transaction ID 0:\d+/;
398+
qr/${$header}xmax ${xmax} precedes oldest valid transaction ID 0:\d+/;
398399
}
399400
elsif ($offnum == 5)
400401
{
@@ -502,7 +503,7 @@ sub header
502503
push @expected,
503504
qr/${header}multitransaction ID 4 equals or exceeds next valid multitransaction ID 1/;
504505
}
505-
elsif ($offnum == 15) # Last offnum must equal ROWCOUNT
506+
elsif ($offnum == 15)
506507
{
507508
# Set both HEAP_XMAX_COMMITTED and HEAP_XMAX_IS_MULTI
508509
$tup->{t_infomask} |= HEAP_XMAX_COMMITTED;
@@ -512,6 +513,17 @@ sub header
512513
push @expected,
513514
qr/${header}multitransaction ID 4000000000 precedes relation minimum multitransaction ID threshold 1/;
514515
}
516+
elsif ($offnum == 16) # Last offnum must equal ROWCOUNT
517+
{
518+
# Corruptly set xmin > next_xid to be in the future.
519+
my $xmin = 123456;
520+
$tup->{t_xmin} = $xmin;
521+
$tup->{t_infomask} &= ~HEAP_XMIN_COMMITTED;
522+
$tup->{t_infomask} &= ~HEAP_XMIN_INVALID;
523+
524+
push @expected,
525+
qr/${$header}xmin ${xmin} equals or exceeds next valid transaction ID 0:\d+/;
526+
}
515527
write_tuple($file, $offset, $tup);
516528
}
517529
close($file)

0 commit comments

Comments
 (0)