8000 Fix a longstanding bug in VACUUM FULL's handling of update chains. T… · danielcode/postgres@6ce2ca3 · GitHub
[go: up one dir, main page]
Skip to content

Commit 6ce2ca3

Browse files
tglsfdctglsfdc
committed
Fix a longstanding bug in VACUUM FULL's handling of update chains. The code
did not expect that a DEAD tuple could follow a RECENTLY_DEAD tuple in an update chain, but because the OldestXmin rule for determining deadness is a simplification of reality, it is possible for this situation to occur (implying that the RECENTLY_DEAD tuple is in fact dead to all observers, but this patch does not attempt to exploit that). The code would follow a chain forward all the way, but then stop before a DEAD tuple when backing up, meaning that not all of the chain got moved. This could lead to copying the chain multiple times (resulting in duplicate copies of the live tuple at its end), or leaving dangling index entries behind (which, aside from generating warnings from later vacuums, creates a risk of wrong query results or bogus duplicate-key errors once the heap slot the index entry points to is repopulated). The fix is to recheck HeapTupleSatisfiesVacuum while following a chain forward, and to stop if a DEAD tuple is reached. Each contiguous group of RECENTLY_DEAD tuples will therefore be copied as a separate chain. The patch also adds a couple of extra sanity checks to verify correct behavior. Per report and test case from Pavan Deolasee.
1 parent 4edaffa commit 6ce2ca3

File tree

1 file changed

+49
-2
lines changed

1 file changed

+49
-2
lines changed

‎src/backend/commands/vacuum.c

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
*
1414
*
1515
* IDENTIFICATION
16-
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.244.2.1 2005/08/26 20:07:16 tgl Exp $
16+
* $Header: /cvsroot/pgsql/src/backend/commands/vacuum.c,v 1.244.2.2 2007/03/14 18:49:32 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -1634,6 +1634,15 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
16341634
* we cannot find the parent tuple in vtlinks. This may be
16351635
* overly conservative; AFAICS it would be safe to move the
16361636
* chain.
1637+
*
1638+
* Also, because we distinguish DEAD and RECENTLY_DEAD tuples
1639+
* using OldestXmin, which is a rather coarse test, it is quite
1640+
* possible to have an update chain in which a tuple we think is
1641+
* RECENTLY_DEAD links forward to one that is definitely DEAD.
1642+
* In such a case the RECENTLY_DEAD tuple must actually be dead,
1643+
* but it seems too complicated to try to make VACUUM remove it.
1644+
* We treat each contiguous set of RECENTLY_DEAD tuples as a
1645+
* separately movable chain, ignoring any intervening DEAD ones.
16371646
*/
16381647
if (((tuple.t_data->t_infomask & HEAP_UPDATED) &&
16391648
!TransactionIdPrecedes(HeapTupleHeaderGetXmin(tuple.t_data),
@@ -1646,6 +1655,7 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
16461655
Buffer Cbuf = buf;
16471656
bool freeCbuf = false;
16481657
bool chain_move_failed = false;
1658+
bool moved_target = false;
16491659
ItemPointerData Ctid;
16501660
HeapTupleData tp = tuple;
16511661
Size tlen = tuple_len;
@@ -1673,7 +1683,14 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
16731683
* If this tuple is in the begin/middle of the chain then
16741684
* we have to move to the end of chain. As with any
16751685
* t_ctid chase, we have to verify that each new tuple
1676-
* is really the descendant of the tuple we came from.
1686+
* is really the descendant of the tuple we came from;
1687+
* however, here we need even more than the normal amount of
1688+
* paranoia. If t_ctid links forward to a tuple determined
1689+
* to be DEAD, then depending on where that tuple is, it
1690+
* might already have been removed, and perhaps even replaced
1691+
* by a MOVED_IN tuple. We don't want to include any DEAD
1692+
* tuples in the chain, so we have to recheck
1693+
* HeapTupleSatisfiesVacuum.
16771694
*/
16781695
while (!(tp.t_data->t_infomask & (HEAP_XMAX_INVALID |
16791696
HEAP_MARKED_FOR_UPDATE)) &&
@@ -1687,6 +1704,8 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
16871704
OffsetNumber nextOffnum;
16881705
ItemId nextItemid;
16891706
HeapTupleHeader nextTdata;
1707+
uint16 sv_infomask;
1708+
HTSV_Result nextTstatus;
16901709

16911710
nextTid = tp.t_data->t_ctid;
16921711
priorXmax = HeapTupleHeaderGetXmax(tp.t_data);
@@ -1717,6 +1736,22 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
17171736
ReleaseBuffer(nextBuf);
17181737
break;
17191738
}
1739+
/* must check for DEAD or MOVED_IN tuple, too */
1740+
sv_infomask = nextTdata->t_infomask;
1741+
nextTstatus = HeapTupleSatisfiesVacuum(nextTdata,
1742+
OldestXmin);
1743+
/* not sure hint-bit change can happen, but be careful */
1744+
if (sv_infomask != nextTdata->t_infomask)
1745+
SetBufferCommitInfoNeedsSave(nextBuf);
1746+
if (nextTstatus == HEAPTUPLE_DEAD ||
1747+
nextTstatus == HEAPTUPLE_INSERT_IN_PROGRESS)
1748+
{
1749+
ReleaseBuffer(nextBuf);
1750+
break;
1751+
}
1752+
/* if it's MOVED_OFF we shoulda moved this one with it */
1753+
if (nextTstatus == HEAPTUPLE_DELETE_IN_PROGRESS)
1754+
elog(ERROR, "updated tuple is already HEAP_MOVED_OFF");
17201755
/* OK, switch our attention to the next tuple in chain */
17211756
tp.t_datamcxt = NULL;
17221757
tp.t_data = nextTdata;
@@ -1787,6 +1822,11 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
17871822
free_vtmove--;
17881823
num_vtmove++;
17891824

1825+
/* Remember if we reached the original target tuple */
1826+
if (ItemPointerGetBlockNumber(&tp.t_self) == blkno &&
1827+
ItemPointerGetOffsetNumber(&tp.t_self) == offnum)
1828+
moved_target = true;
1829+
17901830
/* At beginning of chain? */
17911831
if (!(tp.t_data->t_infomask & HEAP_UPDATED) ||
17921832
TransactionIdPrecedes(HeapTupleHeaderGetXmin(tp.t_data),
@@ -1856,6 +1896,13 @@ repair_frag(VRelStats *vacrelstats, Relation onerel,
18561896
ReleaseBuffer(Cbuf);
18571897
freeCbuf = false;
18581898

1899+
/* Double-check that we will move the current target tuple */
1900+
if (!moved_target && !chain_move_failed)
1901+
{
1902+
elog(DEBUG2, "failed to chain back to target --- cannot continue repair_frag");
1903+
chain_move_failed = true;
1904+
}
1905+
18591906
if (chain_move_failed)
18601907
{
18611908
/*

0 commit comments

Comments
 (0)
0