Do not emit FPW for unlogged relations in BRIN empty-page

Started by Kirill Reshke26 days ago2 messages
#1Kirill Reshke
reshkekirill@gmail.com
1 attachment(s)

Hi hackers.

I have been reviewing other patches, and spotted code in
`brin_initialize_empty_new_buffer`. This function emits FPW for
newly-initialized BRIN bufferafter `MarkBufferDirty`. It seems to me
that this is unnecessary for UNLOGGED relations.

I have re-checked instances of `MarkBufferDirty` and it seems to me
that we always do WAL-logging stuff under RelationNeedsWAL marco,
except for ambuildempty and few other cases.

This code is hard to hit, so no reproducer here. In my understanding,
this function executes under some concurrent patterns for index write
activity. But I did not get when exactly.
WDYT?

--
Best regards,
Kirill Reshke

Attachments:

v1-0001-Do-not-emit-FPW-for-unlogged-relations-in-BRIN-em.patchapplication/octet-stream; name=v1-0001-Do-not-emit-FPW-for-unlogged-relations-in-BRIN-em.patchDownload
From 01a58102fc82c5a445183603f324eb24e639bbd1 Mon Sep 17 00:00:00 2001
From: reshke <reshke@double.cloud>
Date: Wed, 17 Dec 2025 15:07:52 +0000
Subject: [PATCH v1] Do not emit FPW for unlogged relations in BRIN empty-page
 ops.

---
 src/backend/access/brin/brin_pageops.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index 91a5ba163f2..c80b87da3d2 100644
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -891,7 +891,11 @@ brin_initialize_empty_new_buffer(Relation idxrel, Buffer buffer)
 	page = BufferGetPage(buffer);
 	brin_page_init(page, BRIN_PAGETYPE_REGULAR);
 	MarkBufferDirty(buffer);
-	log_newpage_buffer(buffer, true);
+
+	/* XLOG stuff */
+	if (RelationNeedsWAL(idxrel))
+		log_newpage_buffer(buffer, true);
+
 	END_CRIT_SECTION();
 
 	/*
-- 
2.43.0

#2Heikki Linnakangas
hlinnaka@iki.fi
In reply to: Kirill Reshke (#1)
1 attachment(s)
Re: Do not emit FPW for unlogged relations in BRIN empty-page

On 17/12/2025 17:13, Kirill Reshke wrote:

Hi hackers.

I have been reviewing other patches, and spotted code in
`brin_initialize_empty_new_buffer`. This function emits FPW for
newly-initialized BRIN bufferafter `MarkBufferDirty`. It seems to me
that this is unnecessary for UNLOGGED relations.

I have re-checked instances of `MarkBufferDirty` and it seems to me
that we always do WAL-logging stuff under RelationNeedsWAL marco,
except for ambuildempty and few other cases.

This code is hard to hit, so no reproducer here. In my understanding,
this function executes under some concurrent patterns for index write
activity. But I did not get when exactly.
WDYT?

Yep, that's a bug. I was able to reproduce it with the attached script.
Run the script,then do "pg_ctl -D data restart -m immediate". Crash
recovery fails like this:

2025-12-18 14:55:00.266 EET [1494915] LOG: database system was
interrupted; last known up at 2025-12-18 14:54:45 EET
2025-12-18 14:55:00.405 EET [1494915] LOG: database system was not
properly shut down; automatic recovery in progress
2025-12-18 14:55:00.408 EET [1494915] LOG: redo starts at 0/1AAA080
2025-12-18 14:55:00.413 EET [1494915] LOG: invalid record length at
0/1EC5268: wanted 24, got 0
2025-12-18 14:55:00.413 EET [1494915] LOG: redo done at 0/1EC5240
system usage: CPU: user: 0.00 s, system: 0.00 s, elapsed: 0.00 s
2025-12-18 14:55:00.414 EET [1494915] FATAL: could not create file
"base/12974/16406": File exists
2025-12-18 14:55:00.414 EET [1494914] LOG: startup process (PID
1494915) exited with exit code 1
2025-12-18 14:55:00.414 EET [1494914] LOG: aborting startup due to
startup process failure

I'll commit and backpatch your fix. Thanks!

- Heikki

Attachments:

unlogged-brin-repro.shapplication/x-shellscript; name=unlogged-brin-repro.shDownload