Hi, On Fri, Sep 25, 2026 at 1:54 AM Michael Paquier <[email protected]> wrote: > > While going quickly through the patch and the thread, my first > reaction was: why not generalizing more the use of transaction-safe > fds where we could for the WAL segment open, as well? > > That does not stand in the logical decoding path, as I am pretty sure > that we may lost an fd we could still need. Looking at the segment > open callbacks, the second case where it does not fit well is a WAL > sender (WalSndSegmentOpen) with the segments that would be opened > outside the transaction realm.
Right. > However, could we consider the option of transient fds for anything > else, particularly the users of wal_segment_open() in xlogutils.c? I would prefer to avoid having two mechanisms to fix the same issue. > With these two cases in mind, I can get behind the idea of a reset > callback. However, I am not convinced that forcing the presence of a > reset callback when the xlogreader state is *allocated* is a correct > design, because it forces the decision to close a fd for everybody who > uses a XLogReaderAllocate, unconditionally, all the time, and it may > not even be required to open an fd at all in some cases (all WAL in > some buffers, heh). The reason why we need to close these fds in the > first place is due to the calls of segment_open, because we open a > segment. I would imagine here that the sane move is to register a > callback *iff* we open a segment. So, add a boolean flag in the state > tracking if the reset callback is registered, and use > GetMemoryChunkContext(state) to save the callback in the memory > context of the xlogreader state, not the CurrentMemoryContext where > the segment is opened. Agreed. A reader whose page_read callback reads everything from WAL buffers, for example with WALReadFromBuffers(), may not call segment_open at all, so it has no file to close, and registering a callback for it at allocation time is wasted. Registering it lazily on the first segment_open call avoids that. I will do it that way in the next version, with the callback on the reader's own context. > Note that xlogreader.h declares a new variable that makes no sense in > FRONTEND code. This needs an #ifdef. Ah, missed that. I will fix it. I will address the comments and post new patches soon. -- Bharath Rupireddy Amazon Web Services: https://aws.amazon.com
