Re: walreceiver is uninterruptible on win32

On Fri, Apr 2, 2010 at 17:26, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> On Thu, Mar 25, 2010 at 15:33, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> Sorry for the delay. The attached patch replaces PQexec() used by dblink
>>> and libpqwalreceiver with pgwin32_PQexec() which is the win32 version of
>>> PQexec().
>>>
>>> pgwin32_PQexec() is provided as the library 'libpqbe.dll', which is created
>>> only on win32. dblink.dll and libpqwalreceiver.dll refer to libpqbe.dll.
>>> Also libpqbe.dll refers to libpq.dll.
>
>> [ assorted objections ]
>
> I disapprove of the whole approach, actually.  The right way to fix this
> is to not touch or replace libpq at all, but to change walreceiver to
> use libpq's async-query facilities directly.  Instead of PQexec, use
> PQsendQuery and then a loop involving PQisBusy, PQgetResult, etc.
> You've more or less done that loop, but you've put it in the wrong
> place.

Any particular reason not to wrap that in a function? Not called
pgwin32_PQexec() then, but something more generic? And not doing any
#defines to change PQexec, but call that wrapper directly?

> The larger point is that I don't believe this issue exists only on
> Windows.  I think we're going to want something like this on all
> platforms, and that implies supporting poll() not just select() for the
> waiting part.

The most important part of the issue doesn't (because PQexec will be
interrupted by a signal), but there may certainly be others.

> The patch also seems confused about whether it's intending to be a
> general-purpose solution or not.  You can maybe take some shortcuts
> if it's only going to be for walreceiver, but if you're going to put
> it in dblink it is *not* acceptable to take shortcuts like not
> processing errors completely.

Yeah, good point.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/