Re: Streaming Replication on win32

On Sun, Feb 14, 2010 at 11:52 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
> Sorry about the delay in responding to this.

Thanks for the response.

> Remember that the win32 code *always* puts the socket in non-blocking
> mode. So we can't just "teach the layer about it". We need some way to
> pass the information down that this is actually something we want to
> be non-blocking, and it can't be the normal flag on the socket. I
> don't really have an idea of where else we'd put it though :( It's in
> the port structure, but not beyond it.

Right.

BTW, pq_getbyte_if_available() always changes the socket to non-blocking
and blocking mode before and after calling secure_read(), respectively.
This code seems wrong on win32. Because, as you said, the socket is always
in non-blocking mode on win32. We should change pq_getbyte_if_available()
so as not to change the socket mode only in win32?

> What we could do, is have an ugly global flag specifically for the
> use-case we have here. Assuming we do create a plataform specific
> pq_getbyte_if_available(), the code-path that would have trouble now
> would be when we call pq_getbyte_if_available(), and it in turns asks
> the socket if there is data, there is, but we end up calling back into
> the SSL code to fetch the data, and it gets an incomplete packet.
> Correct? So the path is basically:
>
> pq_getbyte_if_available() -> secure_read() -> SSL_read() ->
> my_sock_read() -> pgwin32_recv()
>
> Given that we know we are working on a single socket here, we could
> use a global flag to tell pgwin32_recv() to become nonblocking. We
> could set this flag directly in the win32-specific version of
> pq_getbyte_if_available(), and make sure it's cleared as soon as we
> exit.
>
> It will obviously fail if we do anything on a *different* socket
> during this time, so it has to be set for a very short time. But that
> seems doable. And we don't call any socket stuff from signal handlers
> so that shouldn't cause issues.

Agreed. Here is the patch which does that (including the above-mentioned
change). I haven't tested it yet because I failed in creating the build
environment for the MSVC :( I'll try to create that again, and test it.
Though I'm not sure how long it takes.

Regards,

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