From f3ead161a0814a48b4d581495e19c17a4260d0b8 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 26 Oct 2026 11:59:08 +0900 Subject: [PATCH v2] Do not 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 resource ,anager, 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, could modify it before the record is copied into the WAL buffers. Theis could result is an intermittent "incorrect resource manager data checksum in record" failure on a standby. As of this commit, never take an image for a REGBUF_NO_CHANGE buffer, even under XLR_CHECK_CONSISTENCY. There is no need to do a consistency check on a block not changed by a record. verifyBackupPageConsistency() is taught to skip a registered block that carries no image rather than asserting one should be present. REGBUF_NO_CHANGE exists since 00d7fb5e2e39, so backpatch down to v17. On older branches, the CRC problem can be reproduced as well, but we lack the mechanism to check how a buffer is handled by the code assembling a record. Reported-by: Alexander Lakhin Author: Greg Burd Diagnosed-by: Thomas Munro Discussion: https://postgr.es/m/ats4pi-RM0Uom0T9@floki Backpatch-through: 17 --- src/include/access/xloginsert.h | 4 +++- src/backend/access/transam/README | 8 ++++++++ src/backend/access/transam/xloginsert.c | 9 ++++++++- src/backend/access/transam/xlogrecovery.c | 7 ++++++- 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/src/include/access/xloginsert.h b/src/include/access/xloginsert.h index 91dfbd5627f5..86264564ebb0 100644 --- a/src/include/access/xloginsert.h +++ b/src/include/access/xloginsert.h @@ -38,7 +38,9 @@ * 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; no + * image is forced for WAL consistency + * checking */ /* prototypes for public functions in xloginsert.c: */ extern void XLogBeginInsert(void); diff --git a/src/backend/access/transam/README b/src/backend/access/transam/README index 231106270fd4..3f4fb0a167dc 100644 --- a/src/backend/access/transam/README +++ b/src/backend/access/transam/README @@ -586,6 +586,14 @@ 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). + WAL consistency checking does not force a full-page image for such a + buffer, as it may be held with only a pin and be modified concurrently + while the record is assembled. Callers holding only a pin should also + pass REGBUF_NO_IMAGE, to avoid an image for a full-page write. + 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 59b0dfe436d1..c34d0c5ad838 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -721,8 +721,15 @@ 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. + * + * Do not force an image for a REGBUF_NO_CHANGE buffer. It may be + * held with only a pin, so the page could be modified concurrently + * while the record is assembled, invalidating the record CRC. */ - 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 20be788da78c..13f15728ce6d 100644 --- a/src/backend/access/transam/xlogrecovery.c +++ b/src/backend/access/transam/xlogrecovery.c @@ -2479,7 +2479,12 @@ verifyBackupPageConsistency(XLogReaderState *record) continue; } - Assert(XLogRecHasBlockImage(record, block_id)); + /* + * Blocks registered with REGBUF_NO_CHANGE may have no image, see + * XLogRecordAssemble(). Nothing to compare for these. + */ + if (!XLogRecHasBlockImage(record, block_id)) + continue; if (XLogRecBlockImageApply(record, block_id)) { -- 2.56.0