Commit a453951d authored by Tom Lane's avatar Tom Lane

Take exclusive buffer lock in scan_heap() to eliminate some corner cases

in which invalid page data could be transiently written to disk by
concurrent bgwriter activity.  There doesn't seem any risk of loss of
actual user data, but an empty page could possibly be left corrupt if a
crash occurs before the correct data gets written out.  Pointed out by
Alvaro Herrera.
parent 4f915cd3
...@@ -13,7 +13,7 @@ ...@@ -13,7 +13,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.314 2005/09/02 19:02:19 tgl Exp $ * $PostgreSQL: pgsql/src/backend/commands/vacuum.c,v 1.315 2005/09/22 17:32:58 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
...@@ -1273,10 +1273,14 @@ scan_heap(VRelStats *vacrelstats, Relation onerel, ...@@ -1273,10 +1273,14 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
page = BufferGetPage(buf); page = BufferGetPage(buf);
/* /*
* We don't bother to do LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE) * Since we are holding exclusive lock on the relation, no other
* because we assume that holding exclusive lock on the relation * backend can be accessing the page; however it is possible that
* will keep other backends from looking at the page. * the background writer will try to write the page if it's already
* marked dirty. To ensure that invalid data doesn't get written to
* disk, we must take exclusive buffer lock wherever we potentially
* modify pages.
*/ */
LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE);
vacpage->blkno = blkno; vacpage->blkno = blkno;
vacpage->offsets_used = 0; vacpage->offsets_used = 0;
...@@ -1297,6 +1301,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel, ...@@ -1297,6 +1301,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
vacpagecopy = copy_vac_page(vacpage); vacpagecopy = copy_vac_page(vacpage);
vpage_insert(vacuum_pages, vacpagecopy); vpage_insert(vacuum_pages, vacpagecopy);
vpage_insert(fraged_pages, vacpagecopy); vpage_insert(fraged_pages, vacpagecopy);
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
WriteBuffer(buf); WriteBuffer(buf);
continue; continue;
} }
...@@ -1312,6 +1317,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel, ...@@ -1312,6 +1317,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
vacpagecopy = copy_vac_page(vacpage); vacpagecopy = copy_vac_page(vacpage);
vpage_insert(vacuum_pages, vacpagecopy); vpage_insert(vacuum_pages, vacpagecopy);
vpage_insert(fraged_pages, vacpagecopy); vpage_insert(fraged_pages, vacpagecopy);
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
ReleaseBuffer(buf); ReleaseBuffer(buf);
continue; continue;
} }
...@@ -1520,6 +1526,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel, ...@@ -1520,6 +1526,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel,
else else
empty_end_pages = 0; empty_end_pages = 0;
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
if (pgchanged) if (pgchanged)
WriteBuffer(buf); WriteBuffer(buf);
else else
......
...@@ -31,7 +31,7 @@ ...@@ -31,7 +31,7 @@
* *
* *
* IDENTIFICATION * IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.58 2005/09/02 19:02:20 tgl Exp $ * $PostgreSQL: pgsql/src/backend/commands/vacuumlazy.c,v 1.59 2005/09/22 17:32:58 tgl Exp $
* *
*------------------------------------------------------------------------- *-------------------------------------------------------------------------
*/ */
...@@ -299,7 +299,7 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats, ...@@ -299,7 +299,7 @@ lazy_scan_heap(Relation onerel, LVRelStats *vacrelstats,
* or temp relation, but it's probably not worth the code space * or temp relation, but it's probably not worth the code space
* to check that, since this surely isn't a critical path. * to check that, since this surely isn't a critical path.
* *
* Note: the comparable code in vacuum.c need not do all this * Note: the comparable code in vacuum.c need not worry
* because it's got exclusive lock on the whole relation. * because it's got exclusive lock on the whole relation.
*/ */
LockBuffer(buf, BUFFER_LOCK_UNLOCK); LockBuffer(buf, BUFFER_LOCK_UNLOCK);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment