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. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers