8000 Repair very-low-probability race condition between relation extension · melor/postgres@cb0ddb6 · GitHub
[go: up one dir, main page]
Skip to content

Commit cb0ddb6

Browse files
tglsfdctglsfdc
committed
Repair very-low-probability race condition between relation extension
and VACUUM: in the interval between adding a new page to the relation and formatting it, it was possible for VACUUM to come along and decide it should format the page too. Though not harmful in itself, this would cause data loss if a third transaction were able to insert tuples into the vacuumed page before the original extender got control back.
1 parent 2e64824 commit cb0ddb6

File tree

2 files changed

+39
-11
lines changed

2 files changed

+39
-11
lines changed

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

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Id: hio.c,v 1.43 2001/10/25 05:49:21 momjian Exp $
11+
* $Id: hio.c,v 1.43.2.1 2005/05/07 21:34:20 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -245,13 +245,6 @@ RelationGetBufferForTuple(Relation relation, Size len,
245245
*/
246246
buffer = ReadBuffer(relation, P_NEW);
247247

248-
/*
249-
* Release the file-extension lock; it's now OK for someone else to
250-
* extend the relation some more.
251-
*/
252-
if (!relation->rd_myxactonly)
253-
UnlockPage(relation, 0, ExclusiveLock);
254-
255248
/*
256249
* We can be certain that locking the otherBuffer first is OK, since
257250
* it must have a lower page number.
@@ -260,9 +253,22 @@ RelationGetBufferForTuple(Relation relation, Size len,
260253
LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE);
261254

262255
/*
263-
* We need to initialize the empty new page.
256+
* Now acquire lock on the new page.
264257
*/
265258
LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
259+
260+
/*
261+
* Release the file-extension lock; it's now OK for someone else to
262+
* extend the relation some more. Note that we cannot release this
263+
* lock before we have buffer lock on the new page, or we risk a
264+
* race condition against vacuumlazy.c --- see comments therein.
265+
*/
266+
if (!relation->rd_myxactonly)
267+
UnlockPage(relation, 0, ExclusiveLock);
268+
269+
/*
270+
* We need to initialize the empty new page.
271+
*/
266272
pageHeader = (Page) BufferGetPage(buffer);
267273
Assert(PageIsNew((PageHeader) pageHeader));
268274
PageInit(pageHeader, BufferGetPageSize(buffer), 0);

‎src/backend/commands/vacuumlazy.c

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
*
3232
*
3333
* IDENTIFICATION
34-
* $Header: /cvsroot/pgsql/src/backend/commands/vacuumlazy.c,v 1.11 2002/01/06 00:37:44 tgl Exp $
34+
* $Header: /cvsroot/pgsql/src/backend/commands/vacuumlazy.c,v 1.11.2.1 2005/05/07 21:34:20 tgl Exp $
3535
*
3636
*-------------------------------------------------------------------------
3737
*/
@@ -259,8 +259,30 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
259259

260260
if (PageIsNew(page))
261261
{
262-
/* Not sure we still need to handle this case, but... */
262+
/*
263+
* An all-zeroes page could be left over if a backend extends
264+
* the relation but crashes before initializing the page.
265+
* Reclaim such pages for use.
266+
*
267+
* We have to be careful here because we could be looking at
268+
* a page that someone has just added to the relation and not
269+
* yet been able to initialize (see RelationGetBufferForTuple).
270+
* To interlock against that, release the buffer read lock
271+
* (which we must do anyway) and grab the relation extension
272< 92DD code class="diff-text syntax-highlighted-line addition">+
* lock before re-locking in exclusive mode. If the page is
273+
* still uninitialized by then, it must be left over from a
274+
* crashed backend, and we can initialize it.
275+
*
276+
* We don't really need the relation lock when this is a new
277+
* or temp relation, but it's probably not worth the code space
278+
* to check that, since this surely isn't a critical path.
279+
*
280+
* Note: the comparable code in vacuum.c need not do all this
281+
* because it's got exclusive lock on the whole relation.
282+
*/
263283
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
284+
LockPage(onerel, 0, ExclusiveLock);
285+
UnlockPage(onerel, 0, ExclusiveLock);
264286
LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
265287
if (PageIsNew(page))
266288
{

0 commit comments

Comments
 (0)
0