On Tue, Jul 14, 2026 at 3:42 PM Melanie Plageman <[email protected]> wrote: > > I've also finished all branch versions of this. Each one's code > differs, but I've managed to keep the commit messages the same, which > should help release notes. Note that REL_19_STABLE bumps > XLOG_PAGE_MAGIC. This would keep us from having to maintain the > backwards compatibility code there, which I think is what we want. > Here are those branches.
I've pushed the main commits to all branches where needed. There are three patches remaining that I proposed for master that do assorted validations: visibility map and PD_ALL_VISIBLE flag validations and some validation in the WAL machinery that registered buffers get used. I plan to commit 0001 and 0003 once my other patches have gone through the buildfarm. I'm considering holding onto 0002 until I add WAL-logging to the VM corruption repair code in heap_page_fix_vm_corruption() -- without which, wal_consistency_checking could fail for pages that had corruption fixed. - Melanie
From 17584bc5632da492b7a01a55993ae78d61d846e0 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Wed, 29 Apr 2026 10:20:20 -0400 Subject: [PATCH v6 1/3] Add VM corruption check to verify_heapam() pg_check_visible() can catch cases were tuples are not visible to all and the VM is set, but it doesn't specifically check page hints. This doesn't check, for example, an INSERT failing to clear PD_ALL_VISIBLE. This also helps users who have access to amcheck but not pg_visibility to diagnose corruption. Author: Melanie Plageman <[email protected]> Reviewed-by: Andres Freund <[email protected]> Discussion: https://postgr.es/m/flat/CAAKRu_bn%2Be7F4yPFBgFbnP%2BsyJRKyNK092bjD2LKvZW7O4Svag%40mail.gmail.com --- contrib/amcheck/verify_heapam.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/contrib/amcheck/verify_heapam.c b/contrib/amcheck/verify_heapam.c index 20ff58aa782..823f05e5916 100644 --- a/contrib/amcheck/verify_heapam.c +++ b/contrib/amcheck/verify_heapam.c @@ -481,6 +481,7 @@ verify_heapam(PG_FUNCTION_ARGS) while ((ctx.buffer = read_stream_next_buffer(stream, NULL)) != InvalidBuffer) { + uint8 vmbits; OffsetNumber maxoff; OffsetNumber predecessor[MaxOffsetNumber]; OffsetNumber successor[MaxOffsetNumber]; @@ -499,6 +500,24 @@ verify_heapam(PG_FUNCTION_ARGS) ctx.blkno = BufferGetBlockNumber(ctx.buffer); ctx.page = BufferGetPage(ctx.buffer); + /* + * It is corruption if the page-level PD_ALL_VISIBLE flag is clear and + * the visibility map bits corresponding to this heap page are set. + * Any corruption on the VM page itself won't be caught, and won't + * cause visibilitymap_get_status() to fail, because the VM page is + * read with ZERO_ON_ERROR. + */ + vmbits = visibilitymap_get_status(ctx.rel, ctx.blkno, &vmbuffer); + + if (!PageIsAllVisible(ctx.page) && + (vmbits & VISIBILITYMAP_VALID_BITS) != 0) + { + ctx.offnum = InvalidOffsetNumber; + ctx.attnum = -1; + report_corruption(&ctx, + psprintf("page is not marked all-visible in page header but visibility map bit is set")); + } + /* Perform tuple checks */ maxoff = PageGetMaxOffsetNumber(ctx.page); for (ctx.offnum = FirstOffsetNumber; ctx.offnum <= maxoff; -- 2.47.3
From 357ea85a4decebafaab750e7812f01ef90db6fb2 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Wed, 29 Apr 2026 10:33:46 -0400 Subject: [PATCH v6 2/3] Stop masking PD_ALL_VISIBLE when checking WAL consistency Setting and clearing PD_ALL_VISIBLE is WAL-logged, so there is no need to mask it for the purposes of WAL consistency checking. There is one minor exception, which is that when fixing VM corruption, we clear PD_ALL_VISIBLE on the heap page without specifically emitting WAL. This is a situation where corruption has already happened, however, so it is okay if WAL consistency checking flags this as well. Stop masking PD_ALL_VISIBLE, as doing so can only conceal real bugs. Author: Melanie Plageman <[email protected]> Reviewed-by: Andres Freund <[email protected]> Discussion: https://postgr.es/m/flat/CAAKRu_bn%2Be7F4yPFBgFbnP%2BsyJRKyNK092bjD2LKvZW7O4Svag%40mail.gmail.com --- src/backend/access/common/bufmask.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/backend/access/common/bufmask.c b/src/backend/access/common/bufmask.c index 5f63d04c9cb..93e34ea38b8 100644 --- a/src/backend/access/common/bufmask.c +++ b/src/backend/access/common/bufmask.c @@ -53,12 +53,6 @@ mask_page_hint_bits(Page page) /* Ignore PD_PAGE_FULL and PD_HAS_FREE_LINES flags, they are just hints. */ PageClearFull(page); PageClearHasFreeLinePointers(page); - - /* - * PD_ALL_VISIBLE is masked during WAL consistency checking. XXX: It is - * worth investigating if we could stop doing this. - */ - PageClearAllVisible(page); } /* -- 2.47.3
From 2d81d0d6194fb6df98f37f624943e80e37a27fc4 Mon Sep 17 00:00:00 2001 From: Melanie Plageman <[email protected]> Date: Wed, 29 Apr 2026 12:08:26 -0400 Subject: [PATCH v6 3/3] xlog: Verify all block references are used during replay It's unclear if this makes sense to actually commit. It requires that blocks read during recovery go through XLogReadBufferForRedoExtended() and has a special case for handling XLOG_FPI_FOR_HINT when full page writes are disabled. Author: Andres Freund <[email protected]> Co-authored-by: Melanie Plageman <[email protected]> Discussion: https://postgr.es/m/oqcsevg35xjan2327x5kdfth6q4fgeqboxfo3v3imeyih2uiny%406sez5dzxl6nt --- src/backend/access/transam/xlog.c | 10 ++++++++++ src/backend/access/transam/xlogreader.c | 4 ++++ src/backend/access/transam/xlogrecovery.c | 16 ++++++++++++++++ src/backend/access/transam/xlogutils.c | 6 ++++++ src/include/access/xlogreader.h | 4 ++++ 5 files changed, 40 insertions(+) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index f8b939853e9..40ba0ac7279 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -9084,6 +9084,16 @@ xlog_redo(XLogReaderState *record) { if (info == XLOG_FPI) elog(ERROR, "XLOG_FPI record did not contain a full-page image"); + +#ifdef USE_ASSERT_CHECKING + + /* + * If full_page_writes are disabled, we don't want to error + * out because we didn't read the block. + */ + if (info == XLOG_FPI_FOR_HINT) + record->record->blocks[block_id].used_read = true; +#endif continue; } diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index 946907a2507..8679a2435ed 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -1796,6 +1796,10 @@ DecodeXLogRecord(XLogReaderState *state, blk = &decoded->blocks[block_id]; blk->in_use = true; +#ifdef USE_ASSERT_CHECKING + blk->used_read = false; +#endif + blk->apply_image = false; COPY_HEADER_FIELD(&fork_flags, sizeof(uint8)); diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index a9ebac2d0ef..a5dfbfbdcef 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -1965,6 +1965,22 @@ ApplyWalRecord(XLogReaderState *xlogreader, XLogRecord *record, TimeLineID *repl /* Now apply the WAL record itself */ GetRmgr(record->xl_rmid).rm_redo(xlogreader); + /* + * Verify that all block references were used during replay. This helps + * detect bugs in redo routines. Every referenced block needs to be + * replayed with XLogReadBufferForRedoExtended(), possibly via a helper, + * to ensure that we replay FPIs, extend the relation if necessary, and + * other perform other similar protections. + */ +#ifdef USE_ASSERT_CHECKING + for (int block_id = 0; block_id <= xlogreader->record->max_block_id; block_id++) + { + DecodedBkpBlock *blk = &xlogreader->record->blocks[block_id]; + + Assert(!blk->in_use || blk->used_read); + } +#endif + /* * After redo, check whether the backup pages associated with the WAL * record are consistent with the existing pages. This check is done only diff --git a/src/backend/access/transam/xlogutils.c b/src/backend/access/transam/xlogutils.c index 58b9dab6a90..cc7c8a06089 100644 --- a/src/backend/access/transam/xlogutils.c +++ b/src/backend/access/transam/xlogutils.c @@ -381,6 +381,12 @@ XLogReadBufferForRedoExtended(XLogReaderState *record, block_id); } +#ifdef USE_ASSERT_CHECKING + /* Shouldn't have multiple read references to block */ + Assert(!record->record->blocks[block_id].used_read); + record->record->blocks[block_id].used_read = true; +#endif + /* * Make sure that if the block is marked with WILL_INIT, the caller is * going to initialize it. And vice versa. diff --git a/src/include/access/xlogreader.h b/src/include/access/xlogreader.h index 4a9a687e879..5b598b20859 100644 --- a/src/include/access/xlogreader.h +++ b/src/include/access/xlogreader.h @@ -145,6 +145,10 @@ typedef struct bool has_data; char *data; uint16 data_len; + +#ifdef USE_ASSERT_CHECKING + bool used_read; +#endif } DecodedBkpBlock; /* -- 2.47.3
