Skip to content

Commit b611c38

Browse files
tglsfdctglsfdc
committed
Patch VACUUM problem with moving chain of update tuples when source
and destination of a tuple lie on the same page.
1 parent c3bbbb1 commit b611c38

File tree

1 file changed

+56
-22
lines changed

1 file changed

+56
-22
lines changed

‎src/backend/commands/vacuum.c

Lines changed: 56 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.148.2.1 2000/09/19 21:01:04 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.148.2.2 2000/10/17 01:39:56 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -815,6 +815,7 @@ vc_scanheap(VRelStats *vacrelstats, Relation onerel,
815815
tuple.t_data->t_cmin))
816816
{
817817
tuple.t_data->t_infomask |= HEAP_XMIN_INVALID;
818+
pgchanged = true;
818819
tupgone = true;
819820
}
820821
else
@@ -829,6 +830,7 @@ vc_scanheap(VRelStats *vacrelstats, Relation onerel,
829830
tuple.t_data->t_cmin))
830831
{
831832
tuple.t_data->t_infomask |= HEAP_XMIN_INVALID;
833+
pgchanged = true;
832834
tupgone = true;
833835
}
834836
else
@@ -876,8 +878,10 @@ vc_scanheap(VRelStats *vacrelstats, Relation onerel,
876878
{
877879
if (tuple.t_data->t_infomask & HEAP_MARKED_FOR_UPDATE)
878880
{
879-
pgchanged = true;
880881
tuple.t_data->t_infomask |= HEAP_XMAX_INVALID;
882+
tuple.t_data->t_infomask &=
883+
~(HEAP_XMAX_COMMITTED | HEAP_MARKED_FOR_UPDATE);
884+
pgchanged = true;
881885
}
882886
else
883887
tupgone = true;
@@ -892,6 +896,8 @@ vc_scanheap(VRelStats *vacrelstats, Relation onerel,
892896
if (tuple.t_data->t_infomask & HEAP_MARKED_FOR_UPDATE)
893897
{
894898
tuple.t_data->t_infomask |= HEAP_XMAX_INVALID;
899+
tuple.t_data->t_infomask &=
900+
~(HEAP_XMAX_COMMITTED | HEAP_MARKED_FOR_UPDATE);
895901
pgchanged = true;
896902
}
897903
else
@@ -905,6 +911,8 @@ vc_scanheap(VRelStats *vacrelstats, Relation onerel,
905911
* from crashed process. - vadim 06/02/97
906912
*/
907913
tuple.t_data->t_infomask |= HEAP_XMAX_INVALID;
914+
tuple.t_data->t_infomask &=
915+
~(HEAP_XMAX_COMMITTED | HEAP_MARKED_FOR_UPDATE);
908916
pgchanged = true;
909917
}
910918
else
@@ -964,6 +972,14 @@ vc_scanheap(VRelStats *vacrelstats, Relation onerel,
964972
{
965973
ItemId lpp;
966974

975+
/*
976+
* Here we are building a temporary copy of the page with
977+
* dead tuples removed. Below we will apply
978+
* PageRepairFragmentation to the copy, so that we can
979+
* determine how much space will be available after
980+
* removal of dead tuples. But note we are NOT changing
981+
* the real page yet...
982+
*/
967983
if (tempPage == (Page) NULL)
968984
{
969985
Size pageSize;
@@ -973,14 +989,12 @@ vc_scanheap(VRelStats *vacrelstats, Relation onerel,
973989
memmove(tempPage, page, pageSize);
974990
}
975991

992+
/* mark it unused on the temp page */
976993
lpp = &(((PageHeader) tempPage)->pd_linp[offnum - 1]);
977-
978-
/* mark it unused */
979994
lpp->lp_flags &= ~LP_USED;
980995

981996
vpc->vpd_offsets[vpc->vpd_offsets_free++] = offnum;
982997
tups_vacuumed++;
983-
984998
}
985999
else
9861000
{
@@ -1548,20 +1562,45 @@ vc_repair_frag(VRelStats *vacrelstats, Relation onerel,
15481562
tuple.t_datamcxt = NULL;
15491563
tuple.t_data = (HeapTupleHeader) PageGetItem(Cpage, Citemid);
15501564
tuple_len = tuple.t_len = ItemIdGetLength(Citemid);
1551-
/* Get page to move in */
1565+
1566+
/*
1567+
* make a copy of the source tuple, and then mark the
1568+
* source tuple MOVED_OFF.
1569+
*/
1570+
heap_copytuple_with_tuple(&tuple, &newtup);
1571+
1572+
RelationInvalidateHeapTuple(onerel, &tuple);
1573+
1574+
TransactionIdStore(myXID, (TransactionId *) &(tuple.t_data->t_cmin));
1575+
tuple.t_data->t_infomask &=
1576+
~(HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID | HEAP_MOVED_IN);
1577+
tuple.t_data->t_infomask |= HEAP_MOVED_OFF;
1578+
1579+
/* Get page to move to */
15521580
cur_buffer = ReadBuffer(onerel, destvpd->vpd_blkno);
15531581

15541582
/*
15551583
* We should LockBuffer(cur_buffer) but don't, at the
1556-
* moment. If you'll do LockBuffer then UNLOCK it
1557-
* before index_insert: unique btree-s call heap_fetch
1558-
* to get t_infomask of inserted heap tuple !!!
1584+
* moment. This should be safe enough, since we have
1585+
* exclusive lock on the whole relation.
1586+
* If you'll do LockBuffer then UNLOCK it before
1587+
* index_insert: unique btree-s call heap_fetch to get
1588+
* t_infomask of inserted heap tuple !!!
15591589
*/
15601590
ToPage = BufferGetPage(cur_buffer);
15611591

15621592
/*
15631593
* If this page was not used before - clean it.
15641594
*
1595+
* NOTE: a nasty bug used to lurk here. It is possible
1596+
* for the source and destination pages to be the same
1597+
* (since this tuple-chain member can be on a page lower
1598+
* than the one we're currently processing in the outer
1599+
* loop). If that's true, then after vc_vacpage() the
1600+
* source tuple will have been moved, and tuple.t_data
1601+
* will be pointing at garbage. Therefore we must do
1602+
* everything that uses tuple.t_data BEFORE this step!!
1603+
*
15651604
* This path is different from the other callers of
15661605
* vc_vacpage, because we have already incremented the
15671606
* vpd's vpd_offsets_used field to account for the
@@ -1579,8 +1618,11 @@ vc_repair_frag(VRelStats *vacrelstats, Relation onerel,
15791618
vc_vacpage(ToPage, destvpd);
15801619
destvpd->vpd_offsets_used = sv_offsets_used;
15811620
}
1582-
heap_copytuple_with_tuple(&tuple, &newtup);
1583-
RelationInvalidateHeapTuple(onerel, &tuple);
1621+
1622+
/*
1623+
* Update the state of the copied tuple, and store it
1624+
* on the destination page.
1625+
*/
15841626
TransactionIdStore(myXID, (TransactionId *) &(newtup.t_data->t_cmin));
15851627
newtup.t_data->t_infomask &=
15861628
~(HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID | HEAP_MOVED_OFF);
@@ -1601,20 +1643,15 @@ vc_repair_frag(VRelStats *vacrelstats, Relation onerel,
16011643
last_move_dest_block = destvpd->vpd_blkno;
16021644

16031645
/*
1604-
* Set t_ctid pointing to itself for last tuple in
1605-
* chain and to next tuple in chain otherwise.
1646+
* Set new tuple's t_ctid pointing to itself for last
1647+
* tuple in chain, and to next tuple in chain otherwise.
16061648
*/
16071649
if (!ItemPointerIsValid(&Ctid))
16081650
newtup.t_data->t_ctid = newtup.t_self;
16091651
else
16101652
newtup.t_data->t_ctid = Ctid;
16111653
Ctid = newtup.t_self;
16121654

1613-
TransactionIdStore(myXID, (TransactionId *) &(tuple.t_data->t_cmin));
1614-
tuple.t_data->t_infomask &=
1615-
~(HEAP_XMIN_COMMITTED | HEAP_XMIN_INVALID | HEAP_MOVED_IN);
1616-
tuple.t_data->t_infomask |= HEAP_MOVED_OFF;
1617-
16181655
num_moved++;
16191656

16201657
/*
@@ -1648,10 +1685,7 @@ vc_repair_frag(VRelStats *vacrelstats, Relation onerel,
16481685
}
16491686
}
16501687
WriteBuffer(cur_buffer);
1651-
if (Cbuf == buf)
1652-
ReleaseBuffer(Cbuf);
1653-
else
1654-
WriteBuffer(Cbuf);
1688+
WriteBuffer(Cbuf);
16551689
}
16561690
cur_buffer = InvalidBuffer;
16571691
pfree(vtmove);

0 commit comments

Comments
 (0)