Re: [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown

On Fri, Nov 9, 2012 at 1:40 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> On Thu, Nov 8, 2012 at 2:22 AM, Heikki Linnakangas
> <hlinnakangas@vmware.com> wrote:
>> On 16.10.2012 15:31, Heikki Linnakangas wrote:
>>>
>>> On 15.10.2012 19:31, Fujii Masao wrote:
>>>>
>>>> On Mon, Oct 15, 2012 at 11:27 PM, Heikki Linnakangas
>>>> <hlinnakangas@vmware.com> wrote:
>>>>>
>>>>> On 15.10.2012 13:13, Heikki Linnakangas wrote:
>>>>>>
>>>>>>
>>>>>> Oh, I didn't remember that we've documented the specific structs
>>>>>> that we
>>>>>> pass around. It's quite bogus anyway to explain the messages the way we
>>>>>> do currently, as they are actually dependent on the underlying
>>>>>> architecture's endianess and padding. I think we should refactor the
>>>>>> protocol to not transmit raw structs, but use pq_sentint and friends to
>>>>>> construct the messages. This was discussed earlier (see
>>>>>>
>>>>>>
>>>>>> http://archives.postgresql.org/message-id/4FE2279C.2070506@enterprisedb.com),
>>>>>>
>>>>>> I think there's consensus that 9.3 would be a good time to do that
>>>>>> as we changed the XLogRecPtr format anyway.
>>>>>
>>>>>
>>>>> This is what I came up with. The replication protocol is now
>>>>> architecture-independent. The WAL format itself is still
>>>>> architecture-independent, of course, but this is useful if you want
>>>>> to e.g
>>>>> use pg_receivexlog to back up a server that runs on a different
>>>>> platform.
>>>>>
>>>>> I chose the int64 format to transmit timestamps, even when compiled with
>>>>> --disable-integer-datetimes.
>>>>>
>>>>> Please review if you have the time..
>>>>
>>>>
>>>> Thanks for the patch!
>>>>
>>>> When I ran pg_receivexlog, I encountered the following error.
>>>
>>>
>>> Yeah, clearly I didn't test this near enough...
>>>
>>> I fixed the bugs you bumped into, new version attached.
>>
>>
>> Committed this now, after fixing a few more bugs that came up during
>> testing.
>
> As I suggested upthread, pg_basebackup and pg_receivexlog no longer
> need to check integer_datetimes before establishing the connection,
> thanks to this commit. If this is right, the attached patch should be applied.
> The patch just removes the check of integer_datetimes by pg_basebackup
> and pg_receivexlog.

Another comment that I made upthread is:

--------
In XLogWalRcvSendReply() and XLogWalRcvSendHSFeedback(),
GetCurrentTimestamp() is called twice. I think that we can skip the
latter call if integer-datetime is enabled because the return value of
GetCurrentTimestamp() and GetCurrentIntegerTimestamp() is in the
same format. It's worth reducing the number of GetCurrentTimestamp()
calls, I think.
--------

Attached patch removes redundant GetCurrentTimestamp() call
from XLogWalRcvSendReply() and XLogWalRcvSendHSFeedback(),
if --enable-integer-datetimes.

Regards,

--
Fujii Masao