Hi, On Thu, Oct 08, 2026 at 12:13:39PM +0900, Michael Paquier wrote: > On Wed, Oct 07, 2026 at 11:34:40AM -0700, Bharath Rupireddy wrote: > > I addressed Bertrand's review comments. I renamed the reset callback > > to keep it generic (closing the open WAL segment file is one cleanup > > that could be done here but it could be extended for any other > > resources held by XLogReader). I adjusted the comments. I like the > > comment around segment_close on not throwing errors, although the core > > callbacks inherently follow that (they ignore errors from close()), > > it's good to have it. I ran pgindent and tests. I wrote the commit > > message. > > Sounds fine to me code-wise. Applied on HEAD after more review and a > few tweaks.
Thanks! As mentioned upthread [1], I think XLogReaderFree() should also check whether seg.ws_file >= 0. Please find attached a small patch doing that. [1]: https://postgr.es/m/asNr203eue0R4zpZ@bdtpg Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
>From 1824f9e56284ba8ca3ccd16e476250ec08e4846f Mon Sep 17 00:00:00 2001 From: Bertrand Drouvot <[email protected]> Date: Thu, 8 Oct 2026 06:33:42 +0000 Subject: [PATCH v1] xlogreader: Treat any negative segment descriptor as closed The segment_close() callback is required to set ws_file to a negative value after closing a WAL segment. XLogReaderFree() only treated -1 as closed, so it could call segment_close() again if another negative sentinel was used. Test for a nonnegative descriptor instead, matching the documented callback convention. Author: Bertrand Drouvot <[email protected]> Reviewed-by: Discussion: https://postgr.es/m/asNr203eue0R4zpZ@bdtpg --- src/backend/access/transam/xlogreader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 100.0% src/backend/access/transam/ diff --git a/src/backend/access/transam/xlogreader.c b/src/backend/access/transam/xlogreader.c index cc61f830982..1db5ca48c3f 100644 --- a/src/backend/access/transam/xlogreader.c +++ b/src/backend/access/transam/xlogreader.c @@ -175,7 +175,7 @@ XLogReaderFree(XLogReaderState *state) &state->reset_cb); #endif - if (state->seg.ws_file != -1) + if (state->seg.ws_file >= 0) state->routine.segment_close(state); if (state->decode_buffer && state->free_decode_buffer) -- 2.34.1
