From a453951dd9902d675b9b61e60afe9e02a00a22c8 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 22 Sep 2005 17:32:58 +0000 Subject: [PATCH] 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. --- src/backend/commands/vacuum.c | 15 +++++++++++---- src/backend/commands/vacuumlazy.c | 4 ++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c index 1e3a420c36..9813183cb9 100644 --- a/src/backend/commands/vacuum.c +++ b/src/backend/commands/vacuum.c @@ -13,7 +13,7 @@ * * * 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, page = BufferGetPage(buf); /* - * We don't bother to do LockBuffer(buf, BUFFER_LOCK_EXCLUSIVE) - * because we assume that holding exclusive lock on the relation - * will keep other backends from looking at the page. + * Since we are holding exclusive lock on the relation, no other + * backend can be accessing the page; however it is possible that + * 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->offsets_used = 0; @@ -1297,6 +1301,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel, vacpagecopy = copy_vac_page(vacpage); vpage_insert(vacuum_pages, vacpagecopy); vpage_insert(fraged_pages, vacpagecopy); + LockBuffer(buf, BUFFER_LOCK_UNLOCK); WriteBuffer(buf); continue; } @@ -1312,6 +1317,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel, vacpagecopy = copy_vac_page(vacpage); vpage_insert(vacuum_pages, vacpagecopy); vpage_insert(fraged_pages, vacpagecopy); + LockBuffer(buf, BUFFER_LOCK_UNLOCK); ReleaseBuffer(buf); continue; } @@ -1520,6 +1526,7 @@ scan_heap(VRelStats *vacrelstats, Relation onerel, else empty_end_pages = 0; + LockBuffer(buf, BUFFER_LOCK_UNLOCK); if (pgchanged) WriteBuffer(buf); else diff --git a/src/backend/commands/vacuumlazy.c b/src/backend/commands/vacuumlazy.c index f324b72673..88ff2d4da4 100644 --- a/src/backend/commands/vacuumlazy.c +++ b/src/backend/commands/vacuumlazy.c @@ -31,7 +31,7 @@ * * * 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, * 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 + * Note: the comparable code in vacuum.c need not worry * because it's got exclusive lock on the whole relation. */ LockBuffer(buf, BUFFER_LOCK_UNLOCK); -- 2.50.0