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

Commit681dbe7

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 parentef5a3b9 commit681dbe7

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
@@ -158,9 +158,8 @@ _hash_getinitbuf(Relation rel, BlockNumber blkno)
158158
if (blkno==P_NEW)
159159
elog(ERROR,"hash AM does not use P_NEW");
160160

161-
buf=ReadBufferExtended(rel,MAIN_FORKNUM,blkno,RBM_ZERO,NULL);
162-
163-
LockBuffer(buf,HASH_WRITE);
161+
buf=ReadBufferExtended(rel,MAIN_FORKNUM,blkno,RBM_ZERO_AND_LOCK,
162+
NULL);
164163

165164
/* ref count and lock type are correct */
166165

@@ -201,11 +200,13 @@ _hash_getnewbuf(Relation rel, BlockNumber blkno)
201200
if (BufferGetBlockNumber(buf)!=blkno)
202201
elog(ERROR,"unexpected hash relation size: %u, should be %u",
203202
BufferGetBlockNumber(buf),blkno);
203+
LockBuffer(buf,HASH_WRITE);
204204
}
205205
else
206-
buf=ReadBufferExtended(rel,MAIN_FORKNUM,blkno,RBM_ZERO,NULL);
207-
208-
LockBuffer(buf,HASH_WRITE);
206+
{
207+
buf=ReadBufferExtended(rel,MAIN_FORKNUM,blkno,RBM_ZERO_AND_LOCK,
208+
NULL);
209+
}
209210

210211
/* ref count and lock type are correct */
211212

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4299,9 +4299,8 @@ heap_xlog_newpage(XLogRecPtr lsn, XLogRecord *record)
42994299
* not do anything that assumes we are touching a heap.
43004300
*/
43014301
buffer=XLogReadBufferExtended(xlrec->node,xlrec->forknum,xlrec->blkno,
4302-
RBM_ZERO);
4302+
RBM_ZERO_AND_LOCK);
43034303
Assert(BufferIsValid(buffer));
4304-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
43054304
page= (Page)BufferGetPage(buffer);
43064305

43074306
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
@@ -3562,12 +3562,8 @@ RestoreBackupBlock(XLogRecPtr lsn, XLogRecord *record, int block_index,
35623562
{
35633563
/* Found it, apply the update */
35643564
buffer=XLogReadBufferExtended(bkpb.node,bkpb.fork,bkpb.block,
3565-
RBM_ZERO);
3565+
get_cleanup_lock ?RBM_ZERO_AND_CLEANUP_LOCK :RBM_ZERO_AND_LOCK);
35663566
Assert(BufferIsValid(buffer));
3567-
if (get_cleanup_lock)
3568-
LockBufferForCleanup(buffer);
3569-
else
3570-
LockBuffer(buffer,BUFFER_LOCK_EXCLUSIVE);
35713567

35723568
page= (Page)BufferGetPage(buffer);
35733569

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

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,16 +234,17 @@ XLogCheckInvalidPages(void)
234234
* The returned buffer is exclusively-locked.
235235
*
236236
* For historical reasons, instead of a ReadBufferMode argument, this only
237-
* supports RBM_ZERO (init == true) and RBM_NORMAL (init == false) modes.
237+
* supports RBM_ZERO_AND_LOCK (init == true) and RBM_NORMAL (init == false)
238+
* modes.
238239
*/
239240
Buffer
240241
XLogReadBuffer(RelFileNodernode,BlockNumberblkno,boolinit)
241242
{
242243
Bufferbuf;
243244

244245
buf=XLogReadBufferExtended(rnode,MAIN_FORKNUM,blkno,
245-
init ?RBM_ZERO :RBM_NORMAL);
246-
if (BufferIsValid(buf))
246+
init ?RBM_ZERO_AND_LOCK :RBM_NORMAL);
247+
if (BufferIsValid(buf)&& !init)
247248
LockBuffer(buf,BUFFER_LOCK_EXCLUSIVE);
248249

249250
returnbuf;
@@ -262,8 +263,8 @@ XLogReadBuffer(RelFileNode rnode, BlockNumber blkno, bool init)
262263
* dropped or truncated. If we don't see evidence of that later in the WAL
263264
* sequence, we'll complain at the end of WAL replay.)
264265
*
265-
* InRBM_ZERO and RBM_ZERO_ON_ERRORmodes, if the page doesn't exist, the
266-
*relation is extendedwith all-zeroes pages up to the given block number.
266+
* InRBM_ZERO_*modes, if the page doesn't exist, the relation is extended
267+
* with all-zeroes pages up to the given block number.
267268
*
268269
* In RBM_NORMAL_NO_LOG mode, we return InvalidBuffer if the page doesn't
269270
* exist, and we don't check for all-zeroes. Thus, no log entry is made
@@ -317,14 +318,20 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
317318
do
318319
{
319320
if (buffer!=InvalidBuffer)
321+
{
322+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK)
323+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
320324
ReleaseBuffer(buffer);
325+
}
321326
buffer=ReadBufferWithoutRelcache(rnode, false,forknum,
322327
P_NEW,mode,NULL);
323328
}
324329
while (BufferGetBlockNumber(buffer)<blkno);
325330
/* Handle the corner case that P_NEW returns non-consecutive pages */
326331
if (BufferGetBlockNumber(buffer)!=blkno)
327332
{
333+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK)
334+
LockBuffer(buffer,BUFFER_LOCK_UNLOCK);
328335
ReleaseBuffer(buffer);
329336
buffer=ReadBufferWithoutRelcache(rnode, false,forknum,blkno,
330337
mode,NULL);

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

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -204,14 +204,19 @@ ReadBuffer(Relation reln, BlockNumber blockNum)
204204
* valid, the page is zeroed instead of throwing an error. This is intended
205205
* for non-critical data, where the caller is prepared to repair errors.
206206
*
207-
* InRBM_ZERO mode, if the page isn't in buffer cache already, it's filled
208-
* with zeros instead of reading it from disk. Useful when the caller is
209-
* going to fill the page from scratch, since this saves I/O and avoids
207+
* InRBM_ZERO_AND_LOCK mode, if the page isn't in buffer cache already, it's
208+
*filledwith zeros instead of reading it from disk. Useful when the caller
209+
*isgoing to fill the page from scratch, since this saves I/O and avoids
210210
* unnecessary failure if the page-on-disk has corrupt page headers.
211+
* The page is returned locked to ensure that the caller has a chance to
212+
* initialize the page before it's made visible to others.
211213
* Caution: do not use this mode to read a page that is beyond the relation's
212214
* current physical EOF; that is likely to cause problems in md.c when
213215
* the page is modified and written out. P_NEW is OK, though.
214216
*
217+
* RBM_ZERO_AND_CLEANUP_LOCK is the same as RBM_ZERO_AND_LOCK, but acquires
218+
* a cleanup-strength lock on the page.
219+
*
215220
* RBM_NORMAL_NO_LOG mode is treated the same as RBM_NORMAL here.
216221
*
217222
* If strategy is not NULL, a nondefault buffer access strategy is used.
@@ -345,6 +350,18 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
345350
isExtend,
346351
found);
347352

353+
/*
354+
* In RBM_ZERO_AND_LOCK mode, the caller expects the buffer to
355+
* be already locked on return.
356+
*/
357+
if (!isLocalBuf)
358+
{
359+
if (mode==RBM_ZERO_AND_LOCK)
360+
LWLockAcquire(bufHdr->content_lock,LW_EXCLUSIVE);
361+
elseif (mode==RBM_ZERO_AND_CLEANUP_LOCK)
362+
LockBufferForCleanup(BufferDescriptorGetBuffer(bufHdr));
363+
}
364+
348365
returnBufferDescriptorGetBuffer(bufHdr);
349366
}
350367

@@ -425,8 +442,11 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
425442
* Read in the page, unless the caller intends to overwrite it and
426443
* just wants us to allocate a buffer.
427444
*/
428-
if (mode==RBM_ZERO)
445+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK||
446+
mode==RBM_DO_NOT_USE)
447+
{
429448
MemSet((char*)bufBlock,0,BLCKSZ);
449+
}
430450
else
431451
{
432452
smgrread(smgr,forkNum,blockNum, (char*)bufBlock);
@@ -453,6 +473,19 @@ ReadBuffer_common(SMgrRelation smgr, bool isLocalBuf, ForkNumber forkNum,
453473
}
454474
}
455475

476+
/*
477+
* In RBM_ZERO_AND_LOCK mode, grab the buffer content lock before marking
478+
* the page as valid, to make sure that no other backend sees the zeroed
479+
* page before the caller has had a chance to initialize it.
480+
*
481+
* Since no-one else can be looking at the page contents yet, there is no
482+
* difference between an exclusive lock and a cleanup-strength lock.
483+
* (Note that we cannot use LockBuffer() of LockBufferForCleanup() here,
484+
* because they assert that the buffer is already valid.)
485+
*/
486+
if (mode==RBM_ZERO_AND_LOCK||mode==RBM_ZERO_AND_CLEANUP_LOCK)
487+
LWLockAcquire(bufHdr->content_lock,LW_EXCLUSIVE);
488+
456489
if (isLocalBuf)
457490
{
458491
/* 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