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

Commit54c98fa

Browse files
committed
Fix predicate-locking of HOT updated rows.
In serializable mode, heap_hot_search_buffer() incorrectly acquired apredicate lock on the root tuple, not the returned tuple that satisfiedthe visibility checks. As explained in README-SSI, the predicate lock doesnot need to be copied or extended to other tuple versions, but for that towork, the correct, visible, tuple version must be locked in the firstplace.The original SSI commit had this bug in it, but it was fixed back in 2013,in commit81fbbfe. But unfortunately, it was reintroduced a few monthslater in commitb89e151. Wising up from that, add a regression testto cover this, so that it doesn't get reintroduced again. Also, move thecode that sets 't_self', so that it happens at the same time that theother HeapTuple fields are set, to make it more clear that all the code inthe loop operate on the "current" tuple in the chain, not the root tuple.Bug spotted by Andres Freund, analysis and original fix by Thomas Munro,test case and some additional changes to the fix by Heikki Linnakangas.Backpatch to all supported versions (9.4).Discussion:https://www.postgresql.org/message-id/20190731210630.nqhszuktygwftjty%40alap3.anarazel.de
1 parent1f79436 commit54c98fa

File tree

4 files changed

+70
-17
lines changed

4 files changed

+70
-17
lines changed

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

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1677,6 +1677,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
16771677
{
16781678
Pagedp= (Page)BufferGetPage(buffer);
16791679
TransactionIdprev_xmax=InvalidTransactionId;
1680+
BlockNumberblkno;
16801681
OffsetNumberoffnum;
16811682
boolat_chain_start;
16821683
boolvalid;
@@ -1686,14 +1687,13 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
16861687
if (all_dead)
16871688
*all_dead=first_call;
16881689

1689-
Assert(TransactionIdIsValid(RecentGlobalXmin));
1690-
1691-
Assert(ItemPointerGetBlockNumber(tid)==BufferGetBlockNumber(buffer));
1690+
blkno=ItemPointerGetBlockNumber(tid);
16921691
offnum=ItemPointerGetOffsetNumber(tid);
16931692
at_chain_start=first_call;
16941693
skip= !first_call;
16951694

1696-
heapTuple->t_self=*tid;
1695+
Assert(TransactionIdIsValid(RecentGlobalXmin));
1696+
Assert(BufferGetBlockNumber(buffer)==blkno);
16971697

16981698
/* Scan through possible multiple members of HOT-chain */
16991699
for (;;)
@@ -1721,10 +1721,16 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
17211721
break;
17221722
}
17231723

1724+
/*
1725+
* Update heapTuple to point to the element of the HOT chain we're
1726+
* currently investigating. Having t_self set correctly is important
1727+
* because the SSI checks and the *Satisfies routine for historical
1728+
* MVCC snapshots need the correct tid to decide about the visibility.
1729+
*/
17241730
heapTuple->t_data= (HeapTupleHeader)PageGetItem(dp,lp);
17251731
heapTuple->t_len=ItemIdGetLength(lp);
17261732
heapTuple->t_tableOid=RelationGetRelid(relation);
1727-
ItemPointerSetOffsetNumber(&heapTuple->t_self,offnum);
1733+
ItemPointerSet(&heapTuple->t_self,blkno,offnum);
17281734

17291735
/*
17301736
* Shouldn't see a HEAP_ONLY tuple at chain start.
@@ -1750,21 +1756,10 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
17501756
*/
17511757
if (!skip)
17521758
{
1753-
/*
1754-
* For the benefit of logical decoding, have t_self point at the
1755-
* element of the HOT chain we're currently investigating instead
1756-
* of the root tuple of the HOT chain. This is important because
1757-
* the *Satisfies routine for historical mvcc snapshots needs the
1758-
* correct tid to decide about the visibility in some cases.
1759-
*/
1760-
ItemPointerSet(&(heapTuple->t_self),BufferGetBlockNumber(buffer),offnum);
1761-
17621759
/* If it's visible per the snapshot, we must return it */
17631760
valid=HeapTupleSatisfiesVisibility(heapTuple,snapshot,buffer);
17641761
CheckForSerializableConflictOut(valid,relation,heapTuple,
17651762
buffer,snapshot);
1766-
/* reset to original, non-redirected, tid */
1767-
heapTuple->t_self=*tid;
17681763

17691764
if (valid)
17701765
{
@@ -1793,7 +1788,7 @@ heap_hot_search_buffer(ItemPointer tid, Relation relation, Buffer buffer,
17931788
if (HeapTupleIsHotUpdated(heapTuple))
17941789
{
17951790
Assert(ItemPointerGetBlockNumber(&heapTuple->t_data->t_ctid)==
1796-
ItemPointerGetBlockNumber(tid));
1791+
blkno);
17971792
offnum=ItemPointerGetOffsetNumber(&heapTuple->t_data->t_ctid);
17981793
at_chain_start= false;
17991794
prev_xmax=HeapTupleHeaderGetUpdateXid(heapTuple->t_data);
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: b1 b2 r1 r2 w1 w2 c1 c2
4+
step b1: BEGIN ISOLATION LEVEL SERIALIZABLE;
5+
step b2: BEGIN ISOLATION LEVEL SERIALIZABLE;
6+
step r1: SELECT * FROM test WHERE i IN (5, 7)
7+
i t
8+
9+
5 apple
10+
7 pear_hot_updated
11+
step r2: SELECT * FROM test WHERE i IN (5, 7)
12+
i t
13+
14+
5 apple
15+
7 pear_hot_updated
16+
step w1: UPDATE test SET t = 'pear_xact1' WHERE i = 7
17+
step w2: UPDATE test SET t = 'apple_xact2' WHERE i = 5
18+
step c1: COMMIT;
19+
step c2: COMMIT;
20+
ERROR: could not serialize access due to read/write dependencies among transactions

‎src/test/isolation/isolation_schedule

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ test: partial-index
1414
test: two-ids
1515
test: multiple-row-versions
1616
test: index-only-scan
17+
test: predicate-lock-hot-tuple
1718
test: fk-contention
1819
test: fk-deadlock
1920
test: fk-deadlock2
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# Test predicate locks on HOT updated tuples.
2+
#
3+
# This test has two serializable transactions. Both select two rows
4+
# from the table, and then update one of them.
5+
# If these were serialized (run one at a time), the transaction that
6+
# runs later would see one of the rows to be updated.
7+
#
8+
# Any overlap between the transactions must cause a serialization failure.
9+
# We used to have a bug in predicate locking HOT updated tuples, which
10+
# caused the conflict to be missed, if the row was HOT updated.
11+
12+
setup
13+
{
14+
CREATETABLEtest (iintPRIMARYKEY,ttext);
15+
INSERTINTOtestVALUES (5,'apple'), (7,'pear'), (11,'banana');
16+
--HOT-update'pear'row.
17+
UPDATEtestSETt='pear_hot_updated'WHEREi=7;
18+
}
19+
20+
teardown
21+
{
22+
DROPTABLEtest;
23+
}
24+
25+
session"s1"
26+
step"b1" {BEGINISOLATIONLEVELSERIALIZABLE; }
27+
step"r1" {SELECT*FROMtestWHEREiIN (5,7) }
28+
step"w1" {UPDATEtestSETt='pear_xact1'WHEREi=7 }
29+
step"c1" {COMMIT; }
30+
31+
session"s2"
32+
step"b2" {BEGINISOLATIONLEVELSERIALIZABLE; }
33+
step"r2" {SELECT*FROMtestWHEREiIN (5,7) }
34+
step"w2" {UPDATEtestSETt='apple_xact2'WHEREi=5 }
35+
step"c2" {COMMIT; }
36+
37+
permutation"b1""b2""r1""r2""w1""w2""c1""c2"

0 commit comments

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