On Mon, Sep 21, 2026 at 03:14:39PM -0500, Sami Imseih wrote:
> At first, I thought palloc_object() would do and be simpler, but keeping
> MCXT_ALLOC_NO_OOM gives the better message, since callers report
> errdetail("Failed while allocating a WAL reading processor."), and
> keeping this consistent with the other allocations is probably best.
> So I agree with what you have.
>
> I do not see anything else that stands out in the patch.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. However, could we consider the option of transient fds for anything else, particularly the users of wal_segment_open() in xlogutils.c? 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. Note that xlogreader.h declares a new variable that makes no sense in FRONTEND code. This needs an #ifdef. -- Michael
signature.asc
Description: PGP signature
