From 304160c3e270e6db75e109c149799d09b6f1461b Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 6 Jan 2006 00:04:20 +0000 Subject: [PATCH] Fix ReadBuffer() to correctly handle the case where it's trying to extend the relation but it finds a pre-existing valid buffer. The buffer does not correspond to any page known to the kernel, so we *must* do smgrextend to ensure that the space becomes allocated. The 7.x branches all do this correctly, but the corner case got lost somewhere during 8.0 bufmgr rewrites. (My fault no doubt :-( ... I think I assumed that such a buffer must be not-BM_VALID, which is not so.) --- src/backend/storage/buffer/bufmgr.c | 46 +++++++++++++++++++++++++---- 1 file changed, 40 insertions(+), 6 deletions(-) diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index a8a37e5ca3..11e83812d0 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.201 2005/12/29 18:08:05 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/storage/buffer/bufmgr.c,v 1.202 2006/01/06 00:04:20 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -164,13 +164,47 @@ ReadBuffer(Relation reln, BlockNumber blockNum) /* if it was already in the buffer pool, we're done */ if (found) { - /* Just need to update stats before we exit */ - pgstat_count_buffer_hit(&reln->pgstat_info, reln); + if (!isExtend) + { + /* Just need to update stats before we exit */ + pgstat_count_buffer_hit(&reln->pgstat_info, reln); - if (VacuumCostActive) - VacuumCostBalance += VacuumCostPageHit; + if (VacuumCostActive) + VacuumCostBalance += VacuumCostPageHit; - return BufferDescriptorGetBuffer(bufHdr); + return BufferDescriptorGetBuffer(bufHdr); + } + + /* + * We get here only in the corner case where we are trying to extend + * the relation but we found a pre-existing buffer marked BM_VALID. + * (This can happen because mdread doesn't complain about reads + * beyond EOF --- which is arguably bogus, but changing it seems + * tricky.) We *must* do smgrextend before succeeding, else the + * page will not be reserved by the kernel, and the next P_NEW call + * will decide to return the same page. Clear the BM_VALID bit, + * do the StartBufferIO call that BufferAlloc didn't, and proceed. + */ + if (isLocalBuf) + { + /* Only need to adjust flags */ + Assert(bufHdr->flags & BM_VALID); + bufHdr->flags &= ~BM_VALID; + } + else + { + /* + * Loop to handle the very small possibility that someone + * re-sets BM_VALID between our clearing it and StartBufferIO + * inspecting it. + */ + do { + LockBufHdr(bufHdr); + Assert(bufHdr->flags & BM_VALID); + bufHdr->flags &= ~BM_VALID; + UnlockBufHdr(bufHdr); + } while (!StartBufferIO(bufHdr, true)); + } } /*