Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit20b6552

Browse files
committed
Fix freezing of a dead HOT-updated tuple
Vacuum calls page-level HOT prune to remove dead HOT tuples before doingliveness checks (HeapTupleSatisfiesVacuum) on the remaining tuples. Butconcurrent transaction commit/abort may turn DEAD some of the HOT tuplesthat survived the prune, before HeapTupleSatisfiesVacuum tests them.This happens to activate the code that decides to freeze the tuple ...which resuscitates it, duplicating data.(This is especially bad if there's any unique constraints, because thoseare now internally violated due to the duplicate entries, though youwon't know until you try to REINDEX or dump/restore the table.)One possible fix would be to simply skip doing anything to the tuple,and hope that the next HOT prune would remove it. But there is aproblem: if the tuple is older than freeze horizon, this would leave anunfrozen XID behind, and if no HOT prune happens to clean it up beforethe containing pg_clog segment is truncated away, it'd later cause anerror when the XID is looked up.Fix the problem by having the tuple freezing routines cope with thesituation: don't freeze the tuple (and keep it dead). In the cases thatthe XID is older than the freeze age, set the HEAP_XMAX_COMMITTED flagso that there is no need to look up the XID in pg_clog later on.An isolation test is included, authored by Michael Paquier, looselybased on Daniel Wood's original reproducer. It only tests oneparticular scenario, though, not all the possible ways for this problemto surface; it be good to have a more reliable way to test this morefully, but it'd require more work.In messagehttps://postgr.es/m/20170911140103.5akxptyrwgpc25bw@alvherre.pgsqlI outlined another test case (more closely matching Dan Wood's) thatexposed a few more ways for the problem to occur.Backpatch all the way back to 9.3, where this problem was introduced bymultixact juggling. In branches 9.3 and 9.4, this includes a backpatchof commit e5ff9fefcd50 (of 9.5 era), since the original is notcorrectable without matching the coding pattern in 9.5 up.Reported-by: Daniel WoodDiagnosed-by: Daniel WoodReviewed-by: Yi Wen Wong, Michaël PaquierDiscussion:https://postgr.es/m/E5711E62-8FDF-4DCA-A888-C200BF6B5742@amazon.com
1 parent66fd86a commit20b6552

File tree

5 files changed

+179
-27
lines changed

5 files changed

+179
-27
lines changed

‎src/backend/access/heap/heapam.c

Lines changed: 40 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -6405,14 +6405,23 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
64056405
Assert(TransactionIdIsValid(xid));
64066406

64076407
/*
6408-
* If the xid is older than the cutoff, it has to have aborted,
6409-
* otherwise the tuple would have gotten pruned away.
6408+
* The updating transaction cannot possibly be still running, but
6409+
* verify whether it has committed, and request to set the
6410+
* COMMITTED flag if so. (We normally don't see any tuples in
6411+
* this state, because they are removed by page pruning before we
6412+
* try to freeze the page; but this can happen if the updating
6413+
* transaction commits after the page is pruned but before
6414+
* HeapTupleSatisfiesVacuum).
64106415
*/
64116416
if (TransactionIdPrecedes(xid,cutoff_xid))
64126417
{
6413-
Assert(!TransactionIdDidCommit(xid));
6414-
*flags |=FRM_INVALIDATE_XMAX;
6415-
xid=InvalidTransactionId;/* not strictly necessary */
6418+
if (TransactionIdDidCommit(xid))
6419+
*flags=FRM_MARK_COMMITTED |FRM_RETURN_IS_XID;
6420+
else
6421+
{
6422+
*flags |=FRM_INVALIDATE_XMAX;
6423+
xid=InvalidTransactionId;/* not strictly necessary */
6424+
}
64166425
}
64176426
else
64186427
{
@@ -6485,13 +6494,16 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
64856494
/*
64866495
* It's an update; should we keep it? If the transaction is known
64876496
* aborted or crashed then it's okay to ignore it, otherwise not.
6488-
* Note that an updater older than cutoff_xid cannot possibly be
6489-
* committed, because HeapTupleSatisfiesVacuum would have returned
6490-
* HEAPTUPLE_DEAD and we would not be trying to freeze the tuple.
64916497
*
64926498
* As with all tuple visibility routines, it's critical to test
64936499
* TransactionIdIsInProgress before TransactionIdDidCommit,
64946500
* because of race conditions explained in detail in tqual.c.
6501+
*
6502+
* We normally don't see committed updating transactions earlier
6503+
* than the cutoff xid, because they are removed by page pruning
6504+
* before we try to freeze the page; but it can happen if the
6505+
* updating transaction commits after the page is pruned but
6506+
* before HeapTupleSatisfiesVacuum.
64956507
*/
64966508
if (TransactionIdIsCurrentTransactionId(xid)||
64976509
TransactionIdIsInProgress(xid))
@@ -6516,13 +6528,6 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
65166528
* we can ignore it.
65176529
*/
65186530

6519-
/*
6520-
* Since the tuple wasn't marked HEAPTUPLE_DEAD by vacuum, the
6521-
* update Xid cannot possibly be older than the xid cutoff.
6522-
*/
6523-
Assert(!TransactionIdIsValid(update_xid)||
6524-
!TransactionIdPrecedes(update_xid,cutoff_xid));
6525-
65266531
/*
65276532
* If we determined that it's an Xid corresponding to an update
65286533
* that must be retained, additionally add it to the list of
@@ -6601,7 +6606,10 @@ FreezeMultiXactId(MultiXactId multi, uint16 t_infomask,
66016606
*
66026607
* It is assumed that the caller has checked the tuple with
66036608
* HeapTupleSatisfiesVacuum() and determined that it is not HEAPTUPLE_DEAD
6604-
* (else we should be removing the tuple, not freezing it).
6609+
* (else we should be removing the tuple, not freezing it). However, note
6610+
* that we don't remove HOT tuples even if they are dead, and it'd be incorrect
6611+
* to freeze them (because that would make them visible), so we mark them as
6612+
* update-committed, and needing further freezing later on.
66056613
*
66066614
* NB: cutoff_xid *must* be <= the current global xmin, to ensure that any
66076615
* XID older than it could neither be running nor seen as running by any
@@ -6712,7 +6720,22 @@ heap_prepare_freeze_tuple(HeapTupleHeader tuple, TransactionId cutoff_xid,
67126720
elseif (TransactionIdIsNormal(xid))
67136721
{
67146722
if (TransactionIdPrecedes(xid,cutoff_xid))
6715-
freeze_xmax= true;
6723+
{
6724+
/*
6725+
* Must freeze regular XIDs older than the cutoff. We must not
6726+
* freeze a HOT-updated tuple, though; doing so would bring it
6727+
* back to life.
6728+
*/
6729+
if (HeapTupleHeaderIsHotUpdated(tuple))
6730+
{
6731+
frz->t_infomask |=HEAP_XMAX_COMMITTED;
6732+
totally_frozen= false;
6733+
changed= true;
6734+
/* must not freeze */
6735+
}
6736+
else
6737+
freeze_xmax= true;
6738+
}
67166739
else
67176740
totally_frozen= false;
67186741
}

‎src/backend/commands/vacuumlazy.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2018,17 +2018,17 @@ lazy_record_dead_tuple(LVRelStats *vacrelstats,
20182018
ItemPointeritemptr)
20192019
{
20202020
/*
2021-
* The arrayshouldn'toverflow under normal behavior, but perhaps it
2022-
*could if we are given areally small maintenance_work_mem. In that
2023-
*case, just forgetthelast few tuples (we'll get 'em next time).
2021+
* The arraymust neveroverflow, since we rely on all deletable tuples
2022+
*being removed; inability to remove atuple might cause an old XID to
2023+
*persist beyondthefreeze limit, which could be disastrous later on.
20242024
*/
2025-
if (vacrelstats->num_dead_tuples<vacrelstats->max_dead_tuples)
2026-
{
2027-
vacrelstats->dead_tuples[vacrelstats->num_dead_tuples]=*itemptr;
2028-
vacrelstats->num_dead_tuples++;
2029-
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
2030-
vacrelstats->num_dead_tuples);
2031-
}
2025+
if (vacrelstats->num_dead_tuples>=vacrelstats->max_dead_tuples)
2026+
elog(ERROR,"dead tuple array overflow");
2027+
2028+
vacrelstats->dead_tuples[vacrelstats->num_dead_tuples]=*itemptr;
2029+
vacrelstats->num_dead_tuples++;
2030+
pgstat_progress_update_param(PROGRESS_VACUUM_NUM_DEAD_TUPLES,
2031+
vacrelstats->num_dead_tuples);
20322032
}
20332033

20342034
/*
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: s1_update s1_commit s1_vacuum s2_key_share s2_commit
4+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
5+
step s1_commit: COMMIT;
6+
step s1_vacuum: VACUUM FREEZE tab_freeze;
7+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
8+
id
9+
10+
3
11+
step s2_commit: COMMIT;
12+
13+
starting permutation: s1_update s1_commit s2_key_share s1_vacuum s2_commit
14+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
15+
step s1_commit: COMMIT;
16+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
17+
id
18+
19+
3
20+
step s1_vacuum: VACUUM FREEZE tab_freeze;
21+
step s2_commit: COMMIT;
22+
23+
starting permutation: s1_update s1_commit s2_key_share s2_commit s1_vacuum
24+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
25+
step s1_commit: COMMIT;
26+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
27+
id
28+
29+
3
30+
step s2_commit: COMMIT;
31+
step s1_vacuum: VACUUM FREEZE tab_freeze;
32+
33+
starting permutation: s1_update s2_key_share s1_commit s1_vacuum s2_commit
34+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
35+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
36+
id
37+
38+
3
39+
step s1_commit: COMMIT;
40+
step s1_vacuum: VACUUM FREEZE tab_freeze;
41+
step s2_commit: COMMIT;
42+
43+
starting permutation: s1_update s2_key_share s1_commit s2_commit s1_vacuum
44+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
45+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
46+
id
47+
48+
3
49+
step s1_commit: COMMIT;
50+
step s2_commit: COMMIT;
51+
step s1_vacuum: VACUUM FREEZE tab_freeze;
52+
53+
starting permutation: s1_update s2_key_share s2_commit s1_commit s1_vacuum
54+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
55+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
56+
id
57+
58+
3
59+
step s2_commit: COMMIT;
60+
step s1_commit: COMMIT;
61+
step s1_vacuum: VACUUM FREEZE tab_freeze;
62+
63+
starting permutation: s2_key_share s1_update s1_commit s1_vacuum s2_commit
64+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
65+
id
66+
67+
3
68+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
69+
step s1_commit: COMMIT;
70+
step s1_vacuum: VACUUM FREEZE tab_freeze;
71+
step s2_commit: COMMIT;
72+
73+
starting permutation: s2_key_share s1_update s1_commit s2_commit s1_vacuum
74+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
75+
id
76+
77+
3
78+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
79+
step s1_commit: COMMIT;
80+
step s2_commit: COMMIT;
81+
step s1_vacuum: VACUUM FREEZE tab_freeze;
82+
83+
starting permutation: s2_key_share s1_update s2_commit s1_commit s1_vacuum
84+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
85+
id
86+
87+
3
88+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
89+
step s2_commit: COMMIT;
90+
step s1_commit: COMMIT;
91+
step s1_vacuum: VACUUM FREEZE tab_freeze;
92+
93+
starting permutation: s2_key_share s2_commit s1_update s1_commit s1_vacuum
94+
step s2_key_share: SELECT id FROM tab_freeze WHERE id = 3 FOR KEY SHARE;
95+
id
96+
97+
3
98+
step s2_commit: COMMIT;
99+
step s1_update: UPDATE tab_freeze SET x = x + 1 WHERE id = 3;
100+
step s1_commit: COMMIT;
101+
step s1_vacuum: VACUUM FREEZE tab_freeze;

‎src/test/isolation/isolation_schedule

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ test: update-locked-tuple
4444
test: propagate-lock-delete
4545
test: tuplelock-conflict
4646
test: tuplelock-update
47+
test: freeze-the-dead
4748
test: nowait
4849
test: nowait-2
4950
test: nowait-3
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Test for interactions of tuple freezing with dead, as well as recently-dead
2+
# tuples using multixacts via FOR KEY SHARE.
3+
setup
4+
{
5+
CREATETABLEtab_freeze (
6+
idintPRIMARYKEY,
7+
namechar(3),
8+
xint);
9+
INSERTINTOtab_freezeVALUES (1,'111',0);
10+
INSERTINTOtab_freezeVALUES (3,'333',0);
11+
}
12+
13+
teardown
14+
{
15+
DROPTABLEtab_freeze;
16+
}
17+
18+
session"s1"
19+
setup{BEGIN; }
20+
step"s1_update"{UPDATEtab_freezeSETx=x+1WHEREid=3; }
21+
step"s1_commit"{COMMIT; }
22+
step"s1_vacuum"{VACUUMFREEZEtab_freeze; }
23+
24+
session"s2"
25+
setup{BEGIN; }
26+
step"s2_key_share"{SELECTidFROMtab_freezeWHEREid=3FORKEYSHARE; }
27+
step"s2_commit"{COMMIT; }

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp