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.
Please let me know if anything apart from above needs to be taken care.
Regards,
Hari babu.