From a59af1da608a84e1470712c5defde821279786f9 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 13:04:10 +0000 Subject: [PATCH 1/2] Add assert-build cross-check that dirtied buffers are WAL-registered The WAL protocol requires every shared buffer modified by a WAL-logged operation to be registered with the WAL record describing the operation (XLogRegisterBuffer/XLogRegisterBlock). Replay can survive violations of this rule when the redo routine finds the page by other means, but the WAL summarizer only sees registered block references, so an unregistered modified page is silently omitted from incremental backups; it also never receives full-page images, leaving it exposed to torn writes. Commit ed62d26 fixed a long-standing violation of exactly this kind (heap records clearing visibility map bits without registering the VM page), which corrupted incremental backups. Nothing enforces the invariant mechanically. In assertion-enabled builds, cross-check it: MarkBufferDirty() records every permanent shared buffer dirtied inside a critical section (XLogRegCheckBufferDirtied in xloginsert.c); each XLogInsert() in that critical section removes tracked buffers that were registered with the inserted record; when the outermost critical section ends, any still-pending buffer is reported (WARNING by default so that a whole test run can collect all violations; define XLOG_REG_CHECK_PANIC to get a PANIC with a backtrace at the site instead). The check is deferred to critical section end because some paths emit several records per critical section and register a buffer only with a later one (e.g. XLOG_HEAP2_NEW_CID precedes the record that registers the heap page). Critical sections that insert no WAL records are not checked, which exempts temp/unlogged relations, wal_level=minimal skip-WAL relfilenodes, and index builds that WAL-log their result afterwards via log_newpage_range. FSM pages are exempted automatically (the FSM is deliberately not WAL-logged), and XLOG_ASSIGN_LSN records are whitelisted: XLogGetFakeLSN() emits them from inside index-AM critical sections operating on skip-WAL relfilenodes purely to advance the insert LSN, and wal_level=minimal cannot produce incremental backups anyway. XLogRegCheckExemptBuffer() provides a documented opt-out for any further legitimate cases; the core regression, isolation, recovery, subscription, and contrib suites currently pass without needing any buffer-level exemption. Verified by temporarily removing the VM-buffer registration that ed62d26 added to heap_insert: the check then reports the unregistered VM page on the first insert that clears an all-visible bit. Closes #73 Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP --- src/backend/access/transam/xloginsert.c | 301 ++++++++++++++++++++++++ src/backend/storage/buffer/bufmgr.c | 9 + src/include/access/xloginsert.h | 12 + src/include/miscadmin.h | 31 +++ 4 files changed, 353 insertions(+) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index f2e10b82b7d3e..aee8cc661c4ad 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -134,6 +134,302 @@ static int max_rdatas; /* allocated size */ static bool begininsert_called = false; +#ifdef USE_ASSERT_CHECKING + +/* + * WAL registration cross-check + * ---------------------------- + * + * The WAL protocol (see access/transam/README) requires that every shared + * buffer that a WAL-logged operation modifies is registered in the WAL + * record describing that operation (XLogRegisterBuffer/XLogRegisterBlock). + * If a modified page is not registered, replay may still happen to work + * (the redo routine can find the page by other means), but the WAL + * summarizer only sees registered block references, so incremental backups + * silently omit the page — the class of corruption fixed for visibility-map + * bits in commit ed62d26 ("Fix VM clear WAL logging by registering VM + * blocks"). Unregistered modified pages also never get full-page images, + * so they are vulnerable to torn writes. + * + * Nothing enforces this invariant mechanically, so in assertion-enabled + * builds we cross-check it: every shared buffer passed to MarkBufferDirty() + * inside a critical section is recorded here (XLogRegCheckBufferDirtied, + * called from bufmgr.c). Whenever XLogInsert() completes a record inside a + * critical section, tracked buffers that were registered with that record + * are considered accounted for and removed. When the outermost critical + * section ends (see END_CRIT_SECTION in miscadmin.h), any tracked buffer + * that is still pending — dirtied, but never registered with any of the WAL + * records emitted in that critical section — is reported. + * + * The check is deliberately deferred to the end of the critical section + * rather than raised at XLogInsert() time, because some legitimate code + * paths emit more than one WAL record per critical section and register a + * dirtied buffer only with a later record (e.g. heap operations emit an + * XLOG_HEAP2_NEW_CID record, which carries no block references, before the + * record that registers the heap buffer). Likewise, critical sections that + * emit no WAL at all (temp/unlogged relations, wal_level=minimal relfilenodes + * created in the current transaction, index builds that WAL-log the result + * only afterwards via log_newpage_range) are not checked: the report is + * only produced if at least one WAL record was inserted in the critical + * section, since only then was there a record the buffer should have been + * registered with. + * + * Automatic exemptions (never tracked): + * + * - Local buffers: temporary relations are never WAL-logged. + * + * - Non-permanent shared buffers (!BufferIsPermanent): unlogged relations + * are not WAL-logged (their INIT_FORK buffers are permanent and remain + * subject to the check). + * + * - Free space map pages: the FSM is deliberately not WAL-logged at all + * (see src/backend/storage/freespace/README); it is self-repairing and + * incremental backups do not need FSM block references because the FSM + * is always copied whole. + * + * Code paths that legitimately dirty a buffer in a WAL-emitting critical + * section without registering it can opt out explicitly with + * XLogRegCheckExemptBuffer(), documenting why the WAL summarizer does not + * need to see the block. + * + * The report level is WARNING rather than an Assert/PANIC so that a full + * test run can collect all violations in one pass; define + * XLOG_REG_CHECK_PANIC to turn violations into PANICs (useful to get a + * backtrace at the exact site). + */ +#ifdef XLOG_REG_CHECK_PANIC +#define XLOG_REG_CHECK_ELEVEL PANIC +#else +#define XLOG_REG_CHECK_ELEVEL WARNING +#endif + +/* + * Fixed-size array: we cannot allocate memory inside a critical section. + * If it overflows, checking is skipped for the rest of the critical section. + */ +#define REGCHECK_MAX_DIRTIED 256 + +typedef struct RegCheckDirtiedBuffer +{ + RelFileLocator rlocator; + ForkNumber forkno; + BlockNumber block; + bool exempt; /* explicitly exempted from the check */ + int nrec_pending; /* records inserted while entry pending */ + RmgrId last_rmid; /* context of the last such record ... */ + uint8 last_info; /* ... for the violation report */ +} RegCheckDirtiedBuffer; + +static RegCheckDirtiedBuffer regcheck_dirtied[REGCHECK_MAX_DIRTIED]; +static int regcheck_ndirtied = 0; +static bool regcheck_overflowed = false; +static int regcheck_ninserts = 0; /* records inserted in current top-level + * critical section */ + +/* + * Reset the WAL registration cross-check state. + * + * Called when a top-level critical section starts (also cleans up any state + * left over by error-path CritSectionCount resets, e.g. in elog.c). + */ +void +XLogRegCheckReset(void) +{ + regcheck_ndirtied = 0; + regcheck_overflowed = false; + regcheck_ninserts = 0; +} + +/* + * Note that 'buffer' was dirtied. Called by MarkBufferDirty(). + * + * Only tracks shared buffers of WAL-logged forks dirtied inside a critical + * section; see the automatic exemptions above. + */ +void +XLogRegCheckBufferDirtied(Buffer buffer) +{ + RegCheckDirtiedBuffer *entry; + RelFileLocator rlocator; + ForkNumber forkno; + BlockNumber block; + + if (CritSectionCount == 0) + return; + + if (BufferIsLocal(buffer)) + return; /* temp relations are never WAL-logged */ + + if (!BufferIsPermanent(buffer)) + return; /* unlogged relations are not WAL-logged */ + + BufferGetTag(buffer, &rlocator, &forkno, &block); + + if (forkno == FSM_FORKNUM) + return; /* the FSM is deliberately not WAL-logged */ + + /* Already tracked? */ + for (int i = 0; i < regcheck_ndirtied; i++) + { + entry = ®check_dirtied[i]; + if (RelFileLocatorEquals(entry->rlocator, rlocator) && + entry->forkno == forkno && entry->block == block) + return; + } + + if (regcheck_ndirtied >= REGCHECK_MAX_DIRTIED) + { + regcheck_overflowed = true; + return; + } + + entry = ®check_dirtied[regcheck_ndirtied++]; + entry->rlocator = rlocator; + entry->forkno = forkno; + entry->block = block; + entry->exempt = false; + entry->nrec_pending = 0; + entry->last_rmid = 0; + entry->last_info = 0; +} + +/* + * Exempt 'buffer' from the WAL registration cross-check for the duration of + * the current critical section. + * + * Call this (after MarkBufferDirty and inside the critical section) in code + * paths that legitimately modify a buffer under a WAL-emitting critical + * section without registering it in the WAL record. Every caller must have + * a comment explaining why the WAL summarizer (and hence incremental + * backup) does not need to see the block. + */ +void +XLogRegCheckExemptBuffer(Buffer buffer) +{ + RelFileLocator rlocator; + ForkNumber forkno; + BlockNumber block; + + if (CritSectionCount == 0) + return; + + if (BufferIsLocal(buffer) || !BufferIsPermanent(buffer)) + return; /* not tracked anyway */ + + BufferGetTag(buffer, &rlocator, &forkno, &block); + + for (int i = 0; i < regcheck_ndirtied; i++) + { + RegCheckDirtiedBuffer *entry = ®check_dirtied[i]; + + if (RelFileLocatorEquals(entry->rlocator, rlocator) && + entry->forkno == forkno && entry->block == block) + { + entry->exempt = true; + return; + } + } +} + +/* + * Called by XLogInsert() just before a record is assembled and inserted, + * while the registered_buffers[] array is still populated. Tracked buffers + * registered with this record are accounted for; the rest stay pending. + */ +static void +XLogRegCheckRecordInserted(RmgrId rmid, uint8 info) +{ + if (CritSectionCount == 0) + return; + + /* + * Record-type whitelist: records that legitimately get inserted from + * within a critical section that modifies pages, without registering + * those pages. Treat them as if no record had been inserted at all. + * + * - XLOG_ASSIGN_LSN: emitted by XLogGetFakeLSN() from inside critical + * sections of index AMs (hash, nbtree, gist) operating on a permanent + * relation whose relfilenode is not WAL-logged (wal_level=minimal and + * the relfilenode was created in the current transaction, see + * RelationNeedsWAL). The record's only purpose is to advance the WAL + * insert position so the AM gets a distinct page LSN; it describes no + * page modification. The dirtied pages need no registration: the + * whole relation is durably synced at commit, and wal_level=minimal + * is incompatible with WAL summarization anyway (summarize_wal + * requires wal_level >= replica), so incremental backups cannot be + * affected. + */ + if (rmid == RM_XLOG_ID && info == XLOG_ASSIGN_LSN) + return; + + regcheck_ninserts++; + + for (int i = 0; i < regcheck_ndirtied;) + { + RegCheckDirtiedBuffer *entry = ®check_dirtied[i]; + bool found = false; + + for (int j = 0; j < max_registered_block_id; j++) + { + registered_buffer *regbuf = ®istered_buffers[j]; + + if (!regbuf->in_use) + continue; + if (RelFileLocatorEquals(regbuf->rlocator, entry->rlocator) && + regbuf->forkno == entry->forkno && + regbuf->block == entry->block) + { + found = true; + break; + } + } + + if (found) + { + /* accounted for; remove by replacing with the last entry */ + *entry = regcheck_dirtied[--regcheck_ndirtied]; + } + else + { + entry->nrec_pending++; + entry->last_rmid = rmid; + entry->last_info = info; + i++; + } + } +} + +/* + * Called when the outermost critical section ends (END_CRIT_SECTION in + * miscadmin.h). Report any buffer that was dirtied inside the critical + * section but not registered with any of the WAL records inserted in it. + */ +void +XLogRegCheckCritSectionEnd(void) +{ + if (regcheck_ninserts > 0 && !regcheck_overflowed) + { + for (int i = 0; i < regcheck_ndirtied; i++) + { + RegCheckDirtiedBuffer *entry = ®check_dirtied[i]; + + if (entry->exempt) + continue; + + elog(XLOG_REG_CHECK_ELEVEL, + "WAL registration check: buffer for relation %u/%u/%u fork %d block %u was dirtied in a critical section that inserted %d WAL record(s), but was not registered with any of them (%d record(s) inserted while pending, last one rmid %d info 0x%02X)", + entry->rlocator.spcOid, entry->rlocator.dbOid, + entry->rlocator.relNumber, (int) entry->forkno, entry->block, + regcheck_ninserts, entry->nrec_pending, + (int) entry->last_rmid, (unsigned int) entry->last_info); + } + } + + XLogRegCheckReset(); +} + +#endif /* USE_ASSERT_CHECKING */ + /* Memory context to hold the registered buffer and data references. */ static MemoryContext xloginsert_cxt; @@ -509,6 +805,11 @@ XLogInsert(RmgrId rmid, uint8 info) return EndPos; } +#ifdef USE_ASSERT_CHECKING + /* Account for tracked dirtied buffers registered with this record. */ + XLogRegCheckRecordInserted(rmid, info); +#endif + do { XLogRecPtr RedoRecPtr; diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index 3908529872a31..91006ef12c380 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -3216,6 +3216,15 @@ MarkBufferDirty(Buffer buffer) if (VacuumCostActive) VacuumCostBalance += VacuumCostPageDirty; } + +#ifdef USE_ASSERT_CHECKING + /* + * If we are inside a critical section, remember this buffer for the WAL + * registration cross-check: it must be registered with one of the WAL + * records inserted in this critical section. See xloginsert.c. + */ + XLogRegCheckBufferDirtied(buffer); +#endif } /* diff --git a/src/include/access/xloginsert.h b/src/include/access/xloginsert.h index 91dfbd5627f50..7f6ab5cbb2550 100644 --- a/src/include/access/xloginsert.h +++ b/src/include/access/xloginsert.h @@ -68,4 +68,16 @@ extern XLogRecPtr XLogGetFakeLSN(Relation rel); extern void InitXLogInsert(void); +#ifdef USE_ASSERT_CHECKING +/* + * WAL registration cross-check (see xloginsert.c). Verifies that every + * shared buffer dirtied inside a WAL-emitting critical section is registered + * with one of the WAL records inserted in that critical section. + * XLogRegCheckReset and XLogRegCheckCritSectionEnd are declared in + * miscadmin.h, next to the critical section macros that call them. + */ +extern void XLogRegCheckBufferDirtied(Buffer buffer); +extern void XLogRegCheckExemptBuffer(Buffer buffer); +#endif + #endif /* XLOGINSERT_H */ diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index 7170a4bff9896..6006b3bf28ff1 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -149,6 +149,35 @@ do { \ QueryCancelHoldoffCount--; \ } while(0) +#ifdef USE_ASSERT_CHECKING + +/* + * In assertion-enabled builds, the critical section entry/exit macros also + * drive the WAL registration cross-check (see xloginsert.c): buffers dirtied + * inside a critical section must be registered with one of the WAL records + * inserted in it. The state is reset when a top-level critical section + * starts and checked when it ends. + */ +extern void XLogRegCheckReset(void); +extern void XLogRegCheckCritSectionEnd(void); + +#define START_CRIT_SECTION() \ +do { \ + if (CritSectionCount == 0) \ + XLogRegCheckReset(); \ + CritSectionCount++; \ +} while(0) + +#define END_CRIT_SECTION() \ +do { \ + Assert(CritSectionCount > 0); \ + CritSectionCount--; \ + if (CritSectionCount == 0) \ + XLogRegCheckCritSectionEnd(); \ +} while(0) + +#else /* !USE_ASSERT_CHECKING */ + #define START_CRIT_SECTION() (CritSectionCount++) #define END_CRIT_SECTION() \ @@ -157,6 +186,8 @@ do { \ CritSectionCount--; \ } while(0) +#endif /* USE_ASSERT_CHECKING */ + /***************************************************************************** * globals.h -- * From 44c6fc78f7715351310638342d476d64d1c68e30 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 18 Jul 2026 15:26:27 +0000 Subject: [PATCH 2/2] Restructure WAL registration check exemptions as explicit classification Replace the ad-hoc exemption list of the WAL registration cross-check with an explicit classification: every dirtied buffer and every inserted record now passes through XLogRegCheckClassifyBuffer() / XLogRegCheckClassifyRecord(), which return a XLogRegCheckClass value. Each non-CHECKED class documents the invariant that makes skipping the check safe and the conditions under which that argument would break, so future exemptions require writing down a safety argument rather than appending to a list: - NOT_WAL_LOGGED_RELATION: local (temporary) and non-permanent (unlogged) buffers; no WAL is generated for their data changes and they are outside incremental backup's guarantees. - SELF_REPAIRING_FORK: FSM pages; deliberately not WAL-logged, tolerated stale/torn, copied whole by incremental backup. - HINT_ONLY: documented for completeness; MarkBufferDirtyHint() does not feed the tracker, and checksummed hint changes are WAL-logged with a registered block via XLOG_FPI_FOR_HINT. - LSN_ONLY_RECORD: records whose entire meaning is the LSN they occupy; the only member is XLOG_ASSIGN_LSN (XLogGetFakeLSN on skip-WAL relfilenodes). Superficially similar records (XLOG_NOOP, XLOG_SWITCH) are deliberately excluded so that the check complains if they ever appear in a page-modifying critical section. Remove the per-buffer opt-out API (XLogRegCheckExemptBuffer): no caller needed it, and an escape hatch that bypasses classification could fossilize bugs. Behavior is otherwise unchanged; regress, isolation, recovery/018_wal_optimize, subscription/026_stats and recovery/027_stream_regress run clean, and the ed62d26-revert kill test still trips the check on the unregistered VM page. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_01PtxA6B4d47ietk5dGsseuP --- src/backend/access/transam/xloginsert.c | 246 +++++++++++++++--------- src/include/access/xloginsert.h | 9 +- 2 files changed, 163 insertions(+), 92 deletions(-) diff --git a/src/backend/access/transam/xloginsert.c b/src/backend/access/transam/xloginsert.c index aee8cc661c4ad..5a242c95c6c3e 100644 --- a/src/backend/access/transam/xloginsert.c +++ b/src/backend/access/transam/xloginsert.c @@ -174,23 +174,14 @@ static bool begininsert_called = false; * section, since only then was there a record the buffer should have been * registered with. * - * Automatic exemptions (never tracked): - * - * - Local buffers: temporary relations are never WAL-logged. - * - * - Non-permanent shared buffers (!BufferIsPermanent): unlogged relations - * are not WAL-logged (their INIT_FORK buffers are permanent and remain - * subject to the check). - * - * - Free space map pages: the FSM is deliberately not WAL-logged at all - * (see src/backend/storage/freespace/README); it is self-repairing and - * incremental backups do not need FSM block references because the FSM - * is always copied whole. - * - * Code paths that legitimately dirty a buffer in a WAL-emitting critical - * section without registering it can opt out explicitly with - * XLogRegCheckExemptBuffer(), documenting why the WAL summarizer does not - * need to see the block. + * Every dirtied buffer and every inserted record is passed through an + * explicit classification (XLogRegCheckClassifyBuffer / + * XLogRegCheckClassifyRecord). There is no ad-hoc exemption list and no + * per-call opt-out: a page change either is subject to the invariant + * (XLOG_REG_CHECK_CHECKED) or belongs to a named class whose definition + * states the invariant that makes skipping it safe, and what would make it + * unsafe. Extending the exemptions therefore requires adding a new class + * here together with its written safety argument. * * The report level is WARNING rather than an Assert/PANIC so that a full * test run can collect all violations in one pass; define @@ -203,6 +194,110 @@ static bool begininsert_called = false; #define XLOG_REG_CHECK_ELEVEL WARNING #endif +/* + * Classification of a page modification (or of an inserted WAL record) with + * respect to the registration invariant. Every value other than + * XLOG_REG_CHECK_CHECKED carries the safety argument for skipping the + * check, and the condition under which that argument would break. + */ +typedef enum XLogRegCheckClass +{ + /* + * Subject to the invariant: a permanent shared buffer of a WAL-logged + * fork, dirtied inside a critical section. If any WAL record is + * inserted in that critical section, one of those records must register + * this block. + */ + XLOG_REG_CHECK_CHECKED, + + /* + * NOT_WAL_LOGGED_RELATION: local buffers (temporary relations) and + * non-permanent shared buffers (unlogged relations). + * + * Safe because: these relations generate no WAL for their data changes + * at all, are excluded from base/incremental backups' consistency + * guarantees (unlogged relations are reset at recovery from their + * WAL-logged INIT_FORK), and the WAL summarizer never needs to see + * them. + * + * Would become unsafe if: buffer persistence stopped tracking relation + * persistence (BufferIsPermanent is the authority; note INIT_FORK + * buffers of unlogged relations are permanent and thus remain CHECKED). + */ + XLOG_REG_CHECK_NOT_WAL_LOGGED_RELATION, + + /* + * SELF_REPAIRING_FORK: free space map pages (FSM_FORKNUM). + * + * Safe because: the FSM is deliberately not WAL-logged (see + * src/backend/storage/freespace/README); it is best-effort information + * that is tolerated stale/torn and repaired during normal operation + * (and by VACUUM). Because FSM changes have no WAL block references + * anywhere, incremental backup treats the FSM fork as unsummarized data + * and copies it whole, so the summarizer does not need FSM block + * references. + * + * Would become unsafe if: the FSM (or a part of it, e.g. the root page) + * started being WAL-logged and incrementally backed up, or if another + * fork were added to this class without the same self-repair and + * copied-whole properties. The visibility map does NOT qualify: VM + * bits carry correctness-critical information (index-only scans, + * relfrozenxid advancement) — that is exactly the ed62d26 bug class. + */ + XLOG_REG_CHECK_SELF_REPAIRING_FORK, + + /* + * HINT_ONLY: pages dirtied via MarkBufferDirtyHint(). + * + * This class is documentation: it can never be returned here because + * MarkBufferDirtyHint() intentionally does not feed the tracker (only + * MarkBufferDirty() does). Hint-bit-only changes are legitimately not + * registered with the current WAL record: they are not required for + * correctness of replay, and when they do matter for durability (page + * checksums enabled), MarkBufferDirtyHint() itself WAL-logs a full-page + * image (XLOG_FPI_FOR_HINT) with the block properly registered, which + * the WAL summarizer sees. + * + * Would become unsafe if: hint-style modifications stopped going + * through MarkBufferDirtyHint()/XLogSaveBufferForHint(), or a caller + * used MarkBufferDirtyHint() for a change replay actually depends on. + */ + XLOG_REG_CHECK_HINT_ONLY, + + /* + * LSN_ONLY_RECORD: WAL records whose entire meaning is the LSN they + * occupy — they describe no page modification by definition, so page + * modifications in the same critical section cannot be "theirs" and the + * record is ignored by the cross-check (as if no record was inserted). + * + * The defining properties of this class are: (i) the record is emitted + * solely to advance/mark the WAL insert position, (ii) it never carries + * block references, and (iii) the modifications surrounding it are + * covered by some other mechanism, stated per record type below. + * + * The only in-tree member is XLOG_ASSIGN_LSN, emitted by + * XLogGetFakeLSN() from inside index-AM critical sections (hash, + * nbtree, gist) that operate on a permanent relation whose relfilenode + * is not WAL-logged (wal_level=minimal and the relfilenode was created + * in the current transaction; XLogGetFakeLSN() asserts + * !RelationNeedsWAL). The surrounding page modifications are covered + * because the whole relation is durably synced at commit, and + * wal_level=minimal is incompatible with WAL summarization + * (summarize_wal requires wal_level >= replica), so incremental backups + * cannot be affected. Superficially similar records (XLOG_NOOP, + * XLOG_SWITCH, ...) are deliberately NOT in this class: they are never + * legitimately emitted from page-modifying critical sections, so if + * they ever appear there the check should complain. + * + * Would become unsafe if: a caller emitted an LSN-only record from a + * critical section that modifies pages of a WAL-logged relation and + * relied on it as the only record of the section — but that situation + * is precisely what this check would then be needed for, so any new + * emitter of XLOG_ASSIGN_LSN must re-justify membership here. + */ + XLOG_REG_CHECK_LSN_ONLY_RECORD, +} XLogRegCheckClass; + /* * Fixed-size array: we cannot allocate memory inside a critical section. * If it overflows, checking is skipped for the rest of the critical section. @@ -214,7 +309,6 @@ typedef struct RegCheckDirtiedBuffer RelFileLocator rlocator; ForkNumber forkno; BlockNumber block; - bool exempt; /* explicitly exempted from the check */ int nrec_pending; /* records inserted while entry pending */ RmgrId last_rmid; /* context of the last such record ... */ uint8 last_info; /* ... for the violation report */ @@ -226,6 +320,43 @@ static bool regcheck_overflowed = false; static int regcheck_ninserts = 0; /* records inserted in current top-level * critical section */ +/* + * Classify a dirtied buffer with respect to the registration invariant. + * Also returns the buffer tag for tracked buffers, since the caller needs + * it and computing it requires the same calls. + * + * HINT_ONLY is never returned here: hint-bit dirtying does not reach the + * tracker at all (see the class definition above). + */ +static XLogRegCheckClass +XLogRegCheckClassifyBuffer(Buffer buffer, RelFileLocator *rlocator, + ForkNumber *forkno, BlockNumber *block) +{ + if (BufferIsLocal(buffer) || !BufferIsPermanent(buffer)) + return XLOG_REG_CHECK_NOT_WAL_LOGGED_RELATION; + + BufferGetTag(buffer, rlocator, forkno, block); + + if (*forkno == FSM_FORKNUM) + return XLOG_REG_CHECK_SELF_REPAIRING_FORK; + + return XLOG_REG_CHECK_CHECKED; +} + +/* + * Classify an inserted WAL record: either it is a record that page + * modifications must be registered with (CHECKED), or it belongs to a class + * that by definition describes no page modification. + */ +static XLogRegCheckClass +XLogRegCheckClassifyRecord(RmgrId rmid, uint8 info) +{ + if (rmid == RM_XLOG_ID && info == XLOG_ASSIGN_LSN) + return XLOG_REG_CHECK_LSN_ONLY_RECORD; + + return XLOG_REG_CHECK_CHECKED; +} + /* * Reset the WAL registration cross-check state. * @@ -243,8 +374,8 @@ XLogRegCheckReset(void) /* * Note that 'buffer' was dirtied. Called by MarkBufferDirty(). * - * Only tracks shared buffers of WAL-logged forks dirtied inside a critical - * section; see the automatic exemptions above. + * Only buffers classified as CHECKED are tracked, and only while inside a + * critical section. */ void XLogRegCheckBufferDirtied(Buffer buffer) @@ -257,16 +388,9 @@ XLogRegCheckBufferDirtied(Buffer buffer) if (CritSectionCount == 0) return; - if (BufferIsLocal(buffer)) - return; /* temp relations are never WAL-logged */ - - if (!BufferIsPermanent(buffer)) - return; /* unlogged relations are not WAL-logged */ - - BufferGetTag(buffer, &rlocator, &forkno, &block); - - if (forkno == FSM_FORKNUM) - return; /* the FSM is deliberately not WAL-logged */ + if (XLogRegCheckClassifyBuffer(buffer, &rlocator, &forkno, &block) != + XLOG_REG_CHECK_CHECKED) + return; /* Already tracked? */ for (int i = 0; i < regcheck_ndirtied; i++) @@ -287,50 +411,11 @@ XLogRegCheckBufferDirtied(Buffer buffer) entry->rlocator = rlocator; entry->forkno = forkno; entry->block = block; - entry->exempt = false; entry->nrec_pending = 0; entry->last_rmid = 0; entry->last_info = 0; } -/* - * Exempt 'buffer' from the WAL registration cross-check for the duration of - * the current critical section. - * - * Call this (after MarkBufferDirty and inside the critical section) in code - * paths that legitimately modify a buffer under a WAL-emitting critical - * section without registering it in the WAL record. Every caller must have - * a comment explaining why the WAL summarizer (and hence incremental - * backup) does not need to see the block. - */ -void -XLogRegCheckExemptBuffer(Buffer buffer) -{ - RelFileLocator rlocator; - ForkNumber forkno; - BlockNumber block; - - if (CritSectionCount == 0) - return; - - if (BufferIsLocal(buffer) || !BufferIsPermanent(buffer)) - return; /* not tracked anyway */ - - BufferGetTag(buffer, &rlocator, &forkno, &block); - - for (int i = 0; i < regcheck_ndirtied; i++) - { - RegCheckDirtiedBuffer *entry = ®check_dirtied[i]; - - if (RelFileLocatorEquals(entry->rlocator, rlocator) && - entry->forkno == forkno && entry->block == block) - { - entry->exempt = true; - return; - } - } -} - /* * Called by XLogInsert() just before a record is assembled and inserted, * while the registered_buffers[] array is still populated. Tracked buffers @@ -343,23 +428,11 @@ XLogRegCheckRecordInserted(RmgrId rmid, uint8 info) return; /* - * Record-type whitelist: records that legitimately get inserted from - * within a critical section that modifies pages, without registering - * those pages. Treat them as if no record had been inserted at all. - * - * - XLOG_ASSIGN_LSN: emitted by XLogGetFakeLSN() from inside critical - * sections of index AMs (hash, nbtree, gist) operating on a permanent - * relation whose relfilenode is not WAL-logged (wal_level=minimal and - * the relfilenode was created in the current transaction, see - * RelationNeedsWAL). The record's only purpose is to advance the WAL - * insert position so the AM gets a distinct page LSN; it describes no - * page modification. The dirtied pages need no registration: the - * whole relation is durably synced at commit, and wal_level=minimal - * is incompatible with WAL summarization anyway (summarize_wal - * requires wal_level >= replica), so incremental backups cannot be - * affected. + * Records that by definition describe no page modification are ignored: + * they neither account for pending buffers nor count as a record the + * critical section's page changes should have been registered with. */ - if (rmid == RM_XLOG_ID && info == XLOG_ASSIGN_LSN) + if (XLogRegCheckClassifyRecord(rmid, info) != XLOG_REG_CHECK_CHECKED) return; regcheck_ninserts++; @@ -413,9 +486,6 @@ XLogRegCheckCritSectionEnd(void) { RegCheckDirtiedBuffer *entry = ®check_dirtied[i]; - if (entry->exempt) - continue; - elog(XLOG_REG_CHECK_ELEVEL, "WAL registration check: buffer for relation %u/%u/%u fork %d block %u was dirtied in a critical section that inserted %d WAL record(s), but was not registered with any of them (%d record(s) inserted while pending, last one rmid %d info 0x%02X)", entry->rlocator.spcOid, entry->rlocator.dbOid, diff --git a/src/include/access/xloginsert.h b/src/include/access/xloginsert.h index 7f6ab5cbb2550..72dcca590550c 100644 --- a/src/include/access/xloginsert.h +++ b/src/include/access/xloginsert.h @@ -72,12 +72,13 @@ extern void InitXLogInsert(void); /* * WAL registration cross-check (see xloginsert.c). Verifies that every * shared buffer dirtied inside a WAL-emitting critical section is registered - * with one of the WAL records inserted in that critical section. - * XLogRegCheckReset and XLogRegCheckCritSectionEnd are declared in - * miscadmin.h, next to the critical section macros that call them. + * with one of the WAL records inserted in that critical section. Buffers + * and records are exempted only through the explicit classification in + * xloginsert.c; there is no per-call opt-out. XLogRegCheckReset and + * XLogRegCheckCritSectionEnd are declared in miscadmin.h, next to the + * critical section macros that call them. */ extern void XLogRegCheckBufferDirtied(Buffer buffer); -extern void XLogRegCheckExemptBuffer(Buffer buffer); #endif #endif /* XLOGINSERT_H */