On January 02, 2013 12:41 PM Hari Babu wrote:
>On January 01, 2013 10:19 PM Boszormenyi Zoltan wrote:
>>I am reviewing your patch.
>> Is the patch in context diff format?
>>Yes.
>
>Thanks for reviewing the patch.
>
>> Does it apply cleanly to the current git master?
>>Not quite cleanly but it doesn't produce rejects or fuzz, only offset
warnings:
>
>Will rebase the patch to head.
>
>> Does it include reasonable tests, necessary doc patches, etc?
>>The test cases are not applicable. There is no test framework for
>>testing network outage in "make check".
>>
>>There are no documentation patches for the new --recvtimeout=INTERVAL
>>and --conntimeout=INTERVAL options for either pg_basebackup or
>>pg_receivexlog.
>
>I will add the documentation for the same.
>
>>Per the previous comment, no. But those are for the backend
>>to notice network breakdowns and as such, they need a
>>separate patch.
>
>I also think it is better to handle it as a separate patch for walsender.
>
>> Are the comments sufficient and accurate?
>>This chunk below removes a comment which seems obvious enough
>>so it's not needed:
>>***************
>>*** 518,524 **** ReceiveXlogStream(PGconn *conn, XLogRecPtr startpos,
uint32 timeline,
>> goto error;
>> }
>>
>>! /* Check the message type. */
>> if (copybuf[0] == 'k')
>> {
>> int pos;
>>--- 559,568 ----
>> goto error;
>> }
>>
>>! /* Set the last reply timestamp */
>>! last_recv_timestamp = localGetCurrentTimestamp();
>>! ping_sent = false;
>>!
>> if (copybuf[0] == 'k')
>> {
>> int pos;
>>***************
>>
>>Other comments are sufficient and accurate.
>
>I will fix and update the patch.
The attached V2 patch in the mail handles all the review comments identified
above.
Regards,
Hari babu.