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
Appearance settings

Commit 061b079

Browse filesBrowse files
tglsfdctglsfdc
committed
Fix multiple bugs in index page locking during hot-standby WAL replay.
In ordinary operation, VACUUM must be careful to take a cleanup lock on each leaf page of a btree index; this ensures that no indexscans could still be "in flight" to heap tuples due to be deleted. (Because of possible index-tuple motion due to concurrent page splits, it's not enough to lock only the pages we're deleting index tuples from.) In Hot Standby, the WAL replay process must likewise lock every leaf page. There were several bugs in the code for that: * The replay scan might come across unused, all-zero pages in the index. While btree_xlog_vacuum itself did the right thing (ie, nothing) with such pages, xlogutils.c supposed that such pages must be corrupt and would throw an error. This accounts for various reports of replication failures with "PANIC: WAL contains references to invalid pages". To fix, add a ReadBufferMode value that instructs XLogReadBufferExtended not to complain when we're doing this. * btree_xlog_vacuum performed the extra locking if standbyState == STANDBY_SNAPSHOT_READY, but that's not the correct test: we won't open up for hot standby queries until the database has reached consistency, and we don't want to do the extra locking till then either, for fear of reading corrupted pages (which bufmgr.c would complain about). Fix by exporting a new function from xlog.c that will report whether we're actually in hot standby replay mode. * To ensure full coverage of the index in the replay scan, btvacuumscan would emit a dummy WAL record for the last page of the index, if no vacuuming work had been done on that page. However, if the last page of the index is all-zero, that would result in corruption of said page, since the functions called on it weren't prepared to handle that case. There's no need to lock any such pages, so change the logic to target the last normal leaf page instead. The first two of these bugs were diagnosed by Andres Freund, the other one by me. Fixes based on ideas from Heikki Linnakangas and myself. This has been wrong since Hot Standby was introduced, so back-patch to 9.0.
1 parent 16cad3e commit 061b079
Copy full SHA for 061b079

File tree

Expand file treeCollapse file tree

7 files changed

+98
-42
lines changed
Filter options
Expand file treeCollapse file tree

7 files changed

+98
-42
lines changed

‎src/backend/access/nbtree/nbtree.c

Copy file name to clipboardExpand all lines: src/backend/access/nbtree/nbtree.c
+41-28Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,8 @@ typedef struct
5656
IndexBulkDeleteCallback callback;
5757
void *callback_state;
5858
BTCycleId cycleid;
59-
BlockNumber lastBlockVacuumed; /* last blkno reached by Vacuum scan */
60-
BlockNumber lastUsedPage; /* blkno of last non-recyclable page */
59+
BlockNumber lastBlockVacuumed; /* highest blkno actually vacuumed */
60+
BlockNumber lastBlockLocked; /* highest blkno we've cleanup-locked */
6161
BlockNumber totFreePages; /* true total # of free pages */
6262
MemoryContext pagedelcontext;
6363
} BTVacState;
@@ -763,7 +763,7 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
763763
vstate.callback_state = callback_state;
764764
vstate.cycleid = cycleid;
765765
vstate.lastBlockVacuumed = BTREE_METAPAGE; /* Initialise at first block */
766-
vstate.lastUsedPage = BTREE_METAPAGE;
766+
vstate.lastBlockLocked = BTREE_METAPAGE;
767767
vstate.totFreePages = 0;
768768

769769
/* Create a temporary memory context to run _bt_pagedel in */
@@ -819,27 +819,30 @@ btvacuumscan(IndexVacuumInfo *info, IndexBulkDeleteResult *stats,
819819
}
820820

821821
/*
822-
* InHotStandby we need to scan right up to the end of the index for
823-
* correct locking, so we may need to write a WAL record for the final
824-
* block in the index if it was not vacuumed. It's possible that VACUUMing
825-
* has actually removed zeroed pages at the end of the index so we need to
826-
* take care to issue the record for last actual block and not for the
827-
* last block that was scanned. Ignore empty indexes.
822+
* If the WAL is replayed in hot standby, the replay process needs to get
823+
* cleanup locks on all index leaf pages, just as we've been doing here.
824+
* However, we won't issue any WAL records about pages that have no items
825+
* to be deleted. For pages between pages we've vacuumed, the replay code
826+
* will take locks under the direction of the lastBlockVacuumed fields in
827+
* the XLOG_BTREE_VACUUM WAL records. To cover pages after the last one
828+
* we vacuum, we need to issue a dummy XLOG_BTREE_VACUUM WAL record
829+
* against the last leaf page in the index, if that one wasn't vacuumed.
828830
*/
829831
if (XLogStandbyInfoActive() &&
830-
num_pages > 1 && vstate.lastBlockVacuumed < (num_pages - 1))
832+
vstate.lastBlockVacuumed < vstate.lastBlockLocked)
831833
{
832834
Buffer buf;
833835

834836
/*
835-
* We can't use _bt_getbuf() here because it always applies
836-
* _bt_checkpage(), which will barf on an all-zero page. We want to
837-
* recycle all-zero pages, not fail. Also, we want to use a
838-
* nondefault buffer access strategy.
837+
* The page should be valid, but we can't use _bt_getbuf() because we
838+
* want to use a nondefault buffer access strategy. Since we aren't
839+
* going to delete any items, getting cleanup lock again is probably
840+
* overkill, but for consistency do that anyway.
839841
*/
840-
buf = ReadBufferExtended(rel, MAIN_FORKNUM, num_pages - 1, RBM_NORMAL,
841-
info->strategy);
842+
buf = ReadBufferExtended(rel, MAIN_FORKNUM, vstate.lastBlockLocked,
843+
RBM_NORMAL, info->strategy);
842844
LockBufferForCleanup(buf);
845+
_bt_checkpage(rel, buf);
843846
_bt_delitems_vacuum(rel, buf, NULL, 0, vstate.lastBlockVacuumed);
844847
_bt_relbuf(rel, buf);
845848
}
@@ -914,10 +917,6 @@ btvacuumpage(BTVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno)
914917
}
915918
}
916919

917-
/* If the page is in use, update lastUsedPage */
918-
if (!_bt_page_recyclable(page) && vstate->lastUsedPage < blkno)
919-
vstate->lastUsedPage = blkno;
920-
921920
/* Page is valid, see what to do with it */
922921
if (_bt_page_recyclable(page))
923922
{
@@ -953,6 +952,13 @@ btvacuumpage(BTVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno)
953952
LockBuffer(buf, BUFFER_LOCK_UNLOCK);
954953
LockBufferForCleanup(buf);
955954

955+
/*
956+
* Remember highest leaf page number we've taken cleanup lock on; see
957+
* notes in btvacuumscan
958+
*/
959+
if (blkno > vstate->lastBlockLocked)
960+
vstate->lastBlockLocked = blkno;
961+
956962
/*
957963
* Check whether we need to recurse back to earlier pages. What we
958964
* are concerned about is a page split that happened since we started
@@ -1019,19 +1025,26 @@ btvacuumpage(BTVacState *vstate, BlockNumber blkno, BlockNumber orig_blkno)
10191025
*/
10201026
if (ndeletable > 0)
10211027
{
1022-
BlockNumber lastBlockVacuumed = BufferGetBlockNumber(buf);
1023-
1028+
/*
1029+
* Notice that the issued XLOG_BTREE_VACUUM WAL record includes an
1030+
* instruction to the replay code to get cleanup lock on all pages
1031+
* between the previous lastBlockVacuumed and this page. This
1032+
* ensures that WAL replay locks all leaf pages at some point.
1033+
*
1034+
* Since we can visit leaf pages out-of-order when recursing,
1035+
* replay might end up locking such pages an extra time, but it
1036+
* doesn't seem worth the amount of bookkeeping it'd take to avoid
1037+
* that.
1038+
*/
10241039
_bt_delitems_vacuum(rel, buf, deletable, ndeletable,
10251040
vstate->lastBlockVacuumed);
10261041

10271042
/*
1028-
* Keep track of the block number of the lastBlockVacuumed, so we
1029-
* can scan those blocks as well during WAL replay. This then
1030-
* provides concurrency protection and allows btrees to be used
1031-
* while in recovery.
1043+
* Remember highest leaf page number we've issued a
1044+
* XLOG_BTREE_VACUUM WAL record for.
10321045
*/
1033-
if (lastBlockVacuumed > vstate->lastBlockVacuumed)
1034-
vstate->lastBlockVacuumed = lastBlockVacuumed;
1046+
if (blkno > vstate->lastBlockVacuumed)
1047+
vstate->lastBlockVacuumed = blkno;
10351048

10361049
stats->tuples_removed += ndeletable;
10371050
/* must recompute maxoff */

‎src/backend/access/nbtree/nbtxlog.c

Copy file name to clipboardExpand all lines: src/backend/access/nbtree/nbtxlog.c
+30-11Lines changed: 30 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -482,28 +482,47 @@ btree_xlog_vacuum(XLogRecPtr lsn, XLogRecord *record)
482482
BTPageOpaque opaque;
483483

484484
/*
485-
* If queries might be active then we need to ensure every block is
485+
* If queries might be active then we need to ensure every leaf page is
486486
* unpinned between the lastBlockVacuumed and the current block, if there
487-
* are any. This ensures that every block in the index is touched during
488-
* VACUUM as required to ensure scans work correctly.
487+
* are any. This prevents replay of the VACUUM from reaching the stage of
488+
* removing heap tuples while there could still be indexscans "in flight"
489+
* to those particular tuples (see nbtree/README).
490+
*
491+
* It might be worth checking if there are actually any backends running;
492+
* if not, we could just skip this.
493+
*
494+
* Since VACUUM can visit leaf pages out-of-order, it might issue records
495+
* with lastBlockVacuumed >= block; that's not an error, it just means
496+
* nothing to do now.
497+
*
498+
* Note: since we touch all pages in the range, we will lock non-leaf
499+
* pages, and also any empty (all-zero) pages that may be in the index. It
500+
* doesn't seem worth the complexity to avoid that. But it's important
501+
* that HotStandbyActiveInReplay() will not return true if the database
502+
* isn't yet consistent; so we need not fear reading still-corrupt blocks
503+
* here during crash recovery.
489504
*/
490-
if (standbyState == STANDBY_SNAPSHOT_READY &&
491-
(xlrec->lastBlockVacuumed + 1) != xlrec->block)
505+
if (HotStandbyActiveInReplay())
492506
{
493-
BlockNumber blkno = xlrec->lastBlockVacuumed + 1;
507+
BlockNumber blkno;
494508

495-
for (; blkno < xlrec->block; blkno++)
509+
for (blkno = xlrec->lastBlockVacuumed + 1; blkno < xlrec->block; blkno++)
496510
{
497511
/*
512+
* We use RBM_NORMAL_NO_LOG mode because it's not an error
513+
* condition to see all-zero pages. The original btvacuumpage
514+
* scan would have skipped over all-zero pages, noting them in FSM
515+
* but not bothering to initialize them just yet; so we mustn't
516+
* throw an error here. (We could skip acquiring the cleanup lock
517+
* if PageIsNew, but it's probably not worth the cycles to test.)
518+
*
498519
* XXX we don't actually need to read the block, we just need to
499520
* confirm it is unpinned. If we had a special call into the
500521
* buffer manager we could optimise this so that if the block is
501522
* not in shared_buffers we confirm it as unpinned.
502-
*
503-
* Another simple optimization would be to check if there's any
504-
* backends running; if not, we could just skip this.
505523
*/
506-
buffer = XLogReadBufferExtended(xlrec->node, MAIN_FORKNUM, blkno, RBM_NORMAL);
524+
buffer = XLogReadBufferExtended(xlrec->node, MAIN_FORKNUM, blkno,
525+
RBM_NORMAL_NO_LOG);
507526
if (BufferIsValid(buffer))
508527
{
509528
LockBufferForCleanup(buffer);

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

Copy file name to clipboardExpand all lines: src/backend/access/transam/xlog.c
+13-1Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7562,7 +7562,8 @@ RecoveryInProgress(void)
75627562
* true. Postmaster knows this by way of signal, not via shared memory.
75637563
*
75647564
* Unlike testing standbyState, this works in any process that's connected to
7565-
* shared memory.
7565+
* shared memory. (And note that standbyState alone doesn't tell the truth
7566+
* anyway.)
75667567
*/
75677568
bool
75687569
HotStandbyActive(void)
@@ -7588,6 +7589,17 @@ HotStandbyActive(void)
75887589
}
75897590
}
75907591

7592+
/*
7593+
* Like HotStandbyActive(), but to be used only in WAL replay code,
7594+
* where we don't need to ask any other process what the state is.
7595+
*/
7596+
bool
7597+
HotStandbyActiveInReplay(void)
7598+
{
7599+
Assert(AmStartupProcess());
7600+
return LocalHotStandbyActive;
7601+
}
7602+
75917603
/*
75927604
* Is this process allowed to insert new WAL records?
75937605
*

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

Copy file name to clipboardExpand all lines: src/backend/access/transam/xlogutils.c
+6Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,10 @@ XLogReadBuffer(RelFileNode rnode, BlockNumber blkno, bool init)
288288
*
289289
* In RBM_ZERO and RBM_ZERO_ON_ERROR modes, if the page doesn't exist, the
290290
* relation is extended with all-zeroes pages up to the given block number.
291+
*
292+
* In RBM_NORMAL_NO_LOG mode, we return InvalidBuffer if the page doesn't
293+
* exist, and we don't check for all-zeroes. Thus, no log entry is made
294+
* to imply that the page should be dropped or truncated later.
291295
*/
292296
Buffer
293297
XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
@@ -328,6 +332,8 @@ XLogReadBufferExtended(RelFileNode rnode, ForkNumber forknum,
328332
log_invalid_page(rnode, forknum, blkno, false);
329333
return InvalidBuffer;
330334
}
335+
if (mode == RBM_NORMAL_NO_LOG)
336+
return InvalidBuffer;
331337
/* OK to extend the file */
332338
/* we do this in recovery only - no rel-extension lock needed */
333339
Assert(InRecovery);

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

Copy file name to clipboardExpand all lines: src/backend/storage/buffer/bufmgr.c
+4-1Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,8 @@ ReadBuffer(Relation reln, BlockNumber blockNum)
207207
* Assume when this function is called, that reln has been opened already.
208208
*
209209
* In RBM_NORMAL mode, the page is read from disk, and the page header is
210-
* validated. An error is thrown if the page header is not valid.
210+
* validated. An error is thrown if the page header is not valid. (But
211+
* note that an all-zero page is considered "valid"; see PageIsVerified().)
211212
*
212213
* RBM_ZERO_ON_ERROR is like the normal mode, but if the page header is not
213214
* valid, the page is zeroed instead of throwing an error. This is intended
@@ -221,6 +222,8 @@ ReadBuffer(Relation reln, BlockNumber blockNum)
221222
* current physical EOF; that is likely to cause problems in md.c when
222223
* the page is modified and written out. P_NEW is OK, though.
223224
*
225+
* RBM_NORMAL_NO_LOG mode is treated the same as RBM_NORMAL here.
226+
*
224227
* If strategy is not NULL, a nondefault buffer access strategy is used.
225228
* See buffer/README for details.
226229
*/

‎src/include/access/xlog.h

Copy file name to clipboardExpand all lines: src/include/access/xlog.h
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -300,6 +300,7 @@ extern void issue_xlog_fsync(int fd, XLogSegNo segno);
300300

301301
extern bool RecoveryInProgress(void);
302302
extern bool HotStandbyActive(void);
303+
extern bool HotStandbyActiveInReplay(void);
303304
extern bool XLogInsertAllowed(void);
304305
extern void GetXLogReceiptTime(TimestampTz *rtime, bool *fromStream);
305306
extern XLogRecPtr GetXLogReplayRecPtr(TimeLineID *replayTLI);

‎src/include/storage/bufmgr.h

Copy file name to clipboardExpand all lines: src/include/storage/bufmgr.h
+3-1Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,9 @@ typedef enum
3838
RBM_NORMAL, /* Normal read */
3939
RBM_ZERO, /* Don't read from disk, caller will
4040
* initialize */
41-
RBM_ZERO_ON_ERROR /* Read, but return an all-zeros page on error */
41+
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+
* replay; otherwise same as RBM_NORMAL */
4244
} ReadBufferMode;
4345

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

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.