Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7eab804

Browse files
committed
Fix race condition between hot standby and restoring a full-page image.
There was a window in RestoreBackupBlock where a page would be zeroed out,but not yet locked. If a backend pinned and locked the page in that window,it saw the zeroed page instead of the old page or new page contents, whichcould lead to missing rows in a result set, or errors.To fix, replace RBM_ZERO with RBM_ZERO_AND_LOCK, which atomically pins,zeroes, and locks the page, if it's not in the buffer cache already.In stable branches, the old RBM_ZERO constant is renamed to RBM_DO_NOT_USE,to avoid breaking any 3rd party extensions that might use RBM_ZERO. Moreimportantly, this avoids renumbering the other enum values, which wouldcause even bigger confusion in extensions that use ReadBufferExtended, buthaven't been recompiled.Backpatch to all supported versions; this has been racy since hot standbywas introduced.
1 parentd47fff3 commit7eab804

File tree

6 files changed

+66
-25
lines changed

6 files changed

+66
-25
lines changed

‎src/backend/access/hash/hashpage.c‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -155,9 +155,8 @@ _hash_getinitbuf(Relation rel, BlockNumber blkno)
155155
if (blkno==P_NEW)
156156
elog(ERROR,"hash AM does not use P_NEW");
157157

158-
buf=ReadBufferExtended(rel,MAIN_FORKNUM,blkno,RBM_ZERO,NULL);
159-
160-
LockBuffer(buf,HASH_WRITE);
158+
buf=ReadBufferExtended(rel,MAIN_FORKNUM,blkno,RBM_ZERO_AND_LOCK,
159+
NULL);
161160

162161
/* ref count and lock type are correct */
163162

@@ -198,11 +197,13 @@ _hash_getnewbuf(Relation rel, BlockNumber blkno, ForkNumber forkNum)
198197
if (BufferGetBlockNumber(buf)!=blkno)
199198
elog(ERROR,"unexpected hash relation size: %u, should be %u",
200199
BufferGetBlockNumber(buf),blkno);
200+
LockBuffer(buf,HASH_WRITE);
201201
}
202202
else
203-
buf=ReadBufferExtended(rel,forkNum,blkno,RBM_ZERO,NULL);
204-
205-
LockBuffer(buf,HASH_WRITE);
203+
{
204+
buf=ReadBufferExtended(rel,forkNum,blkno,RBM_ZERO_AND_LOCK,
205+
NULL);
206+
}
206207

207208
/* ref count and lock type are correct */
208209

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4884,9 +4884,8 @@ heap_xlog_newpage(XLogRecPtr lsn, XLogRecord *record)
48844884
* not do anything that assumes we are touching a heap.
48854885
*/
48864886
buffer=XLogReadBufferExtended(xlrec->node,xlrec->forknum,xlrec->blkno,
4887-
RBM_ZERO);
4887+
RBM_ZERO_AND_LOCK);
48884888
Assert(BufferIsValid(buffer));
4889-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
48904889
page= (Page)BufferGetPage(buffer);
48914890

48924891
Assert(record->xl_len==SizeOfHeapNewpage+BLCKSZ);

‎src/backend/access/transam/xlog.c‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3813,12 +3813,8 @@ RestoreBackupBlock(XLogRecPtr lsn, XLogRecord *record, int block_index,
38133813
{
38143814
/* Found it, apply the update */
38153815
buffer=XLogReadBufferExtended(bkpb.node,bkpb.fork,bkpb.block,
3816-
RBM_ZERO);
3816+
get_cleanup_lock ?RBM_ZERO_AND_CLEANUP_LOCK :RBM_ZERO_AND_LOCK);
38173817
Assert(BufferIsValid(buffer));
3818-
if (get_cleanup_lock)
3819-
LockBufferForCleanup(buffer);
3820-
else
3821-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
38223818

38233819
page= (Page)BufferGetPage(buffer);
38243820

‎src/backend/access/transam/xlogutils.c‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -257,16 +257,17 @@ XLogCheckInvalidPages(void)
257257
* The returned buffer is exclusively-locked.
258258
*
259259
* For historical reasons, instead of a ReadBufferMode argument, this only
260-
* supports RBM_ZERO (init == true) and RBM_NORMAL (init == false) modes.
260+
* supports RBM_ZERO_AND_LOCK (init == true) and RBM_NORMAL (init == false)
261+
* modes.
261262
*/
262263
Buffer
263264
XLogReadBuffer(RelFileNodernode,BlockNumberblkno,boolinit)
264265
{
265266
Bufferbuf;
266267

267268
buf=XLogReadBufferExtended(rnode,MAIN_FORKNUM,blkno,
268-
init ?RBM_ZERO :RBM_NORMAL);
269-
if (BufferIsValid(buf))
269+
init ?RBM_ZERO_AND_LOCK :RBM_NORMAL);
270+
if (BufferIsValid(buf)&& !init)
270271
LockBuffer(buf,BUFFER_LOCK_EXCLUSIVE);
271272

272273
returnbuf;
@@ -285,8 +286,8 @@ XLogReadBuffer(RelFileNode rnode, BlockNumber blkno, bool init)
285286
* dropped or truncated. If we don't see evidence of that later in the WAL
286287
* sequence, we'll complain at the end of WAL replay.)
287288
*
288-
* InRBM_ZERO and RBM_ZERO_ON_ERRORmodes, if the page doesn't exist, the
289-
*relation is extendedwith all-zeroes pages up to the given block number.
289+
* InRBM_ZERO_*modes, if the page doesn't exist, the relation is extended
290+
* with all-zeroes pages up to the given block number.
290291
*
291292
* In RBM_NORMAL_NO_LOG mode, we return InvalidBuffer if the page doesn't
292293
* exist, and we don't check for all-zeroes. Thus, no log entry is made
@@ -340,14 +341,20 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
340341
do
341342
{
342343
if (buffer!=InvalidBuffer)
344+
{
345+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK)
346+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
343347
ReleaseBuffer(buffer);
348+
}
344349
buffer=ReadBufferWithoutRelcache(rnode,forknum,
345350
P_NEW,mode,NULL);
346351
}
347352
while (BufferGetBlockNumber(buffer)<blkno);
348353
/* Handle the corner case that P_NEW returns non-consecutive pages */
349354
if (BufferGetBlockNumber(buffer)!=blkno)
350355
{
356+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK)
357+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
351358
ReleaseBuffer(buffer);
352359
buffer=ReadBufferWithoutRelcache(rnode,forknum,blkno,
353360
mode,NULL);

‎src/backend/storage/buffer/bufmgr.c‎

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -210,14 +210,19 @@ ReadBuffer(Relation reln, BlockNumber blockNum)
210210
* valid, the page is zeroed instead of throwing an error. This is intended
211211
* for non-critical data, where the caller is prepared to repair errors.
212212
*
213-
* InRBM_ZERO mode, if the page isn't in buffer cache already, it's filled
214-
* with zeros instead of reading it from disk. Useful when the caller is
215-
* going to fill the page from scratch, since this saves I/O and avoids
213+
* InRBM_ZERO_AND_LOCK mode, if the page isn't in buffer cache already, it's
214+
*filledwith zeros instead of reading it from disk. Useful when the caller
215+
*isgoing to fill the page from scratch, since this saves I/O and avoids
216216
* unnecessary failure if the page-on-disk has corrupt page headers.
217+
* The page is returned locked to ensure that the caller has a chance to
218+
* initialize the page before it's made visible to others.
217219
* Caution: do not use this mode to read a page that is beyond the relation's
218220
* current physical EOF; that is likely to cause problems in md.c when
219221
* the page is modified and written out. P_NEW is OK, though.
220222
*
223+
* RBM_ZERO_AND_CLEANUP_LOCK is the same as RBM_ZERO_AND_LOCK, but acquires
224+
* a cleanup-strength lock on the page.
225+
*
221226
* RBM_NORMAL_NO_LOG mode is treated the same as RBM_NORMAL here.
222227
*
223228
* If strategy is not NULL, a nondefault buffer access strategy is used.
@@ -359,6 +364,18 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
359364
isExtend,
360365
found);
361366

367+
/*
368+
* In RBM_ZERO_AND_LOCK mode, the caller expects the buffer to
369+
* be already locked on return.
370+
*/
371+
if (!isLocalBuf)
372+
{
373+
if (mode==RBM_ZERO_AND_LOCK)
374+
LWLockAcquire(bufHdr->content_lock,LW_EXCLUSIVE);
375+
elseif (mode==RBM_ZERO_AND_CLEANUP_LOCK)
376+
LockBufferForCleanup(BufferDescriptorGetBuffer(bufHdr));
377+
}
378+
362379
returnBufferDescriptorGetBuffer(bufHdr);
363380
}
364381

@@ -439,8 +456,11 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
439456
* Read in the page, unless the caller intends to overwrite it and
440457
* just wants us to allocate a buffer.
441458
*/
442-
if (mode==RBM_ZERO)
459+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK||
460+
mode==RBM_DO_NOT_USE)
461+
{
443462
MemSet((char*)bufBlock,0,BLCKSZ);
463+
}
444464
else
445465
{
446466
instr_timeio_start,
@@ -481,6 +501,19 @@ ReadBuffer_common(SMgrRelation smgr, char relpersistence, ForkNumber forkNum,
481501
}
482502
}
483503

504+
/*
505+
* In RBM_ZERO_AND_LOCK mode, grab the buffer content lock before marking
506+
* the page as valid, to make sure that no other backend sees the zeroed
507+
* page before the caller has had a chance to initialize it.
508+
*
509+
* Since no-one else can be looking at the page contents yet, there is no
510+
* difference between an exclusive lock and a cleanup-strength lock.
511+
* (Note that we cannot use LockBuffer() of LockBufferForCleanup() here,
512+
* because they assert that the buffer is already valid.)
513+
*/
514+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK)
515+
LWLockAcquire(bufHdr->content_lock,LW_EXCLUSIVE);
516+
484517
if (isLocalBuf)
485518
{
486519
/* Only need to adjust flags */

‎src/include/storage/bufmgr.h‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,16 @@ typedef enum BufferAccessStrategyType
3636
typedefenum
3737
{
3838
RBM_NORMAL,/* Normal read */
39-
RBM_ZERO,/* Don't read from disk, caller will
40-
* initialize */
39+
RBM_DO_NOT_USE,/* This used to be RBM_ZERO. Only kept for
40+
* binary compatibility with 3rd party
41+
* extensions. */
4142
RBM_ZERO_ON_ERROR,/* Read, but return an all-zeros page on error */
42-
RBM_NORMAL_NO_LOG/* Don't log page as invalid during WAL
43+
RBM_NORMAL_NO_LOG,/* Don't log page as invalid during WAL
4344
* replay; otherwise same as RBM_NORMAL */
45+
RBM_ZERO_AND_LOCK,/* Don't read from disk, caller will
46+
* initialize. Also locks the page. */
47+
RBM_ZERO_AND_CLEANUP_LOCK/* Like RBM_ZERO_AND_LOCK, but locks the page
48+
* in "cleanup" mode */
4449
}ReadBufferMode;
4550

4651
/* in globals.c ... this duplicates miscadmin.h */

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp