projects / postgresql.git / commitdiff
summary | shortlog | log | commit | commitdiff | tree
raw | patch | inline | side by side (parent: 2e64824)
Repair very-low-probability race condition between relation extension
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 7 May 2005 21:34:20 +0000 (21:34 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 7 May 2005 21:34:20 +0000 (21:34 +0000)
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.

src/backend/access/heap/hio.c patch | blob | blame | history
src/backend/commands/vacuumlazy.c patch | blob | blame | history

diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index 44ecb3c8c7101894f139abfc48beada158f0a377..965a7d52541dd336e0e14db390124776245eb699 100644 (file)
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $Id: hio.c,v 1.43 2001/10/25 05:49:21 momjian Exp $
+ *   $Id: hio.c,v 1.43.2.1 2005/05/07 21:34:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -245,13 +245,6 @@ RelationGetBufferForTuple(Relation relation, Size len,
     */
    buffer = ReadBuffer(relation, P_NEW);
 
-   /*
-    * Release the file-extension lock; it's now OK for someone else to
-    * extend the relation some more.
-    */
-   if (!relation->rd_myxactonly)
-       UnlockPage(relation, 0, ExclusiveLock);
-
    /*
     * We can be certain that locking the otherBuffer first is OK, since
     * it must have a lower page number.
@@ -260,9 +253,22 @@ RelationGetBufferForTuple(Relation relation, Size len,
        LockBuffer(otherBuffer, BUFFER_LOCK_EXCLUSIVE);
 
    /*
-    * We need to initialize the empty new page.
+    * Now acquire lock on the new page.
     */
    LockBuffer(buffer, BUFFER_LOCK_EXCLUSIVE);
+
+   /*
+    * Release the file-extension lock; it's now OK for someone else to
+    * extend the relation some more.  Note that we cannot release this
+    * lock before we have buffer lock on the new page, or we risk a
+    * race condition against vacuumlazy.c --- see comments therein.
+    */
+   if (!relation->rd_myxactonly)
+       UnlockPage(relation, 0, ExclusiveLock);
+
+   /*
+    * We need to initialize the empty new page.
+    */
    pageHeader = (Page) BufferGetPage(buffer);
    Assert(PageIsNew((PageHeader) pageHeader));
    PageInit(pageHeader, BufferGetPageSize(buffer), 0);
diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c
index 088ad9c8aedad16e10e297cc7c0db9251882cc9d..c87e4192621a1ad0210053e1dd4fc6d9ffa8a75a 100644 (file)
--- a/src/backend/commands/vacuumlazy.c
+++ b/src/backend/commands/vacuumlazy.c
@@ -31,7 +31,7 @@
  *
  *
  * IDENTIFICATION
- *   $Header: /cvsroot/pgsql/src/backend/commands/vacuumlazy.c,v 1.11 2002/01/06 00:37:44 tgl Exp $
+ *   $Header: /cvsroot/pgsql/src/backend/commands/vacuumlazy.c,v 1.11.2.1 2005/05/07 21:34:20 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -259,8 +259,30 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
 
        if (PageIsNew(page))
        {
-           /* Not sure we still need to handle this case, but... */
+           /*
+            * An all-zeroes page could be left over if a backend extends
+            * the relation but crashes before initializing the page.
+            * Reclaim such pages for use.
+            *
+            * We have to be careful here because we could be looking at
+            * a page that someone has just added to the relation and not
+            * yet been able to initialize (see RelationGetBufferForTuple).
+            * To interlock against that, release the buffer read lock
+            * (which we must do anyway) and grab the relation extension
+            * lock before re-locking in exclusive mode.  If the page is
+            * still uninitialized by then, it must be left over from a
+            * crashed backend, and we can initialize it.
+            *
+            * We don't really need the relation lock when this is a new
+            * or temp relation, but it's probably not worth the code space
+            * to check that, since this surely isn't a critical path.
+            *
+            * Note: the comparable code in vacuum.c need not do all this
+            * because it's got exclusive lock on the whole relation.
+            */
            LockBuffer(buf, BUFFER_LOCK_UNLOCK);
+           LockPage(onerel, 0, ExclusiveLock);
+           UnlockPage(onerel, 0, ExclusiveLock);
            LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
            if (PageIsNew(page))
            {
This is the main PostgreSQL git repository.