Re: Replication server timeout patch

On 24.03.2011 15:24, Fujii Masao wrote:
> On Wed, Mar 23, 2011 at 7:33 PM, Heikki Linnakangas
> <heikki.linnakangas@enterprisedb.com>  wrote:
>> I don't much like the API for this. Walsender shouldn't need to know about
>> the details of the FE/BE protocol, pq_putbytes_if_available() seems too low
>> level to be useful.
>>
>> I think a better API would be to have a non-blocking version of
>> pq_putmessage(). We can make the output buffer in pqcomm.c resizeable, so
>> that when the message doesn't fit in the output buffer in pq_putmessage(),
>> the buffer is enlarged instead of trying to flush it.
>>
>> Attached is a patch using that approach. This is a much smaller patch, and
>> easier to understand.
>
> Agreed. Thanks for improving the patch.
>
> pq_flush_if_writable() calls internal_flush() without using PG_TRY block.
> This seems unsafe because for example pgwin32_waitforsinglesocket()
> called by secure_write() can throw ERROR.

Perhaps it's time to give up on the assumption that the socket is in
blocking mode except within those two functions. Attached patch adds the
pq_set_nonblocking() function from your patch, and adds calls to it
before all secure_read/write operations to put the socket in the right
mode. There's only a few of those operations.

Should we use COMMERROR instead of ERROR if we fail to put the socket in
the right mode?

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com