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

Commit27336e4

Browse files
committed
Repair race condition introduced into heap_update() in 7.1 ---
PageGetFreeSpace() was being called while not holding the buffer lock, whichnot only could yield a garbage answer, but even if it's the right answer theremight be less space available after we reacquire the buffer lock.Also repair potential deadlock introduced by my recent performance improvementin RelationGetBufferForTuple(): it was possible for two heap_updates to try tolock two buffers in opposite orders. The fix creates a global rule thatbuffers of a single heap relation should be locked in decreasing block numberorder. Currently, this only applies to heap_update; VACUUM can get away withignoring the rule since it holds exclusive lock on the whole relation anyway.However, if we try to implement a VACUUM that can run in parallel with othertransactions, VACUUM will also have to obey the lock order rule.
1 parentae38a1f commit27336e4

File tree

3 files changed

+116
-47
lines changed

3 files changed

+116
-47
lines changed

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

Lines changed: 64 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.114 2001/05/12 19:58:27 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/heap/heapam.c,v 1.115 2001/05/16 22:35:12 tgl Exp $
1212
*
1313
*
1414
* INTERFACE ROUTINES
@@ -1317,7 +1317,7 @@ heap_insert(Relation relation, HeapTuple tup)
13171317
#endif
13181318

13191319
/* Find buffer for this tuple */
1320-
buffer=RelationGetBufferForTuple(relation,tup->t_len);
1320+
buffer=RelationGetBufferForTuple(relation,tup->t_len,0);
13211321

13221322
/* NO ELOG(ERROR) from here till changes are logged */
13231323
START_CRIT_SECTION();
@@ -1578,6 +1578,8 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
15781578
newbuf;
15791579
boolneed_toast,
15801580
already_marked;
1581+
Sizenewtupsize,
1582+
pagefree;
15811583
intresult;
15821584

15831585
/* increment access statistics */
@@ -1685,8 +1687,10 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
16851687
HeapTupleHasExtended(newtup)||
16861688
(MAXALIGN(newtup->t_len)>TOAST_TUPLE_THRESHOLD));
16871689

1688-
if (need_toast||
1689-
(unsigned)MAXALIGN(newtup->t_len)>PageGetFreeSpace((Page)dp))
1690+
newtupsize=MAXALIGN(newtup->t_len);
1691+
pagefree=PageGetFreeSpace((Page)dp);
1692+
1693+
if (need_toast||newtupsize>pagefree)
16901694
{
16911695
_locked_tuple_.node=relation->rd_node;
16921696
_locked_tuple_.tid=oldtup.t_self;
@@ -1704,17 +1708,60 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
17041708

17051709
/* Let the toaster do its thing */
17061710
if (need_toast)
1711+
{
17071712
heap_tuple_toast_attrs(relation,newtup,&oldtup);
1713+
newtupsize=MAXALIGN(newtup->t_len);
1714+
}
17081715

1709-
/* Now, do we need a new page for the tuple, or not? */
1710-
if ((unsigned)MAXALIGN(newtup->t_len) <=PageGetFreeSpace((Page)dp))
1711-
newbuf=buffer;
1716+
/*
1717+
* Now, do we need a new page for the tuple, or not? This is a bit
1718+
* tricky since someone else could have added tuples to the page
1719+
* while we weren't looking. We have to recheck the available space
1720+
* after reacquiring the buffer lock. But don't bother to do that
1721+
* if the former amount of free space is still not enough; it's
1722+
* unlikely there's more free now than before.
1723+
*
1724+
* What's more, if we need to get a new page, we will need to acquire
1725+
* buffer locks on both old and new pages. To avoid deadlock against
1726+
* some other backend trying to get the same two locks in the other
1727+
* order, we must be consistent about the order we get the locks in.
1728+
* We use the rule "lock the higher-numbered page of the relation
1729+
* first". To implement this, we must do RelationGetBufferForTuple
1730+
* while not holding the lock on the old page, and we must tell it
1731+
* to give us a page beyond the old page.
1732+
*/
1733+
if (newtupsize>pagefree)
1734+
{
1735+
/* Assume there's no chance to put newtup on same page. */
1736+
newbuf=RelationGetBufferForTuple(relation,newtup->t_len,
1737+
BufferGetBlockNumber(buffer)+1);
1738+
/* Now reacquire lock on old tuple's page. */
1739+
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
1740+
}
17121741
else
1713-
newbuf=RelationGetBufferForTuple(relation,newtup->t_len);
1714-
1715-
/* Re-acquire the lock on the old tuple's page. */
1716-
/* this seems to be deadlock free... */
1717-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
1742+
{
1743+
/* Re-acquire the lock on the old tuple's page. */
1744+
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
1745+
/* Re-check using the up-to-date free space */
1746+
pagefree=PageGetFreeSpace((Page)dp);
1747+
if (newtupsize>pagefree)
1748+
{
1749+
/*
1750+
* Rats, it doesn't fit anymore. We must now unlock and
1751+
* relock to avoid deadlock. Fortunately, this path should
1752+
* seldom be taken.
1753+
*/
1754+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
1755+
newbuf=RelationGetBufferForTuple(relation,newtup->t_len,
1756+
BufferGetBlockNumber(buffer)+1);
1757+
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
1758+
}
1759+
else
1760+
{
1761+
/* OK, it fits here, so we're done. */
1762+
newbuf=buffer;
1763+
}
1764+
}
17181765
}
17191766
else
17201767
{
@@ -1723,6 +1770,11 @@ heap_update(Relation relation, ItemPointer otid, HeapTuple newtup,
17231770
newbuf=buffer;
17241771
}
17251772

1773+
/*
1774+
* At this point newbuf and buffer are both pinned and locked,
1775+
* and newbuf has enough space for the new tuple.
1776+
*/
1777+
17261778
/* NO ELOG(ERROR) from here till changes are logged */
17271779
START_CRIT_SECTION();
17281780

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

Lines changed: 49 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Id: hio.c,v 1.38 2001/05/12 19:58:27 tgl Exp $
11+
* $Id: hio.c,v 1.39 2001/05/16 22:35:12 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -66,18 +66,25 @@ RelationPutHeapTuple(Relation relation,
6666
/*
6767
* RelationGetBufferForTuple
6868
*
69-
* Returns exclusive-locked buffer with free space >= given len.
69+
*Returns exclusive-locked buffer with free space >= given len,
70+
*being careful to select only a page at or beyond minblocknum
71+
*in the relation.
7072
*
71-
* Note that we use LockPage to lock relation for extension. We can
72-
* do this as long as in all other places we use page-level locking
73-
* for indices only. Alternatively, we could define pseudo-table as
74-
* we do for transactions with XactLockTable.
73+
*The minblocknum parameter is needed to prevent deadlock between
74+
*concurrent heap_update operations; see heap_update for details.
75+
*Pass zero if you don't particularly care which page you get.
7576
*
76-
* ELOG(ERROR) is allowed here, so this routine *must* be called
77-
* before any (unlogged) changes are made in buffer pool.
77+
*Note that we use LockPage to lock relation for extension. We can
78+
*do this as long as in all other places we use page-level locking
79+
*for indices only. Alternatively, we could define pseudo-table as
80+
*we do for transactions with XactLockTable.
81+
*
82+
*ELOG(ERROR) is allowed here, so this routine *must* be called
83+
*before any (unlogged) changes are made in buffer pool.
7884
*/
7985
Buffer
80-
RelationGetBufferForTuple(Relationrelation,Sizelen)
86+
RelationGetBufferForTuple(Relationrelation,Sizelen,
87+
BlockNumberminblocknum)
8188
{
8289
Bufferbuffer=InvalidBuffer;
8390
PagepageHeader;
@@ -103,16 +110,19 @@ RelationGetBufferForTuple(Relation relation, Size len)
103110
if (relation->rd_nblocks>0)
104111
{
105112
lastblock=relation->rd_nblocks-1;
106-
buffer=ReadBuffer(relation,lastblock);
107-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
108-
pageHeader= (Page)BufferGetPage(buffer);
109-
if (len <=PageGetFreeSpace(pageHeader))
110-
returnbuffer;
111-
/*
112-
* Doesn't fit, so we'll have to try someplace else.
113-
*/
114-
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
115-
/* buffer release will happen below... */
113+
if (lastblock >=minblocknum)
114+
{
115+
buffer=ReadBuffer(relation,lastblock);
116+
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
117+
pageHeader= (Page)BufferGetPage(buffer);
118+
if (len <=PageGetFreeSpace(pageHeader))
119+
returnbuffer;
120+
/*
121+
* Doesn't fit, so we'll have to try someplace else.
122+
*/
123+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
124+
/* buffer release will happen below... */
125+
}
116126
}
117127

118128
/*
@@ -137,32 +147,38 @@ RelationGetBufferForTuple(Relation relation, Size len)
137147
*/
138148
relation->rd_nblocks=RelationGetNumberOfBlocks(relation);
139149

140-
if (relation->rd_nblocks>oldnblocks)
150+
if ((BlockNumber)relation->rd_nblocks>oldnblocks)
141151
{
142152
/*
143153
* Someone else has indeed extended the relation recently.
144154
* Try to fit our tuple into the new last page.
145155
*/
146156
lastblock=relation->rd_nblocks-1;
147-
buffer=ReleaseAndReadBuffer(buffer,relation,lastblock, false);
148-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
149-
pageHeader= (Page)BufferGetPage(buffer);
150-
if (len <=PageGetFreeSpace(pageHeader))
157+
if (lastblock >=minblocknum)
151158
{
152-
/* OK, we don't need to extend again. */
153-
if (!relation->rd_myxactonly)
154-
UnlockPage(relation,0,ExclusiveLock);
155-
returnbuffer;
159+
buffer=ReleaseAndReadBuffer(buffer,relation,lastblock, false);
160+
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
161+
pageHeader= (Page)BufferGetPage(buffer);
162+
if (len <=PageGetFreeSpace(pageHeader))
163+
{
164+
/* OK, we don't need to extend again. */
165+
if (!relation->rd_myxactonly)
166+
UnlockPage(relation,0,ExclusiveLock);
167+
returnbuffer;
168+
}
169+
/*
170+
* Doesn't fit, so we'll have to extend the relation (again).
171+
*/
172+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
173+
/* buffer release will happen below... */
156174
}
157-
/*
158-
* Doesn't fit, so we'll have to extend the relation (again).
159-
*/
160-
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
161-
/* buffer release will happen below... */
162175
}
163176

164177
/*
165178
* Extend the relation by one page and update rd_nblocks for next time.
179+
*
180+
* Note: at this point minblocknum is ignored; we won't extend by more
181+
* than one block...
166182
*/
167183
lastblock=relation->rd_nblocks;
168184
buffer=ReleaseAndReadBuffer(buffer,relation,lastblock, true);

‎src/include/access/hio.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2001, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $Id: hio.h,v 1.17 2001/01/24 19:43:19 momjian Exp $
10+
* $Id: hio.h,v 1.18 2001/05/16 22:35:12 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -18,6 +18,7 @@
1818

1919
externvoidRelationPutHeapTuple(Relationrelation,Bufferbuffer,
2020
HeapTupletuple);
21-
externBufferRelationGetBufferForTuple(Relationrelation,Sizelen);
21+
externBufferRelationGetBufferForTuple(Relationrelation,Sizelen,
22+
BlockNumberminblocknum);
2223

2324
#endif/* HIO_H */

0 commit comments

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