Re: 2pc leaks fds

On 2020-May-08, Kyotaro Horiguchi wrote:

> I agree to the direction of this patch. Thanks for the explanation.
> The patch looks good to me except the two points below.

Thanks!  I pushed the patch.  I fixed the walsender commentary as you
suggested, but I'm still of the opinion that we might want to use the
XLogReader abstraction in physical walsender than work without it; if
nothing else, that would simplify WALRead's API.

I didn't change this one though:

> wal_segment_close(XlogReaderState *state) is setting
> state->seg.ws_file to -1.  On the other hand wal_segment_close(state,..)
> doesn't update ws_file and the caller sets the returned value to
> (eventually) the same field.
> 
> +			seg->ws_file = state->routine.segment_open(state, nextSegNo,
> +													   segcxt, &tli);
> 
> If you are willing to do so, I think it is better to make the callback
> functions are responsible to update the seg.ws_file and the callers
> don't care.

I agree that this would be a good idea, but it's more than just a
handful of lines of changes so I think we should consider it separately.
Attached as 0002.  I also realized while doing this that we can further
simplify WALRead()'s API if we're willing to bend walsender a little bit
more into the fake xlogreader thing; that's 0001.

I marked the open item closed nonetheless.

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