On Mon, Mar 28, 2016 at 10:08 PM, Thomas Munro
<thomas.munro@enterprisedb.com> wrote:
> On Tue, Mar 29, 2016 at 1:56 AM, Thomas Munro
> <thomas.munro@enterprisedb.com> wrote:
>> On Mon, Mar 28, 2016 at 8:54 PM, Michael Paquier
>> <michael.paquier@gmail.com> wrote:
>>> I have been also thinking a lot about this patch, and the fact that
>>> the WAL receiver latch is being used within the internals of
>>> libpqwalreceiver has been bugging me a lot, because this makes the
>>> wait phase happening within the libpqwalreceiver depend on something
>>> that only the WAL receiver had a only control on up to now (among the
>>> things thought: having a second latch for libpqwalreceiver, having an
>>> event interface for libpqwalreceiver, switch libpq_receive into being
>>> asynchronous...).
>>
>> Yeah, it bugs me too. Do you prefer this?
>>
>> int walrcv_receive(char **buffer, int *wait_fd);
>>
>> Return value -1 means end-of-copy as before, return value 0 means "no
>> data available now, please call me again when *wait_fd is ready to
>> read". Then walreceiver.c can look after the WaitLatchOrSocket call
>> and deal with socket readiness, postmaster death, timeout and latch,
>> and libpqwalreceiver.c doesn't know anything about all that stuff
>> anymore, but it is now part of the interface that it must expose a
>> file descriptor for readiness testing when it doesn't have data
>> available.
>>
>> Please find attached a new patch series which does it that way.
>
> Oops, there is a bug in the primary disconnection case when len == 1
> and it breaks out of the loop and wait_fd is invalid. I'll follow up
> on that tomorrow, but I'm interested to hear your thoughts (and anyone
> else's!) on that interface change and general approach.
I definitely prefer that, that's neater! libpq_select could be
simplified because a timeout does not matter much.
--
Michael