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

Commit 38e9f90

Browse files
committed
Fix lazy_scan_heap so that it won't mark pages all-frozen too soon.
Commit a892234 added a new bit per page to the visibility map fork indicating whether the page is all-frozen, but incorrectly assumed that if lazy_scan_heap chose to freeze a tuple then that tuple would not need to later be frozen again. This turns out to be false, because xmin and xmax (and conceivably xvac, if dealing with tuples from very old releases) could be frozen at separate times. Thanks to Andres Freund for help in uncovering and tracking down this issue.
1 parent c7a25c2 commit 38e9f90

File tree

3 files changed

+40
-16
lines changed

3 files changed

+40
-16
lines changed

src/backend/access/heap/heapam.c

+32-13
Original file line numberDiff line numberDiff line change
@@ -6377,7 +6377,9 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
63776377
* are older than the specified cutoff XID and cutoff MultiXactId. If so,
63786378
* setup enough state (in the *frz output argument) to later execute and
63796379
* WAL-log what we would need to do, and return TRUE. Return FALSE if nothing
6380-
* is to be changed.
6380+
* is to be changed. In addition, set *totally_frozen_p to true if the tuple
6381+
* will be totally frozen after these operations are performed and false if
6382+
* more freezing will eventually be required.
63816383
*
63826384
* Caller is responsible for setting the offset field, if appropriate.
63836385
*
@@ -6402,12 +6404,12 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
64026404
bool
64036405
heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
64046406
TransactionId cutoff_multi,
6405-
xl_heap_freeze_tuple *frz)
6406-
6407+
xl_heap_freeze_tuple *frz, bool *totally_frozen_p)
64076408
{
64086409
bool changed = false;
64096410
bool freeze_xmax = false;
64106411
TransactionId xid;
6412+
bool totally_frozen = true;
64116413

64126414
frz->frzflags = 0;
64136415
frz->t_infomask2 = tuple->t_infomask2;
@@ -6416,11 +6418,15 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
64166418

64176419
/* Process xmin */
64186420
xid = HeapTupleHeaderGetXmin(tuple);
6419-
if (TransactionIdIsNormal(xid) &&
6420-
TransactionIdPrecedes(xid, cutoff_xid))
6421+
if (TransactionIdIsNormal(xid))
64216422
{
6422-
frz->t_infomask |= HEAP_XMIN_FROZEN;
6423-
changed = true;
6423+
if (TransactionIdPrecedes(xid, cutoff_xid))
6424+
{
6425+
frz->t_infomask |= HEAP_XMIN_FROZEN;
6426+
changed = true;
6427+
}
6428+
else
6429+
totally_frozen = false;
64246430
}
64256431

64266432
/*
@@ -6458,6 +6464,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
64586464
if (flags & FRM_MARK_COMMITTED)
64596465
frz->t_infomask &= HEAP_XMAX_COMMITTED;
64606466
changed = true;
6467+
totally_frozen = false;
64616468
}
64626469
else if (flags & FRM_RETURN_IS_MULTI)
64636470
{
@@ -6479,16 +6486,19 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
64796486
frz->xmax = newxmax;
64806487

64816488
changed = true;
6489+
totally_frozen = false;
64826490
}
64836491
else
64846492
{
64856493
Assert(flags & FRM_NOOP);
64866494
}
64876495
}
6488-
else if (TransactionIdIsNormal(xid) &&
6489-
TransactionIdPrecedes(xid, cutoff_xid))
6496+
else if (TransactionIdIsNormal(xid))
64906497
{
6491-
freeze_xmax = true;
6498+
if (TransactionIdPrecedes(xid, cutoff_xid))
6499+
freeze_xmax = true;
6500+
else
6501+
totally_frozen = false;
64926502
}
64936503

64946504
if (freeze_xmax)
@@ -6514,8 +6524,15 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
65146524
if (tuple->t_infomask & HEAP_MOVED)
65156525
{
65166526
xid = HeapTupleHeaderGetXvac(tuple);
6517-
if (TransactionIdIsNormal(xid) &&
6518-
TransactionIdPrecedes(xid, cutoff_xid))
6527+
/*
6528+
* For Xvac, we ignore the cutoff_xid and just always perform the
6529+
* freeze operation. The oldest release in which such a value can
6530+
* actually be set is PostgreSQL 8.4, because old-style VACUUM FULL
6531+
* was removed in PostgreSQL 9.0. Note that if we were to respect
6532+
* cutoff_xid here, we'd need to make surely to clear totally_frozen
6533+
* when we skipped freezing on that basis.
6534+
*/
6535+
if (TransactionIdIsNormal(xid))
65196536
{
65206537
/*
65216538
* If a MOVED_OFF tuple is not dead, the xvac transaction must
@@ -6537,6 +6554,7 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
65376554
}
65386555
}
65396556

6557+
*totally_frozen_p = totally_frozen;
65406558
return changed;
65416559
}
65426560

@@ -6587,9 +6605,10 @@ heap_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
65876605
{
65886606
xl_heap_freeze_tuple frz;
65896607
bool do_freeze;
6608+
bool tuple_totally_frozen;
65906609

65916610
do_freeze = heap_prepare_freeze_tuple(tuple, cutoff_xid, cutoff_multi,
6592-
&frz);
6611+
&frz, &tuple_totally_frozen);
65936612

65946613
/*
65956614
* Note that because this is not a WAL-logged operation, we don't need to

src/backend/commands/vacuumlazy.c

+6-2
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,8 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
10541054
}
10551055
else
10561056
{
1057+
bool tuple_totally_frozen;
1058+
10571059
num_tuples += 1;
10581060
hastup = true;
10591061

@@ -1062,9 +1064,11 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
10621064
* freezing. Note we already have exclusive buffer lock.
10631065
*/
10641066
if (heap_prepare_freeze_tuple(tuple.t_data, FreezeLimit,
1065-
MultiXactCutoff, &frozen[nfrozen]))
1067+
MultiXactCutoff, &frozen[nfrozen],
1068+
&tuple_totally_frozen))
10661069
frozen[nfrozen++].offset = offnum;
1067-
else if (heap_tuple_needs_eventual_freeze(tuple.t_data))
1070+
1071+
if (!tuple_totally_frozen)
10681072
all_frozen = false;
10691073
}
10701074
} /* scan along page */

src/include/access/heapam_xlog.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,8 @@ extern XLogRecPtr log_heap_freeze(Relation reln, Buffer buffer,
386386
extern bool heap_prepare_freeze_tuple(HeapTupleHeader tuple,
387387
TransactionId cutoff_xid,
388388
TransactionId cutoff_multi,
389-
xl_heap_freeze_tuple *frz);
389+
xl_heap_freeze_tuple *frz,
390+
bool *totally_frozen);
390391
extern void heap_execute_freeze_tuple(HeapTupleHeader tuple,
391392
xl_heap_freeze_tuple *xlrec_tp);
392393
extern XLogRecPtr log_heap_visible(RelFileNode rnode, Buffer heap_buffer,

0 commit comments

Comments
 (0)