On 03/11/2026 03:06, Narek Galstyan wrote:
Hi all,
I noticed that bloom indexes are not properly WAL-looged on unlogged
tables and attached is my attempt at fixing it
Since commit ccadf73163c <https://github.com/postgres/postgres/commit/
ccadf73163ca88bdaa74b8223d4dde05d17f550b> (discussion: https://
www.postgresql.org/message-id/6e5bbc08-cdfc-
[email protected] <https://www.postgresql.org/message-
id/[email protected]>), `contrib/bloom` index
AM's blbuildempty() wrote the metapage of the init fork through generic
WAL, which emits no WAL record for an unlogged relation.
The init fork must be WAL-logged as it is used to recreate main fork of
an unlogged relation after a crash.
Without this fix, after a crash, an unlogged and bloom-indexed relation
gets a zero-ed out index meta page. As a result, insertions and index
scans fail with "Relation is not a bloom index" or "could not read
blocks 0..0", until an index rebuild.
Yep, my fault with ccadf73163c.
Your proposed fix fixes it, but only for contrib/bloom. Any other
indexam using generic WAL would also need to know that they cannot use
generic WAL for the init fork. How about the attached, to teach
GenericXLogFinish() to WAL log the init fork, instead?
- Heikki
From 8f9be07a43d2d68e1217888678be5d383380f0e8 Mon Sep 17 00:00:00 2001
From: Heikki Linnakangas <[email protected]>
Date: Wed, 4 Nov 2026 20:38:12 +0200
Subject: [PATCH 1/1] Fix WAL-logging init fork (for unlogged rels) with
generic WAL
The generic WAL code skips emitting WAL records for unlogged
relations, but it only considered RelationNeedsWAL() and didn't take
into account that the init fork of an unlogged relation does need to
be WAL-logged. That meant that WAL-logging the init fork from
ambuildempty() using generic WAL did not work.
Since commit ccadf73163c, contrib/bloom's blbuildempty() uses generic
WAL for the metapage, so it's been broken ever since. Backpatch to
v16 like that commit. Earlier versions have the same problem, but
since there's no (core) code using generic WAL from ambuildempty()
before that, it doesn't seem worth the risk backpatching further.
Co-authored-by: Narek Galstyan <[email protected]>
Discussion: https://www.postgresql.org/message-id/cahsn6_y9zcaj5vc4-noqrnu3io_tovdmjkbgafw-wonuwf6...@mail.gmail.com
Backpatch-through: 16
---
contrib/bloom/t/001_wal.pl | 22 ++++++
src/backend/access/transam/generic_xlog.c | 90 ++++++++++++++---------
2 files changed, 78 insertions(+), 34 deletions(-)
diff --git a/contrib/bloom/t/001_wal.pl b/contrib/bloom/t/001_wal.pl
index 683b1876055..27864495243 100644
--- a/contrib/bloom/t/001_wal.pl
+++ b/contrib/bloom/t/001_wal.pl
@@ -81,4 +81,26 @@ for my $i (1 .. 10)
test_index_replay("insert $i");
}
+# Also test an unlogged index. (We had bugs in WAL-logging the init
+# fork in the past.)
+$node_primary->safe_psql("postgres",
+ "CREATE UNLOGGED TABLE tst_unlogged (i int4, t text);");
+$node_primary->safe_psql("postgres",
+ "CREATE INDEX bloomidx_unlogged ON tst_unlogged USING bloom (i, t);");
+$node_primary->wait_for_catchup($node_standby);
+
+my $unlogged_queries = qq(INSERT INTO tst_unlogged VALUES (1, 'a');
+SET enable_seqscan=off;
+SELECT count(*) FROM tst_unlogged WHERE i = 1;
+);
+
+$node_primary->stop('immediate');
+$node_primary->start;
+is($node_primary->safe_psql("postgres", $unlogged_queries),
+ '1', 'unlogged index usable after crash recovery');
+
+$node_standby->promote;
+is($node_standby->safe_psql("postgres", $unlogged_queries),
+ '1', 'unlogged index usable after standby promotion');
+
done_testing();
diff --git a/src/backend/access/transam/generic_xlog.c b/src/backend/access/transam/generic_xlog.c
index 7f82186d0d6..ac7caaeb138 100644
--- a/src/backend/access/transam/generic_xlog.c
+++ b/src/backend/access/transam/generic_xlog.c
@@ -67,6 +67,7 @@ struct GenericXLogState
PGIOAlignedBlock images[MAX_GENERIC_XLOG_PAGES];
/* Info about each page, see above */
GenericXLogPageData pages[MAX_GENERIC_XLOG_PAGES];
+ /* RelationNeedsWAL() result */
bool isLogged;
};
@@ -331,36 +332,59 @@ GenericXLogRegisterBuffer(GenericXLogState *state, Buffer buffer, int flags)
/*
* Apply changes represented by GenericXLogState to the actual buffers,
- * and emit a generic xlog record.
+ * and emit a generic xlog record (if needed).
*/
XLogRecPtr
GenericXLogFinish(GenericXLogState *state)
{
XLogRecPtr lsn;
int i;
+ bool logged[MAX_GENERIC_XLOG_PAGES];
+ bool loggedAny = false;
- if (state->isLogged)
+ START_CRIT_SECTION();
+
+ /*
+ * Compute deltas if necessary, write changes to buffers, mark buffers
+ * dirty, and register changes.
+ */
+ for (i = 0; i < MAX_GENERIC_XLOG_PAGES; i++)
{
- /* Logged relation: make xlog record in critical section. */
- XLogBeginInsert();
+ GenericXLogPageData *pageData = &state->pages[i];
+ bool xlogNeeded;
- START_CRIT_SECTION();
+ if (BufferIsInvalid(pageData->buffer))
+ {
+ logged[i] = false;
+ continue;
+ }
- /*
- * Compute deltas if necessary, write changes to buffers, mark buffers
- * dirty, and register changes.
- */
- for (i = 0; i < MAX_GENERIC_XLOG_PAGES; i++)
+ /* Does this page need to be WAL-logged? */
+ if (state->isLogged)
+ xlogNeeded = true;
+ else
{
- GenericXLogPageData *pageData = &state->pages[i];
- Page page;
- PageHeader pageHeader;
+ RelFileLocator rlocator;
+ ForkNumber forknum;
+ BlockNumber blkno;
- if (BufferIsInvalid(pageData->buffer))
- continue;
+ BufferGetTag(pageData->buffer, &rlocator, &forknum, &blkno);
- page = BufferGetPage(pageData->buffer);
- pageHeader = (PageHeader) pageData->image;
+ xlogNeeded = (forknum == INIT_FORKNUM);
+ }
+ logged[i] = xlogNeeded;
+
+ /* call XLogBeginInsert() before starting to build the WAL record */
+ if (xlogNeeded && !loggedAny)
+ {
+ XLogBeginInsert();
+ loggedAny = true;
+ }
+
+ if (xlogNeeded)
+ {
+ Page page = BufferGetPage(pageData->buffer);
+ PageHeader pageHeader;
/*
* Compute delta while we still have both the unmodified page and
@@ -374,6 +398,7 @@ GenericXLogFinish(GenericXLogState *state)
* pd_lower and pd_upper in order to avoid divergence between
* actual page state and what replay would produce.
*/
+ pageHeader = (PageHeader) pageData->image;
memcpy(page, pageData->image, pageHeader->pd_lower);
memset(page + pageHeader->pd_lower, 0,
pageHeader->pd_upper - pageHeader->pd_lower);
@@ -394,7 +419,18 @@ GenericXLogFinish(GenericXLogState *state)
XLogRegisterBufData(i, pageData->delta, pageData->deltaLen);
}
}
+ else
+ {
+ memcpy(BufferGetPage(pageData->buffer),
+ pageData->image,
+ BLCKSZ);
+ /* We don't worry about zeroing the "hole" in this case */
+ MarkBufferDirty(pageData->buffer);
+ }
+ }
+ if (loggedAny)
+ {
/* Insert xlog record */
lsn = XLogInsert(RM_GENERIC_ID, 0);
@@ -403,32 +439,18 @@ GenericXLogFinish(GenericXLogState *state)
{
GenericXLogPageData *pageData = &state->pages[i];
- if (BufferIsInvalid(pageData->buffer))
+ if (!logged[i])
continue;
+
PageSetLSN(BufferGetPage(pageData->buffer), lsn);
}
- END_CRIT_SECTION();
}
else
{
- /* Unlogged relation: skip xlog-related stuff */
- START_CRIT_SECTION();
- for (i = 0; i < MAX_GENERIC_XLOG_PAGES; i++)
- {
- GenericXLogPageData *pageData = &state->pages[i];
-
- if (BufferIsInvalid(pageData->buffer))
- continue;
- memcpy(BufferGetPage(pageData->buffer),
- pageData->image,
- BLCKSZ);
- /* We don't worry about zeroing the "hole" in this case */
- MarkBufferDirty(pageData->buffer);
- }
- END_CRIT_SECTION();
/* We don't have a LSN to return, in this case */
lsn = InvalidXLogRecPtr;
}
+ END_CRIT_SECTION();
pfree(state);
--
2.47.3
