From 09a645463af50abdfb3b9ea20decee42ff88d4b1 Mon Sep 17 00:00:00 2001 From: Greg Burd Date: Thu, 22 Oct 2026 14:43:17 -0400 Subject: [PATCH] Don't take a full-page image for REGBUF_NO_CHANGE buffers A buffer registered with REGBUF_NO_CHANGE is not modified by the record and may be held with only a pin (no content lock). The hash index code registers the primary bucket page this way in hashbucketcleanup(), _hash_freeovflpage() and _hash_squeezebucket(), solely to force a cleanup lock on that block during replay. When wal_consistency_checking includes the hash RM, XLogRecordAssemble() included a full-page image for such a block anyway, because XLR_CHECK_CONSISTENCY forces an image regardless of REGBUF_NO_IMAGE. The record CRC is then computed over the image while a concurrent backend, which legitimately holds nothing stronger than a pin on that page, is free to modify it before the record is copied into the WAL buffers. The result is an intermittent "incorrect resource manager data checksum in record" failure on a standby (or under a crash-recovery consistency check), reproducible on the buildfarm with wal_consistency_checking and concurrent inserts plus autovacuum on a hash index. Fix by never taking an image for a REGBUF_NO_CHANGE buffer, even under XLR_CHECK_CONSISTENCY. There is nothing to consistency-check on a block the record does not change, and the page is not safe to read while assembling the record. verifyBackupPageConsistency() is taught to skip a registered block that carries no image rather than asserting one is present. Reported-by: Alexander Lakhin Diagnosed-by: Thomas Munro Discussion: https://postgr.es/m/055bb33c-bccc-bc1d-c2f8-8598534448ac@gmail.com --- src/backend/access/transam/README | 9 +++++++++ src/backend/access/transam/xloginsert.c | 15 ++++++++++++++- src/backend/access/transam/xlogrecovery.c | 8 +++++++- src/include/access/xloginsert.h | 7 ++++++- 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/backend/access/transam/README b/src/backend/access/transam/README index 231106270fd..138c4c0ada6 100644 --- a/src/backend/access/transam/README +++ b/src/backend/access/transam/README @@ -586,6 +586,15 @@ void XLogRegisterBuffer(uint8 block_id, Buffer buf, uint8 flags); XLogRegisterBufData() is included in the WAL record even if a full-page image is taken. + The REGBUF_NO_CHANGE flag indicates that the buffer is registered without + being modified by this record, and possibly without an exclusive content + lock (for example, to force a cleanup lock on the block during replay). + Such a buffer never has a full-page image taken, not even when WAL + consistency checking is enabled: its contents may change concurrently + while the record is assembled, which would compute the record CRC over + data a concurrent backend can still modify, and there is nothing to + consistency-check on a block the record does not change. + void XLogRegisterData(const void *data, int len); XLogRegisterData is used to include arbitrary data in the WAL record. If diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index 59b0dfe436d..c3c95f89276 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -721,8 +721,21 @@ XLogRecordAssemble(RmgrId rmid, uint8 info, /* * If needs_backup is true or WAL checking is enabled for current * resource manager, log a full-page write for the current block. + * + * However, a buffer registered with REGBUF_NO_CHANGE is not modified + * by this record and may be held with only a pin (no content lock), + * so its contents could change under us while we assemble the record. + * Including such a page would compute the record CRC over data that a + * concurrent backend can modify before the record is copied into the + * WAL buffers, producing a spurious "incorrect resource manager data + * checksum" failure. These blocks are registered only to force a + * cleanup lock during replay, so there is nothing to consistency-check + * anyway; never take an image for them. */ - include_image = needs_backup || (info & XLR_CHECK_CONSISTENCY) != 0; + if (regbuf->flags & REGBUF_NO_CHANGE) + include_image = needs_backup; + else + include_image = needs_backup || (info & XLR_CHECK_CONSISTENCY) != 0; if (include_image) { diff --git a/src/backend/access/transam/xlogrecovery.c b/src/backend/access/transam/xlogrecovery.c index 20be788da78..52cb5cdc0a1 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -2479,7 +2479,13 @@ verifyBackupPageConsistency(XLogReaderState *record) continue; } - Assert(XLogRecHasBlockImage(record, block_id)); + /* + * A block registered with REGBUF_NO_CHANGE is not modified by this + * record and carries no full-page image (see XLogRecordAssemble), so + * there is nothing to consistency-check for it. Skip it. + */ + if (!XLogRecHasBlockImage(record, block_id)) + continue; if (XLogRecBlockImageApply(record, block_id)) { diff --git a/src/include/access/xloginsert.h b/src/include/access/xloginsert.h index 91dfbd5627f..6ae2692d5b9 100644 --- a/src/include/access/xloginsert.h +++ b/src/include/access/xloginsert.h @@ -38,7 +38,12 @@ * will be skipped) */ #define REGBUF_KEEP_DATA 0x10 /* include data even if a full-page image * is taken */ -#define REGBUF_NO_CHANGE 0x20 /* intentionally register clean buffer */ +#define REGBUF_NO_CHANGE 0x20 /* intentionally register clean buffer; + * suppresses the full-page image that + * XLR_CHECK_CONSISTENCY would otherwise + * take, since the page is not changed by + * this record and may not be + * content-locked */ /* prototypes for public functions in xloginsert.c: */ extern void XLogBeginInsert(void); -- 2.54.0