Re: walreceiver is uninterruptible on win32

On Tue, Apr 13, 2010 at 9:21 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Tue, Apr 13, 2010 at 1:56 AM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>>> If adding new shared library is too big change at this point, I think
>>> that we should postpone the fix only for dblink to 9.1 or later. Since
>>> no one has complained about this long-term problem of dblink, I'm not
>>> sure it really should be fixed right now. Thought?
>>
>> +1. Let's fix walreceiver for now, and we can revisit dblink later.
>> Since we haven't had any complaints so far...
>
> OK. I'll focus on walreceiver now.

The attached patch changes walreceiver so that it uses new function
libpqrcv_PQexec() instead of PQexec() for sending handshake message.
libpqrcv_PQexec() sends a query and wait for the results by using
the asynchronous libpq functions and the backend version of select().
It's interruptible by signals. You would be able to shut the standby
server down in the middle of handshake for replication connection.

http://archives.postgresql.org/pgsql-hackers/2010-03/msg00625.php
> Just replacing PQexec() with PQsendQuery() is pretty straightforward, we
> could put that replacement in a file in port/win32. Replacing
> PQconnectdb() is more complicated because you need to handle connection
> timeout. I suggest that we only add the replacement for PQexec(), and
> live with the situation for PQconnectdb(), that covers 99% of the
> scenarios anyway.

According to the suggestion, I replaced only the PQexec() and didn't
add the replacement of PQconnect() for now.

http://archives.postgresql.org/pgsql-hackers/2010-04/msg00077.php
> As for the code itself, don't we need a CHECK_FOR_INTERRUPTS in there
> for it to be actually useful?

Since the backend version of select() receives any incoming signals
on Win32, CHECK_FOR_INTERRUPTS seems not to be needed in the loop,
at least in walreceiver. No? The patch doesn't put it in there, and
I was able to interrupt walreceiver executing libpqrcv_PQexec() when
I tested the patch on Win32.

http://archives.postgresql.org/pgsql-hackers/2010-04/msg00084.php
> 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 patch uses libpq_select() to check for the socket. It supports
poll() and select().

> 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.

The patch still takes some shortcuts since we decided to postpone
the fix for dblink to 9.1 or later.

Regards,

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center