Skip site navigation (1) Skip section navigation (2)

Re: Review of "pg_basebackup and pg_receivexlog to use non-blocking socket communication", was: Re: Re: [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown

From: Hari Babu <haribabu(dot)kommi(at)huawei(dot)com>
To: "'Boszormenyi Zoltan'" <zb(at)cybertec(dot)at>
Cc: "'Fujii Masao'" <masao(dot)fujii(at)gmail(dot)com>, "'Heikki Linnakangas'"<hlinnakangas(at)vmware(dot)com>, <pgsql-hackers(at)postgresql(dot)org>, "'Amit kapila'"<amit(dot)kapila(at)huawei(dot)com>
Subject: Re: Review of "pg_basebackup and pg_receivexlog to use non-blocking socket communication", was: Re: Re: [BUGS] BUG #7534: walreceiver takes long time to detect n/w breakdown
Date: 2013-01-04 12:43:08
Message-ID: 007701cdea79$0cf35b30$26da1190$@kommi@huawei.com (view raw or flat)
Thread:
Lists: pgsql-bugspgsql-hackers
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.


Attachment: pg_basebkup_recvxlog_noblock_comm_v2.patch
Description: application/octet-stream (24.3 KB)

In response to

Responses

pgsql-hackers by date

Next:From: Amit KapilaDate: 2013-01-04 13:53:58
Subject: Re: Performance Improvement by reducing WAL for Update Operation
Previous:From: Samuel VogelDate: 2013-01-04 12:00:21
Subject: Re: Print b-tree tuples

pgsql-bugs by date

Next:From: dagDate: 2013-01-04 14:06:51
Subject: BUG #7785: Bad plan for UNION ALL view containing JOIN
Previous:From: Dave PageDate: 2013-01-04 11:39:36
Subject: Re: BUG #7781: pgagent incorrect installation

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group