Re: Streaming Replication on win32

2010/2/15 Fujii Masao <masao(dot)fujii(at)gmail(dot)com>:
> On Sun, Feb 14, 2010 at 11:52 PM, Magnus Hagander <magnus(at)hagander(dot)net> wrote:
>> 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?

Yes.

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

I changed your patch to this, because I find it a lot simpler. The
change is in the checking in pgwin32_recv - there is no need to ever
call waitforsinglesocket, we can just exit out early.

Do you see any issue with that?

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