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
