Re: Replication server timeout patch

On Mon, Feb 28, 2011 at 8:08 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Sun, Feb 27, 2011 at 11:52 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>>> There are two things that I think are pretty clear.  If the receiver
>>> has wal_receiver_status_interval=0, then we should ignore
>>> replication_timeout for that connection.
>>
>> The patch still doesn't check that wal_receiver_status_interval
>> is set up properly. I'll implement that later.
>
> Done. I attached the updated patch.

Why does internal_flush_if_writable compute bufptr differently from
internal_flush? And shouldn't it be static?

It seems to me that this ought to be refactored so that you don't
duplicate so much code. Maybe static int internal_flush(bool
nonblocking).

I don't think that the while (bufptr < bufend) loop needs to contain
the code to set and clear the nonblocking state. You could do the
whole loop with nonblocking mode turned on and then reenable it just
once at the end. Besides possibly being clearer, that would be more
efficient and leave less room for unexpected failures.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company