Re: 2pc leaks fds

On 2020-Apr-22, Andres Freund wrote:

> I'm in favor of doing so. Not necessarily primarily to avoid repeated
> API changes, but because I don't think the v13 changes went in the quite
> right direction.
> 
> ISTM that we should:
> - have the three callbacks you mention above
> - change WALSegmentOpen to also get the XLogReaderState
> - add private state to WALOpenSegment, so it can be used even when not
>   accessing data in files / when one needs more information to close the
>   file.
> - disambiguate between WALOpenSegment (struct describing an open
>   segment) and WALSegmentOpen (callback to open a segment) (note that
>   the read page callback uses a *CB naming, why not follow?)

Here's a first attempt at that.  The segment_open/close callbacks are
now given at XLogReaderAllocate time, and are passed the XLogReaderState
pointer.  I wrote a comment to explain that the page_read callback can
use WALRead() if it wishes to do so; but if it does, then segment_open
has to be provided.  segment_close is mandatory (since we call it at
XLogReaderFree).

Of the half a dozen cases that exist, three are slightly weird:

* Physical walsender does not use a xlogreader at all.  I think we could
  beat that code up so that it does.  But for the moment I just cons up
  a fake xlogreader, which only has the segment_open pointer set up, so
  that it can call WALRead.

* main xlog.c uses an xlogreader with XLogPageRead(), which does not use
  WALRead.  Therefore it does not pass open_segment.  It does not use
  xlogreader->seg.ws_file either.  Eventually we may want to beat this
  one up also.

* pg_rewind has its own page read callback, SimpleXLogPageRead, which
  does all the required opening and closing.  I don't think it'd be an
  improvement to force this to use segment_open.  Oddly enough, it calls
  itself "simple" but is unique in having the ability to read files from
  the wal archive.

All tests are passing for me.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services